1969820 - Pasting clipboard content in Firefox appends U+0000 null characters — occurs with Windows echo text | clip and PowerPro clip.set("text")

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect)

Description

[fraster]

Steps to reproduce:

in Windows execute
echo text|clip
or
in powerpro
clipset("text"

Actual results:

Paste the clipboard content into in input of url bar field using right-click → Paste then input field contains: "test" that includes 4 null bytes (0x00 0x00 0x00 0x00) as padding or terminator

Expected results:

input field contains: "test" without extra character

Comment 1

[fraster]

see:
https://interno.altervista.org/temp/firefoxClip.gif
and
https://groups.io/g/PowerPro/topic/113401820#msg3565

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::DOM: Copy & Paste and Drag & Drop' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 3

[Alice0775 White]

I can reproduce the issue on Nightly141.0a1 Windows11.

Regression window:
https://hg-edge.mozilla.org/integration/autoland/pushloghtml?fromchange=d74aee22f0ac3d2462dc623d35274e5701e054e1&tochange=a7594596cdb1b28f26589d2a1113f767837ac2b0

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

:handyman, since you are the author of the regressor, bug 1966443, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 5

[fraster]

It occour also in input html tag.

Comment 6

[fraster]

I missing echo in cmd dos:
echo test|clip

Comment 7

[David Parks [:handyman]]

I'll just remove all of the null bytes at the end of the string (modulo Word's stray LF). This means that Windows will behave differently than Linux and Mac but I guess they don't have the same tradition of applications putting garbage on the clipboard. To recount:

• The clipboard docs say that the clipboard data is terminated by a null byte, despite it having a separate length value. This belief was presumably the origin of the original code, and was therefore the cause of bug 1966443.
• To fix that, we ignored docs and used the count, but removed the last char if it was null because "a lot of things were appending a null". So that was already a hack to get around bad clipboard contents.
• That led to bug 1968267 -- Word was appending a null followed by a line feed to its contents. So we started also skipping that 2 byte combination as well.
• This bug is about other apps (like the terminal piping output to the clipboard) adding streams of nulls to the end of their byte count. So we'll ignore that, too. We could probably use the solution that bug 1852790 comment 9 says Chrome uses, of removing all nulls, but that's further from our Linux and Mac implementations and is more severe.

Comment 8

[David Parks [:handyman]]

Attachment: Bug 1969820: Remove 0x00 0x0a and all nulls from clipboard r=#win-reviewers!

We previously removed (0x00 0x0a | 0x00)? from the end of the contents.
We now remove (0x00 0x0a)? from the end, and any 0x00 found anywhere in the
contents.

Comment 9

[David Parks [:handyman]]

Attachment: Bug 1969820: Remove 0x00 0x0a and trailing nulls from clipboard r=#win-reviewers!

We previously removed (0x00 0x0a|0x00)?. We now remove (0x00)*(0x00 0x0a)?.

Comment 10

[David Parks [:handyman]]

We'll choose one of these.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/547b1585d778
https://hg.mozilla.org/integration/autoland/rev/6eebb919b816
Remove 0x00 0x0a and trailing nulls from clipboard r=win-reviewers,gstoll

Comment 12

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/6eebb919b816

Comment 13

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:handyman, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
  • See https://wiki.mozilla.org/Release_Management/Requesting_an_Uplift for documentation on how to request an uplift.
• If no, please set status-firefox140 to wontfix.

For more information, please visit BugBot documentation.

Comment 14

[Greg Stoll :gstoll]

Attachment: Bug 1969820: Remove 0x00 0x0a and trailing nulls from clipboard r=#win-reviewers!

We previously removed (0x00 0x0a|0x00)?. We now remove (0x00)*(0x00 0x0a)?.

Original Revision: https://phabricator.services.mozilla.com/D252588

Comment 15

[Phabricator Automation]

firefox-beta Uplift Approval Request

• User impact if declined: extra characters at the end of pasted strings from some Windows applications
• Code covered by automated testing: no
• Fix verified in Nightly: yes
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: n/a
• Risk associated with taking this patch: low
• Explanation of risk level: Just stripping NULLs and the NULL/LF combo from the end of pasted strings
• String changes made/needed: no
• Is Android affected?: no

Comment 16

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/e7e395fb0a83
https://hg.mozilla.org/releases/mozilla-beta/rev/32b0599db950

Comment 17

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/4c02d92b62af
https://hg.mozilla.org/releases/mozilla-beta/rev/698fe0a437e8

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/4cbd07eea6ce
https://hg.mozilla.org/integration/autoland/rev/bcb64d8b5ed5
Revert "Bug 1969820: Remove 0x00 0x0a and trailing nulls from clipboard r=win-reviewers,gstoll" as requested by Meehan.

Comment 19

[Donal Meehan [:dmeehan]]

Backed out from beta, as part of backing out the regressor Bug 1966443

Comment 20

[agoloman]

Backout merged into central: https://hg-edge.mozilla.org/mozilla-central/rev/bcb64d8b5ed5235d0a8f95038616faec8b107224

Comment 21

[David Parks [:handyman]]

Fixed by backout