|
|
Chromium Code Reviews|
Created:
3 years, 10 months ago by arthursonzogni Modified:
3 years, 10 months ago Reviewers:
Mike West CC:
blink-reviews, chromium-reviews, clamy Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionContent-Security-Policy: Add test with 'filesystem' and 'blob'.
A few tests that show how Content-Security-Policy works with blob-urls
and filesystem-urls, especially when the inner url is used.
BUG=692046
Review-Url: https://codereview.chromium.org/2691063003
Cr-Commit-Position: refs/heads/master@{#450350}
Committed: https://chromium.googlesource.com/chromium/src/+/27477347836bad10c666f3918b6d77bc742f4785
Patch Set 1 : Content-Security-Policy: Add test with 'filesystem' 'blob'. #
Total comments: 4
Patch Set 2 : Add TODO and BUG id. #Messages
Total messages: 15 (8 generated)
Patchset #1 (id:1) has been deleted
arthursonzogni@chromium.org changed reviewers: + mkwst@chromium.org
Hi Mike, Please take a look, thanks! https://codereview.chromium.org/2691063003/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/frame/csp/SourceListDirectiveTest.cpp (right): https://codereview.chromium.org/2691063003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/frame/csp/SourceListDirectiveTest.cpp:196: KURL(base, "blob:https://not-example.test/1be95204-93d6-4GUID"))); Registering a new scheme as bypassing CSP can make some URL not allowed anymore. Can you give me the confirmation that it is the expected behavior? https://codereview.chromium.org/2691063003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/frame/csp/SourceListDirectiveTest.cpp:220: KURL(base, "filesystem:https://not-example.test/file.txt"))); Same case here with filesystem URL.
Hi Mike, Please take a look, thanks!
Do these tests pass today or fail today? The behavior they're documenting doesn't seem right. https://codereview.chromium.org/2691063003/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/frame/csp/SourceListDirectiveTest.cpp (right): https://codereview.chromium.org/2691063003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/frame/csp/SourceListDirectiveTest.cpp:196: KURL(base, "blob:https://not-example.test/1be95204-93d6-4GUID"))); On 2017/02/14 at 10:30:20, arthursonzogni wrote: > Registering a new scheme as bypassing CSP can make some URL not allowed anymore. > Can you give me the confirmation that it is the expected behavior? This seems very strange. It's certainly not what I'd expect. https://codereview.chromium.org/2691063003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/frame/csp/SourceListDirectiveTest.cpp:220: KURL(base, "filesystem:https://not-example.test/file.txt"))); On 2017/02/14 at 10:30:20, arthursonzogni wrote: > Same case here with filesystem URL. Same.
On 2017/02/14 12:22:19, Mike West (sloooooow) wrote: > Do these tests pass today or fail today? The behavior they're documenting > doesn't seem right. > > https://codereview.chromium.org/2691063003/diff/20001/third_party/WebKit/Sour... > File third_party/WebKit/Source/core/frame/csp/SourceListDirectiveTest.cpp > (right): > > https://codereview.chromium.org/2691063003/diff/20001/third_party/WebKit/Sour... > third_party/WebKit/Source/core/frame/csp/SourceListDirectiveTest.cpp:196: > KURL(base, "blob:https://not-example.test/1be95204-93d6-4GUID"))); > On 2017/02/14 at 10:30:20, arthursonzogni wrote: > > Registering a new scheme as bypassing CSP can make some URL not allowed > anymore. > > Can you give me the confirmation that it is the expected behavior? > > This seems very strange. It's certainly not what I'd expect. > > https://codereview.chromium.org/2691063003/diff/20001/third_party/WebKit/Sour... > third_party/WebKit/Source/core/frame/csp/SourceListDirectiveTest.cpp:220: > KURL(base, "filesystem:https://not-example.test/file.txt"))); > On 2017/02/14 at 10:30:20, arthursonzogni wrote: > > Same case here with filesystem URL. > > Same. Yes, these tests pass today. It passes on my implementation on the browser-side too, since I followed this implementation. I will fill a bug and add a comment near these two lines with the bug ID.
Documenting current behavior LGTM, but please do file a bug, because the current behavior seems wrong.
Description was changed from ========== Content-Security-Policy: Add test with 'filesystem' and 'blob'. A few tests that show how Content-Security-Policy works with blob-urls and filesystem-urls, especially when the inner url is used. BUG=None. ========== to ========== Content-Security-Policy: Add test with 'filesystem' and 'blob'. A few tests that show how Content-Security-Policy works with blob-urls and filesystem-urls, especially when the inner url is used. BUG=692046 ==========
Patchset #2 (id:40001) has been deleted
The CQ bit was checked by arthursonzogni@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from mkwst@chromium.org Link to the patchset: https://codereview.chromium.org/2691063003/#ps60001 (title: "Add TODO and BUG id.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 60001, "attempt_start_ts": 1487078796829620,
"parent_rev": "d3906b062889017cc44f27156d2318a6db9f0e7c", "commit_rev":
"27477347836bad10c666f3918b6d77bc742f4785"}
Message was sent while issue was closed.
Description was changed from ========== Content-Security-Policy: Add test with 'filesystem' and 'blob'. A few tests that show how Content-Security-Policy works with blob-urls and filesystem-urls, especially when the inner url is used. BUG=692046 ========== to ========== Content-Security-Policy: Add test with 'filesystem' and 'blob'. A few tests that show how Content-Security-Policy works with blob-urls and filesystem-urls, especially when the inner url is used. BUG=692046 Review-Url: https://codereview.chromium.org/2691063003 Cr-Commit-Position: refs/heads/master@{#450350} Committed: https://chromium.googlesource.com/chromium/src/+/27477347836bad10c666f3918b6d... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:60001) as https://chromium.googlesource.com/chromium/src/+/27477347836bad10c666f3918b6d... |
