Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(292)

Issue 414223004: Implement NavigationScheduler::schedulePageBlock() as a redirect to empty substitute data. (Closed)

Created:
6 years, 5 months ago by Tom Sepez
Modified:
6 years, 4 months ago
CC:
blink-reviews, gavinp+loader_chromium.org, Nate Chapin, mkwst+watchlist_chromium.org
Project:
blink
Visibility:
Public.

Description

Implement NavigationScheduler::schedulePageBlock() as a redirect to empty substitute data. This replaces the long-standing kludge of navigating to "data:," so that we preserve the URL of the page that was blocked. Otherwise, cross-origin detection of the XSSAuditor is possible via a variety of techniques owing to the change in the URL. We lose the benefit of the unique origin, however. I don't think actually provides any benefit, if only blank content is going into the replacement page. As a consequence, the parent frame will successfully see same-origin content in some of the tests. The cross-origin test remains unmodified, showing that there aren't new leaks (full-block-script-tag-cross-domain). The upside is I can remove a lot of logic that was introduced recently to preserve pages for view-source of the blocked page. The window-open-block-mode test is such an example. There will be more cleanup possible on the chrome side once this CL lands. BUG=396544 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=179240

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+34 lines, -65 lines) Patch
M LayoutTests/http/tests/security/contentSecurityPolicy/1.1/reflected-xss-and-xss-protection-allow-block-expected.txt View 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/http/tests/security/contentSecurityPolicy/1.1/reflected-xss-and-xss-protection-block-allow-expected.txt View 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/http/tests/security/contentSecurityPolicy/1.1/reflected-xss-and-xss-protection-block-block-expected.txt View 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/http/tests/security/contentSecurityPolicy/1.1/reflected-xss-and-xss-protection-block-filter-expected.txt View 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/http/tests/security/contentSecurityPolicy/1.1/reflected-xss-and-xss-protection-block-invalid-expected.txt View 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/http/tests/security/contentSecurityPolicy/1.1/reflected-xss-and-xss-protection-block-unset-expected.txt View 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/http/tests/security/contentSecurityPolicy/1.1/reflected-xss-and-xss-protection-filter-block-expected.txt View 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/http/tests/security/contentSecurityPolicy/1.1/reflected-xss-and-xss-protection-invalid-block-expected.txt View 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/http/tests/security/contentSecurityPolicy/1.1/reflected-xss-and-xss-protection-unset-block-expected.txt View 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/http/tests/security/xssAuditor/full-block-base-href-expected.txt View 1 chunk +0 lines, -1 line 0 comments Download
M LayoutTests/http/tests/security/xssAuditor/full-block-iframe-javascript-url-expected.txt View 1 chunk +0 lines, -1 line 0 comments Download
M LayoutTests/http/tests/security/xssAuditor/full-block-javascript-link-expected.txt View 1 chunk +0 lines, -1 line 0 comments Download
M LayoutTests/http/tests/security/xssAuditor/full-block-link-onclick-expected.txt View 1 chunk +0 lines, -1 line 0 comments Download
M LayoutTests/http/tests/security/xssAuditor/full-block-object-tag-expected.txt View 1 chunk +0 lines, -1 line 0 comments Download
M LayoutTests/http/tests/security/xssAuditor/full-block-script-tag.html View 2 chunks +3 lines, -3 lines 0 comments Download
M LayoutTests/http/tests/security/xssAuditor/full-block-script-tag-expected.txt View 1 chunk +1 line, -2 lines 0 comments Download
M LayoutTests/http/tests/security/xssAuditor/full-block-script-tag-with-source-expected.txt View 1 chunk +0 lines, -1 line 0 comments Download
D LayoutTests/http/tests/security/xssAuditor/window-open-block-mode.html View 1 chunk +0 lines, -29 lines 0 comments Download
D LayoutTests/http/tests/security/xssAuditor/window-open-block-mode-expected.txt View 1 chunk +0 lines, -13 lines 0 comments Download
M LayoutTests/http/tests/security/xssAuditor/xss-protection-parsing-03-expected.txt View 1 chunk +0 lines, -1 line 0 comments Download
M LayoutTests/http/tests/security/xssAuditor/xss-protection-parsing-04-expected.txt View 1 chunk +0 lines, -1 line 0 comments Download
M Source/core/loader/NavigationScheduler.cpp View 3 chunks +21 lines, -1 line 2 comments Download

Messages

Total messages: 7 (0 generated)
Tom Sepez
Please review: @mkwst - LayoutTests/ changes @japhet - NavigationScheduler.cpp @abarth - general sanity of what ...
6 years, 5 months ago (2014-07-25 19:41:02 UTC) #1
Nate Chapin
NavigationScheduler.cpp LGTM with an optional nit https://codereview.chromium.org/414223004/diff/1/Source/core/loader/NavigationScheduler.cpp File Source/core/loader/NavigationScheduler.cpp (right): https://codereview.chromium.org/414223004/diff/1/Source/core/loader/NavigationScheduler.cpp#newcode352 Source/core/loader/NavigationScheduler.cpp:352: const KURL& url ...
6 years, 5 months ago (2014-07-25 20:33:38 UTC) #2
abarth-chromium
LGTM
6 years, 4 months ago (2014-07-30 16:34:10 UTC) #3
Mike West
LGTM3, thanks, and apologies for the delay. https://codereview.chromium.org/414223004/diff/1/Source/core/loader/NavigationScheduler.cpp File Source/core/loader/NavigationScheduler.cpp (right): https://codereview.chromium.org/414223004/diff/1/Source/core/loader/NavigationScheduler.cpp#newcode187 Source/core/loader/NavigationScheduler.cpp:187: OwnPtr<UserGestureIndicator> gestureIndicator ...
6 years, 4 months ago (2014-07-30 16:57:39 UTC) #4
Tom Sepez
The CQ bit was checked by tsepez@chromium.org
6 years, 4 months ago (2014-07-30 17:00:34 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/tsepez@chromium.org/414223004/1
6 years, 4 months ago (2014-07-30 17:01:40 UTC) #6
commit-bot: I haz the power
6 years, 4 months ago (2014-07-30 17:05:27 UTC) #7
Message was sent while issue was closed.
Change committed as 179240

Powered by Google App Engine
This is Rietveld 408576698