New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 821690 link

Starred by 9 users

Pasting from clipboard pastes excess garbage at the end

Reported by arnav...@gmail.com, Mar 14 2018

Issue description

UserAgent: Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/66.0.3359.22 Safari/537.36

Steps to reproduce the problem:
1. Open a cmd.exe window and type some text, like the letter "a"
2. Select and copy the "a" from the cmd.exe window
3. Paste it in the chrome omnibar

What is the expected behavior?
"a" gets pasted

What went wrong?
"a" followed by garbage gets pasted

Did this work before? Yes Likely introduced by https://chromium-review.googlesource.com/c/chromium/src/+/803798

Chrome version: 66.0.3359.22  Channel: n/a
OS Version: 6.1 (Windows 7, Windows Server 2008 R2)
Flash Version: 

When receiving CF_UNICODETEXT data with GetClipboardData, that commit uses GlobalSize to get the length of the clipboard text. On attaching a debugger and breaking on GetClipboardData and GlobalSize, I can see that GlobalSize returns a larger value than 2 or 4 that that code would expect for a single letter copied. It actually returns 36. Similarly for longer text copied it returns a longer value than the byte length.

GlobalSize not being appropriate for getting the length of text is documented at least here: https://stackoverflow.com/questions/22075754/

This only happens when pasting text copied from conhost windows, like cmd.exe, powershell.exe and pwsh.exe. I did not investigate why, but I did observe that pasting the text from cmd.exe into another text application, copying from *there*, and then pasting into chrome has no garbage.
 
Same here except it's in Chrome 65.0.3325.146 official build, 64bit, Windows 7.
Hope it helps to identify source of the issue.

Comment 2 by arnav...@gmail.com, Mar 14 2018

Yes, the commit I linked was introduced in 65.0.3292.0 per git tags, so every release after that will be affected.
Labels: Needs-Bisect Needs-Triage-M66
Cc: sindhu.chelamcherla@chromium.org
Components: -UI UI>Browser>Omnibox Blink>DataTransfer
Labels: -Pri-2 -Needs-Bisect FoundIn-66 Triaged-ET FoundIn-67 M-65 RegressedIn-65 Target-67 Target-66 Target-65 FoundIn-65 ReleaseBlock-Stable hasbisect Pri-1
Owner: chrisha@chromium.org
Status: Assigned (was: Unconfirmed)
Able to reproduce this issue on reported version 66.0.3359.22, on latest stable 65.0.3325.168, on latest beta 66.0.3359.33 and on latest canary 67.0.3371.0 using Windows 7 with steps mentioned in comment#0. Attaching screenshot of observing behavior. Issue is not seen in Windows 10, Ubuntu 14.04 and Mac 10.13.3

Good Build: 65.0.3391.0
Bad Build: 65.0.3392.0

Unable to provide per-revision bisect as we don't have setup as of now. Hence providing Chromium bisect results.

CHANGELOG URL:

https://chromium.googlesource.com/chromium/src/+log/d04afb748bd39048c0a27556fcbcdf86f080bc7b..54f63b81e9ba3b727b83c8ec972f47f2fe2a441b

Suspecting  https://chromium-review.googlesource.com/803798 from changelog.

@ chrisha: Please confirm the bug and help in re-assigning if it is not related to your change. Also adding RB-Stable for M-65. Please remove if not the case.

Thanks!
821690.png
2.2 KB View Download
I'm unable to reproduce this on latest M67 canary on either Win7 or Win10. It looks like this issue might depend strongly on the exact OS version and service pack?

In terms of blocking release, pasting to the Omnibox is done by 22% of the user population, so yes, I'd say this is a problem.

The first comment details the issue quite clearly. The previous behaviour was broken in that we didn't respect GlobalSize, and would actually read past the end of the buffer sometimes, as not all applications append a terminating null to the clipboard data. When fixing this I was testing on Win10, where the GlobalSize is always accurate. The correct behaviour would be to respect both GlobalSize as a max, and look for a terminating null.

Comment 6 by gov...@chromium.org, Mar 15 2018

Labels: M-66 M-67
Fix landed here (with a typo'd bug number):

https://chromium-review.googlesource.com/c/chromium/src/+/964722
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/56f6bc94eae3c761aedf9468dbe8d4c0471899e7

commit 56f6bc94eae3c761aedf9468dbe8d4c0471899e7
Author: Chris Hamilton <chrisha@chromium.org>
Date: Thu Mar 15 20:43:30 2018

Fix trailing garbage when pasting from clipboard.

A previous CL used GlobalSize as the explicit size of clipboard data, removing
trailing nulls. This was a fix to open-ended copying logic that would
occasionally read past the end of the buffer for clipboard entries not
containing terminating nulls. It turns out that some clipboard entries
contain much less data than the entire buffer, and don't pad out to the
end with terminating nulls (only seen on Win7 when copying from command line or
other console apps). The fix is to copy until first null or end of the
buffer.

BUG= 821690 

Change-Id: Id0e733ccf10b0f7d2f7ec401a584d7f14d4fd063
Reviewed-on: https://chromium-review.googlesource.com/964722
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Chris Hamilton <chrisha@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543487}
[modify] https://crrev.com/56f6bc94eae3c761aedf9468dbe8d4c0471899e7/ui/base/clipboard/clipboard_win.cc

Comment 9 by arnav...@gmail.com, Mar 15 2018

@ chrisha: I agree with your proposed fix.

Out of curiosity, when you say you're unable to reproduce this on Windows 7, you're copied the text from a cmd.exe / powershell.exe / pwsh.exe window?
Waiting for confirmation of fix on canary, then will request merge to M65 and M66.

Re comment #9: Yeah, I tried from cmd.exe on an old Win7 laptop. No matter the length of text copied, I never saw any additional junk.
NextAction: 2018-03-16
Thank you chrisha@ for quick fix. Pls update the bug with canary result tomorrow. 
The NextAction date has arrived: 2018-03-16
Labels: TE-Verified-M67 TE-Verified-67.0.3372.0
Able to reproduce this issue on reported version 66.0.3359.22. Hence verifying the fix on latest canary 67.0.3372.0 using Windows 7.

Observing "a" only on copying and pasting from cmd.exe to Omnibox. Attaching screenshot for reference. Hence adding TE-Verified labels.

Thanks!
821690_ 67.0.3372.0.PNG
19.3 KB View Download
Status: Fixed (was: Assigned)
Labels: Merge-TBD
[Auto-generated comment by a script] We noticed that this issue is targeted for M-65; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-65 label, otherwise remove Merge-TBD label. Thanks.
Labels: -Merge-TBD Merge-Request-66 Merge-Request-65
Requesting merge to M65 and M66. This is a very low risk change involving a few lines in a single file.

(Waiting for test team to verify the fix, but works fine for me locally.)
Status: Verified (was: Fixed)
Verified by test team as fixed in 67.0.3372.0.
Project Member

Comment 18 by sheriffbot@chromium.org, Mar 16 2018

Labels: -Merge-Request-66 Merge-Review-66 Hotlist-Merge-Review
This bug requires manual review: M66 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), josafat@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Looking at go/crash, there are no crashes at all in ui/base/clipboard on this canary version, so doesn't look like the fix is causing any issues.
Labels: -Merge-Request-65 Merge-Approved-65
Approving merge to M65 branch 3325 based on comments #13 and  #19. Please merge ASAP. Thank you.
Labels: -Merge-Review-66 Merge-Approved-66
Approved 66, branch:3359
Project Member

Comment 22 by bugdroid1@chromium.org, Mar 16 2018

Labels: -merge-approved-66 merge-merged-3359
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/39939d8aaa5b0efc42e978c9449b981b13aa8a89

commit 39939d8aaa5b0efc42e978c9449b981b13aa8a89
Author: Chris Hamilton <chrisha@chromium.org>
Date: Fri Mar 16 17:10:50 2018

[Merge M66] Fix trailing garbage when pasting from clipboard.

A previous CL used GlobalSize as the explicit size of clipboard data, removing
trailing nulls. This was a fix to open-ended copying logic that would
occasionally read past the end of the buffer for clipboard entries not
containing terminating nulls. It turns out that some clipboard entries
contain much less data than the entire buffer, and don't pad out to the
end with terminating nulls (only seen on Win7 when copying from command line or
other console apps). The fix is to copy until first null or end of the
buffer.

BUG= 821690 

Change-Id: Id0e733ccf10b0f7d2f7ec401a584d7f14d4fd063
Reviewed-on: https://chromium-review.googlesource.com/964722
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Chris Hamilton <chrisha@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#543487}(cherry picked from commit 56f6bc94eae3c761aedf9468dbe8d4c0471899e7)
Reviewed-on: https://chromium-review.googlesource.com/966685
Reviewed-by: Chris Hamilton <chrisha@chromium.org>
Cr-Commit-Position: refs/branch-heads/3359@{#286}
Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276}
[modify] https://crrev.com/39939d8aaa5b0efc42e978c9449b981b13aa8a89/ui/base/clipboard/clipboard_win.cc

Project Member

Comment 23 by bugdroid1@chromium.org, Mar 16 2018

Labels: -merge-approved-65 merge-merged-3325
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/008e5b991e0703295b6c1dffa70f84f5417316f0

commit 008e5b991e0703295b6c1dffa70f84f5417316f0
Author: Chris Hamilton <chrisha@chromium.org>
Date: Fri Mar 16 17:15:06 2018

[Merge M65] Fix trailing garbage when pasting from clipboard.

A previous CL used GlobalSize as the explicit size of clipboard data, removing
trailing nulls. This was a fix to open-ended copying logic that would
occasionally read past the end of the buffer for clipboard entries not
containing terminating nulls. It turns out that some clipboard entries
contain much less data than the entire buffer, and don't pad out to the
end with terminating nulls (only seen on Win7 when copying from command line or
other console apps). The fix is to copy until first null or end of the
buffer.

BUG= 821690 

Change-Id: Id0e733ccf10b0f7d2f7ec401a584d7f14d4fd063
Reviewed-on: https://chromium-review.googlesource.com/964722
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Chris Hamilton <chrisha@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#543487}(cherry picked from commit 56f6bc94eae3c761aedf9468dbe8d4c0471899e7)
Reviewed-on: https://chromium-review.googlesource.com/966686
Reviewed-by: Chris Hamilton <chrisha@chromium.org>
Cr-Commit-Position: refs/branch-heads/3325@{#712}
Cr-Branched-From: bc084a8b5afa3744a74927344e304c02ae54189f-refs/heads/master@{#530369}
[modify] https://crrev.com/008e5b991e0703295b6c1dffa70f84f5417316f0/ui/base/clipboard/clipboard_win.cc

Thank you chrisha@ for quick fix and merge. 
Requesting postmortem for this. Please see go/chrome-postmortems for the process to follow. 

Comment 25 Deleted

Pls also verify this bug on M65 version 65.0.3325.172 or higher.
Labels: TE-Verified-M65 TE-Verified-65.0.3325.177
Able to reproduce this issue on reported version 66.0.3359.22. Hence verifying the fix on latest M-65 65.0.3325.177 using Windows 7.

Observing "a" only on copying and pasting from cmd.exe to Omnibox. Attaching screenshot for reference. Hence adding TE-Verified labels.

Thanks!
821690_65.0.3325.177.PNG
11.4 KB View Download
Was this a big enough issue to warrant a post-mortem? Or is it simply standard practice for any merge back to stable?
It is a standard practice, to request a postmortem for any issue which caused us to do respin a Chrome release that was shipping to the Stable channel. Please let me know if you've any further question. Thank you.
Cc: krajshree@chromium.org kkaluri@chromium.org
 Issue 820855  has been merged into this issue.
This patch is now pushing out to stable channel in version 65.0.3325.181 for Desktop (Win,Mac & Linux).
Labels: TE-Verified-66.0.3359.45
Able to reproduce this issue on reported version 66.0.3359.22. Hence verifying the fix on latest M-66 66.0.3359.45 using Windows 7.

Observing "a" only on copying and pasting from cmd.exe to Omnibox. Attaching screenshot for reference. Hence adding TE-Verified labels.

Thanks!
Cc: chrisha@chromium.org
 Issue 803089  has been merged into this issue.

Sign in to add a comment