| Summary: | "Unrecognized Content-Security-Policy directive 'worker-src'." | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Kate Cheney <katherine_cheney> | ||||||||||||||
| Component: | New Bugs | Assignee: | Kate Cheney <katherine_cheney> | ||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||
| Severity: | Normal | CC: | bfulgham, cdumez, eric.carlson, ews-watchlist, glenn, japhet, jer.noble, mkwst, pgriffis, philipj, sergio, webkit-bug-importer | ||||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||||
| Version: | WebKit Nightly Build | ||||||||||||||||
| Hardware: | Unspecified | ||||||||||||||||
| OS: | Unspecified | ||||||||||||||||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=229875 | ||||||||||||||||
| Attachments: |
|
||||||||||||||||
|
Description
Kate Cheney
2022-01-14 17:56:51 PST
Created attachment 449238 [details]
WIP patch
Created attachment 449460 [details]
WIP patch
Created attachment 449511 [details]
WIP patch
Overall patch looks really good. Was one regression: script-src-strict_dynamic_worker-importScripts.https.html Looks like when it falls back to the `script-src` directive it expects all of its rules to pass including 'strict-dynamic'. I wonder if we match that behavior with all other directives that can fallback to `script-src`. And fixed, looks great. Yes! I found https://github.com/w3c/webappsec-csp/issues/200 which pretty much outlines exactly what you suggested. Hopefully that fixes all the regressions. Created attachment 449581 [details]
Patch
Something I could probably clarify in the Changelog is that some worker-src tests are still skipped because they log seemingly random URL paths that change with each test run, making them impossible to write expectations for. Comment on attachment 449581 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=449581&action=review Awesome work, Kate! r=me > Source/WebCore/page/csp/ContentSecurityPolicy.cpp:619 > + String consoleMessage = consoleMessageForViolation(ContentSecurityPolicyDirectiveNames::workerSrc, violatedDirective, blockedURL, "Refused to load"); Nit: consoleMessage could be auto. > Source/WebCore/page/csp/ContentSecurityPolicyDirectiveList.cpp:155 > +{ I found this ChangeLog comment to be helpful. I wonder if it might be useful to reference here (perhaps with a spec reference): // worker-src defers to child-src, then script-src, then default-src. > LayoutTests/imported/w3c/ChangeLog:90 > + * web-platform-tests/content-security-policy/gen/top.meta/worker-src-self/worker-import-data.https-expected.txt: So exciting to see! :-) > LayoutTests/TestExpectations:-489 > -imported/w3c/web-platform-tests/content-security-policy/gen/top.http-rp/script-src-self/worker-import-data.https.html [ Skip ] Yes!!! > LayoutTests/imported/w3c/web-platform-tests/content-security-policy/blob/self-doesnt-match-blob.sub-expected.txt:4 > +PASS Expecting logs: ["violated-directive=worker-src","TEST COMPLETE"] :-) Created attachment 450150 [details]
Patch for landing
(In reply to Brent Fulgham from comment #10) > Comment on attachment 449581 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=449581&action=review > > Awesome work, Kate! r=me > thanks for the review! > > Source/WebCore/page/csp/ContentSecurityPolicy.cpp:619 > > + String consoleMessage = consoleMessageForViolation(ContentSecurityPolicyDirectiveNames::workerSrc, violatedDirective, blockedURL, "Refused to load"); > > Nit: consoleMessage could be auto. > Fixed! > > Source/WebCore/page/csp/ContentSecurityPolicyDirectiveList.cpp:155 > > +{ > > I found this ChangeLog comment to be helpful. I wonder if it might be useful > to reference here (perhaps with a spec reference): > > // worker-src defers to child-src, then script-src, then default-src. > Ditto, added this in PFL. > > LayoutTests/imported/w3c/ChangeLog:90 > > + * web-platform-tests/content-security-policy/gen/top.meta/worker-src-self/worker-import-data.https-expected.txt: > > So exciting to see! :-) > > > LayoutTests/TestExpectations:-489 > > -imported/w3c/web-platform-tests/content-security-policy/gen/top.http-rp/script-src-self/worker-import-data.https.html [ Skip ] > > Yes!!! > > > LayoutTests/imported/w3c/web-platform-tests/content-security-policy/blob/self-doesnt-match-blob.sub-expected.txt:4 > > +PASS Expecting logs: ["violated-directive=worker-src","TEST COMPLETE"] > > :-) !!! :) !! Will need a simple rebase over r288678 Tools/Scripts/svn-apply failed to apply attachment 450150 [details] to trunk.
Please resolve the conflicts and upload a new patch.
Created attachment 450160 [details]
Patch
Committed r288701 (246499@main): <https://commits.webkit.org/246499@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 450160 [details]. |