Issue metadata
Sign in to add a comment
|
Pasting from clipboard pastes excess garbage at the end
Reported by
arnav...@gmail.com,
Mar 14 2018
|
||||||||||||||||||||||
Issue descriptionUserAgent: 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.
,
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.
,
Mar 14 2018
,
Mar 15 2018
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!
,
Mar 15 2018
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.
,
Mar 15 2018
,
Mar 15 2018
Fix landed here (with a typo'd bug number): https://chromium-review.googlesource.com/c/chromium/src/+/964722
,
Mar 15 2018
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
,
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?
,
Mar 15 2018
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.
,
Mar 15 2018
Thank you chrisha@ for quick fix. Pls update the bug with canary result tomorrow.
,
Mar 16 2018
The NextAction date has arrived: 2018-03-16
,
Mar 16 2018
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!
,
Mar 16 2018
,
Mar 16 2018
[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.
,
Mar 16 2018
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.)
,
Mar 16 2018
Verified by test team as fixed in 67.0.3372.0.
,
Mar 16 2018
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
,
Mar 16 2018
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.
,
Mar 16 2018
Approving merge to M65 branch 3325 based on comments #13 and #19. Please merge ASAP. Thank you.
,
Mar 16 2018
Approved 66, branch:3359
,
Mar 16 2018
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
,
Mar 16 2018
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
,
Mar 16 2018
Thank you chrisha@ for quick fix and merge. Requesting postmortem for this. Please see go/chrome-postmortems for the process to follow.
,
Mar 19 2018
Pls also verify this bug on M65 version 65.0.3325.172 or higher.
,
Mar 19 2018
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!
,
Mar 19 2018
Was this a big enough issue to warrant a post-mortem? Or is it simply standard practice for any merge back to stable?
,
Mar 19 2018
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.
,
Mar 20 2018
,
Mar 20 2018
This patch is now pushing out to stable channel in version 65.0.3325.181 for Desktop (Win,Mac & Linux).
,
Mar 21 2018
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!
,
Apr 9 2018
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by jiri.no...@gmail.com
, Mar 14 2018