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

Issue 790955 link

Starred by 17 users

Docs duplicates attributes on `<script>` elements, which recent CSP hardening breaks.

Reported by nutan.ga...@etouch.net, Dec 1 2017

Issue description

Chrome Version: 64.0.3282.0 5fdc0fab22ce7efd32532ee989b223fa12f8171e-refs/heads/master@{#520840} 
OS: Windows(7,8,10), Linux(14.1 LTS)

Steps to Reproduce:
1. Launch chrome, navigate to https://docs.google.com.
2. Sign-in with valid credential and observe google docs page

Actual: Google Docs is unresponsive
Expected: Google Docs should work proper

This is a Regression issue and will soon update other info.
Good Build: 64.0.3281.0
Bad Build: 64.0.3282.0

 
Cc: msrchandra@chromium.org ranjitkan@chromium.org rbasuvula@chromium.org nyerramilli@chromium.org
Components: Platform>Apps
Labels: -Type-Bug -Pri-3 M-64 hasbisect-per-revision OS-Linux OS-Mac OS-Windows Pri-1 Type-Bug-Regression
Owner: mkwst@chromium.org
Status: Assigned (was: Unconfirmed)
This is a Regression issue broken in M-64 and below is the bisect info.
Good Build: 64.0.3281.0
Bad Build: 64.0.3282.0

You are probably looking for a change made after 520521 (known good), but no later than 520522 (first known bad).

CHANGELOG URL:
The script might not always return single CL as suspect as some perf builds might get missing due to failure.

https://chromium.googlesource.com/chromium/src/+log/4e197153a40aa5b298784c6132cac985d42a471e..f2e953cf9384199344e12d54658565a7e5ecde2e

Suspect: https://chromium.googlesource.com/chromium/src/+/f2e953cf9384199344e12d54658565a7e5ecde2e

Note: Issue is also reproducible on Mac (10.12.6, 10.13.2)


Actual Video.mp4
877 KB View Download
Expected Video.mp4
1.3 MB View Download

Comment 2 by mkwst@chromium.org, Dec 1 2017

Components: Blink>SecurityFeature>ContentSecurityPolicy
Labels: ReleaseBlock-Stable
Summary: Docs duplicates attributes on `<script>` elements, which recent CSP hardening breaks. (was: Regression: Google apps (i.e.Docs, Slides, Sheets) are unresponsive after sign in )
Yup. Docs is repeating attributes on script elements, probably accidentally.

I filed b/70008415 against Docs. If it turns out not to be simple for them to fix, I'll revert the patch from 64. Keeping this open either way, and marking as a stable blocker.

Comment 3 by mkwst@chromium.org, Dec 1 2017

Cc: lmiranda@google.com
Talked to Docs folks: it's not clear this will be trivial for them to fix, and the end of year freeze is risky to butt up against. I'm uploading a patch now to pull the behavior back behind a flag, and we'll try it again once Docs has a chance to fix things up.
Project Member

Comment 4 by bugdroid1@chromium.org, Dec 1 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/659d8949be45590f81159b8553b94153457f5744

commit 659d8949be45590f81159b8553b94153457f5744
Author: Mike West <mkwst@chromium.org>
Date: Fri Dec 01 15:28:22 2017

CSP: Pull duplicate attribute hardening back out.

I expected this to be a no-op, but broke Docs. C'est la vie.
Manually verified that Docs isn't broken after this patch.

Bug:  790955 
Change-Id: Id8290a5c1f826d1f374e6531de8e8433a814b21c
Reviewed-on: https://chromium-review.googlesource.com/803478
Commit-Queue: Mike West <mkwst@chromium.org>
Reviewed-by: Andy Paicu <andypaicu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520944}
[add] https://crrev.com/659d8949be45590f81159b8553b94153457f5744/third_party/WebKit/LayoutTests/external/wpt/content-security-policy/script-src/nonce-enforce-blocked-expected.txt
[modify] https://crrev.com/659d8949be45590f81159b8553b94153457f5744/third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicy.cpp

Comment 5 by mkwst@chromium.org, Dec 4 2017

Status: Started (was: Assigned)
Leaving this open to request a merge back to 64 if it turns out that the first patch made it in before the branch.
Labels: TE-Verified-M65 TE-Verified-65.0.3284.0
Tested above issue on Windows (7,8,8.1,10),Linux (14.04 LTS),Mac OS X(10.12.6,10.13.2) using Canary #65.0.3284.0 and issue is fixed. Kindly review an attached video.

Thank you!
Current_Result.mp4
1.2 MB View Download
Cc: mmanchala@chromium.org
 Issue 791241  has been merged into this issue.

Comment 8 by mkwst@chromium.org, Dec 4 2017

Labels: Merge-Request-64
Hello, lovely release manager folks. The original patch made it in just under the wire for 64, so it would be great to merge this followup back to fix Docs. :)
Labels: ReleaseBlock-Beta
I don't think we could ship a beta with this bug.
(I agree with Eric, FWIW)
 Issue 791571  has been merged into this issue.
 Issue 791553  has been merged into this issue.
Cc: hirosh...@chromium.org
Issue 791834 has been merged into this issue.
Cc: abdulsyed@chromium.org
/cc abdulsyed@ for visibility. Given the number of dupes already, it would be nice to merge this back. :)
Can we please get confirmation that this bug won't make into 64? As it stands, Docs Editors will be unusable if 64 is shipped with this bug.
As is, docs is broken in the M64 branch and working in the M65 branch. I fully expect we'll be able to merge the patch above (https://chromium-review.googlesource.com/803478) back to that branch; I'm just waiting for release manager approval.
 Issue 791546  has been merged into this issue.
Project Member

Comment 19 by sheriffbot@chromium.org, Dec 5 2017

Labels: -Merge-Request-64 Hotlist-Merge-Approved Merge-Approved-64
Your change meets the bar and is auto-approved for M64. Please go ahead and merge the CL to branch 3282 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 20 by bugdroid1@chromium.org, Dec 5 2017

Labels: -merge-approved-64 merge-merged-3282
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bc5cf65496f5b37214eb581ee0ca88f605aa2e90

commit bc5cf65496f5b37214eb581ee0ca88f605aa2e90
Author: Mike West <mkwst@chromium.org>
Date: Tue Dec 05 16:39:31 2017

CSP: Pull duplicate attribute hardening back out.

I expected this to be a no-op, but broke Docs. C'est la vie.
Manually verified that Docs isn't broken after this patch.

TBR=mkwst@chromium.org

(cherry picked from commit 659d8949be45590f81159b8553b94153457f5744)

Bug:  790955 
Change-Id: Id8290a5c1f826d1f374e6531de8e8433a814b21c
Reviewed-on: https://chromium-review.googlesource.com/803478
Commit-Queue: Mike West <mkwst@chromium.org>
Reviewed-by: Andy Paicu <andypaicu@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#520944}
Reviewed-on: https://chromium-review.googlesource.com/809006
Reviewed-by: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/branch-heads/3282@{#30}
Cr-Branched-From: 5fdc0fab22ce7efd32532ee989b223fa12f8171e-refs/heads/master@{#520840}
[add] https://crrev.com/bc5cf65496f5b37214eb581ee0ca88f605aa2e90/third_party/WebKit/LayoutTests/external/wpt/content-security-policy/script-src/nonce-enforce-blocked-expected.txt
[modify] https://crrev.com/bc5cf65496f5b37214eb581ee0ca88f605aa2e90/third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicy.cpp

 Issue 791943  has been merged into this issue.

Comment 22 by dhw@chromium.org, Dec 5 2017

 Issue 791857  has been merged into this issue.
Cc: kbleicher@chromium.org
Labels: OS-Chrome
Able to reproduce the issue on Chrome 64.0.3282.7/CrOS 10176.4.0 - Peppy, Paine
Labels: -ReleaseBlock-Beta -ReleaseBlock-Stable ReleaseBlock-Dev
Unable to reproduce the issue on previous release build (64.0.3280.5/1072.0.0-Candy). Mark it as "ReleaseBlock_dev" since GoogleDoc is top priority of Google properties test scope. 

Cc: manoranj...@chromium.org
mkwst@- thanks for the fix. So #20 should fix the issue correct?
songsuk@:  Is this blocker limited to these two boards?  We're actively working to create / push a DEV RC... 

Can we prioritize if not done already?

Thanks

Comment 27 by w...@chromium.org, Dec 5 2017

Re #26: No, this is a general issue for all Chrome platforms and ChromeOS boards; the merge to M64 in #20 should have resolved the issue.
Cc: abod...@chromium.org sdantul...@chromium.org dhadd...@chromium.org
reproduced on daisy and cave CrOS 10176.4.0/64.0.3282.7
also, google spreadsheets is not working and always popup message as "still loading ..."

Comment 31 by w...@chromium.org, Dec 5 2017

Status: Fixed (was: Started)
Breaking change has been reverted (see comment #4) and fix merged back to M64 (see comment #20), so closing this out as Fixed.
looks good at chrome OS 10176.5.0/64.0.3282.11. and filled reload issue at https://bugs.chromium.org/p/chromium/issues/detail?id=792567
Labels: Hotlist-ConOps

Sign in to add a comment