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

Issue 690946 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug

Blocking:
issue 576261



Sign in to add a comment

PlzNavigate: Missing line number in console messages when the 'frame-src' CSP is infringed.

Project Member Reported by arthurso...@chromium.org, Feb 10 2017

Issue description

With PlzNavigate, after the CL https://codereview.chromium.org/2655463006/ (Not landed yet -- 02/10/2017)

The console error message doesn't display the line number of the script that inserted the blocked iframe.

For instance, in the test:
http/tests/security/contentSecurityPolicy/1.1/child-src/frame-blocked.html

- CONSOLE ERROR: line 13: Refused to frame ...
+ CONSOLE ERROR: Refused to frame ...



 
Project Member

Comment 1 by bugdroid1@chromium.org, Mar 30 2017

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

commit 8bec3f2a2edf7217f83d10fe929816e698c64a35
Author: arthursonzogni <arthursonzogni@chromium.org>
Date: Thu Mar 30 11:55:52 2017

PlzNavigate & CSP. Use the SourceLocation in violation reports.

The SourceLocation struct is available during a navigation thanks to this CL:
https://codereview.chromium.org/2720763002

This patch makes use of it for CSP. It fixes several test where the line
number in console messages was missing.

BUG= 690946 ,705098
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation;master.tryserver.chromium.linux:linux_chromium_browser_side_navigation_rel

Review-Url: https://codereview.chromium.org/2761153003
Cr-Commit-Position: refs/heads/master@{#460727}

[modify] https://crrev.com/8bec3f2a2edf7217f83d10fe929816e698c64a35/content/browser/frame_host/ancestor_throttle.cc
[modify] https://crrev.com/8bec3f2a2edf7217f83d10fe929816e698c64a35/content/browser/frame_host/form_submission_throttle.cc
[modify] https://crrev.com/8bec3f2a2edf7217f83d10fe929816e698c64a35/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/8bec3f2a2edf7217f83d10fe929816e698c64a35/content/browser/frame_host/render_frame_host_impl.h
[modify] https://crrev.com/8bec3f2a2edf7217f83d10fe929816e698c64a35/content/common/content_security_policy/content_security_policy.cc
[modify] https://crrev.com/8bec3f2a2edf7217f83d10fe929816e698c64a35/content/common/content_security_policy/content_security_policy.h
[modify] https://crrev.com/8bec3f2a2edf7217f83d10fe929816e698c64a35/content/common/content_security_policy/content_security_policy_unittest.cc
[modify] https://crrev.com/8bec3f2a2edf7217f83d10fe929816e698c64a35/content/common/content_security_policy/csp_context.cc
[modify] https://crrev.com/8bec3f2a2edf7217f83d10fe929816e698c64a35/content/common/content_security_policy/csp_context.h
[modify] https://crrev.com/8bec3f2a2edf7217f83d10fe929816e698c64a35/content/common/content_security_policy/csp_context_unittest.cc
[modify] https://crrev.com/8bec3f2a2edf7217f83d10fe929816e698c64a35/content/common/frame_messages.h
[modify] https://crrev.com/8bec3f2a2edf7217f83d10fe929816e698c64a35/content/renderer/content_security_policy_util.cc
[modify] https://crrev.com/8bec3f2a2edf7217f83d10fe929816e698c64a35/third_party/WebKit/LayoutTests/FlagExpectations/enable-browser-side-navigation
[modify] https://crrev.com/8bec3f2a2edf7217f83d10fe929816e698c64a35/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
[modify] https://crrev.com/8bec3f2a2edf7217f83d10fe929816e698c64a35/third_party/WebKit/public/platform/WebContentSecurityPolicyStruct.h
[rename] https://crrev.com/8bec3f2a2edf7217f83d10fe929816e698c64a35/third_party/WebKit/public/platform/WebSourceLocation.h
[modify] https://crrev.com/8bec3f2a2edf7217f83d10fe929816e698c64a35/third_party/WebKit/public/web/WebConsoleMessage.h
[modify] https://crrev.com/8bec3f2a2edf7217f83d10fe929816e698c64a35/third_party/WebKit/public/web/WebDataSource.h
[modify] https://crrev.com/8bec3f2a2edf7217f83d10fe929816e698c64a35/third_party/WebKit/public/web/WebFrameClient.h

Cc: jam@chromium.org
Labels: Proj-PlzNavigate Proj-PlzNavigate-Blocking
mkwst@ Do you think this bug is blocking PlzNavigate for launch in M60?

I investigated and found what the problem is. The source location is captured in LocalFrameClientImpl::DecidePolicyForNavigation([...])
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/web/LocalFrameClientImpl.cpp?type=cs&l=555

```
  std::unique_ptr<SourceLocation> source_location =
      SourceLocation::Capture(web_frame_->GetFrame()->GetDocument());
```

The problem is that it is not always the frame that is navigating that has initiated the navigation. It might be its parent for instance.
Owner: arthurso...@chromium.org
Status: Started (was: Available)
Project Member

Comment 5 by bugdroid1@chromium.org, Jun 2 2017

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

commit ccfe0b5a32bc0ed36a95eb1da8e7f4b03268a661
Author: arthursonzogni <arthursonzogni@chromium.org>
Date: Fri Jun 02 10:40:27 2017

PlzNavigate: Fix wrong SourceLocation.

Before this CL, when an iframe is asked to navigate, the SourceLocation
used was the wrong one. It was always the one of the current document, even
when the navigation was initiated by another frame.

TEST=http/tests/security/contentSecurityPolicy/frame-src-cross-origin-load.html

The test still does not pass however. It is due to
 https://crbug.com/718942  which is not related to this issue.

BUG= 690946 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_browser_side_navigation_rel

Review-Url: https://codereview.chromium.org/2901223003
Cr-Commit-Position: refs/heads/master@{#476613}

[modify] https://crrev.com/ccfe0b5a32bc0ed36a95eb1da8e7f4b03268a661/third_party/WebKit/LayoutTests/FlagExpectations/enable-browser-side-navigation
[modify] https://crrev.com/ccfe0b5a32bc0ed36a95eb1da8e7f4b03268a661/third_party/WebKit/Source/core/frame/LocalFrameClient.h
[modify] https://crrev.com/ccfe0b5a32bc0ed36a95eb1da8e7f4b03268a661/third_party/WebKit/Source/core/loader/EmptyClients.cpp
[modify] https://crrev.com/ccfe0b5a32bc0ed36a95eb1da8e7f4b03268a661/third_party/WebKit/Source/core/loader/EmptyClients.h
[modify] https://crrev.com/ccfe0b5a32bc0ed36a95eb1da8e7f4b03268a661/third_party/WebKit/Source/core/loader/FrameLoader.cpp
[modify] https://crrev.com/ccfe0b5a32bc0ed36a95eb1da8e7f4b03268a661/third_party/WebKit/Source/core/loader/FrameLoader.h
[modify] https://crrev.com/ccfe0b5a32bc0ed36a95eb1da8e7f4b03268a661/third_party/WebKit/Source/web/LocalFrameClientImpl.cpp
[modify] https://crrev.com/ccfe0b5a32bc0ed36a95eb1da8e7f4b03268a661/third_party/WebKit/Source/web/LocalFrameClientImpl.h

Status: Fixed (was: Started)

Comment 7 by jam@chromium.org, Jun 8 2017

Labels: Merge-Request-60
Project Member

Comment 8 by sheriffbot@chromium.org, Jun 8 2017

Labels: -Merge-Request-60 Hotlist-Merge-Approved Merge-Approved-60
Your change meets the bar and is auto-approved for M60. Please go ahead and merge the CL to branch 3112 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), josafat@(ChromeOS), bustamante@(Desktop)

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

Comment 9 by bugdroid1@chromium.org, Jun 8 2017

Labels: -merge-approved-60 merge-merged-3112
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/536326a013d7b979106706b80e964dc803c160c5

commit 536326a013d7b979106706b80e964dc803c160c5
Author: John Abd-El-Malek <jam@chromium.org>
Date: Thu Jun 08 17:20:06 2017

PlzNavigate: Fix wrong SourceLocation.

Before this CL, when an iframe is asked to navigate, the SourceLocation
used was the wrong one. It was always the one of the current document, even
when the navigation was initiated by another frame.

TEST=http/tests/security/contentSecurityPolicy/frame-src-cross-origin-load.html

The test still does not pass however. It is due to
 https://crbug.com/718942  which is not related to this issue.

BUG= 690946 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_browser_side_navigation_rel

Review-Url: https://codereview.chromium.org/2901223003
Cr-Original-Commit-Position: refs/heads/master@{#476613}
Review-Url: https://codereview.chromium.org/2924293002 .
Cr-Commit-Position: refs/branch-heads/3112@{#257}
Cr-Branched-From: b6460e24cf59f429d69de255538d0fc7a425ccf9-refs/heads/master@{#474897}

[modify] https://crrev.com/536326a013d7b979106706b80e964dc803c160c5/third_party/WebKit/LayoutTests/FlagExpectations/enable-browser-side-navigation
[modify] https://crrev.com/536326a013d7b979106706b80e964dc803c160c5/third_party/WebKit/Source/core/frame/LocalFrameClient.h
[modify] https://crrev.com/536326a013d7b979106706b80e964dc803c160c5/third_party/WebKit/Source/core/loader/EmptyClients.cpp
[modify] https://crrev.com/536326a013d7b979106706b80e964dc803c160c5/third_party/WebKit/Source/core/loader/EmptyClients.h
[modify] https://crrev.com/536326a013d7b979106706b80e964dc803c160c5/third_party/WebKit/Source/core/loader/FrameLoader.cpp
[modify] https://crrev.com/536326a013d7b979106706b80e964dc803c160c5/third_party/WebKit/Source/core/loader/FrameLoader.h
[modify] https://crrev.com/536326a013d7b979106706b80e964dc803c160c5/third_party/WebKit/Source/web/LocalFrameClientImpl.cpp
[modify] https://crrev.com/536326a013d7b979106706b80e964dc803c160c5/third_party/WebKit/Source/web/LocalFrameClientImpl.h

Sign in to add a comment