Bug 235260 – "Unrecognized Content-Security-Policy directive 'worker-src'."

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Kate Cheney]

"Unrecognized Content-Security-Policy directive 'worker-src'."

[Kate Cheney]

Created attachment 449238 [details]
WIP patch

[Kate Cheney]

Created attachment 449460 [details]
WIP patch

[Kate Cheney]

Created attachment 449511 [details]
WIP patch

[Patrick Griffis]

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`.

[Patrick Griffis]

And fixed, looks great.

[Kate Cheney]

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.

[Kate Cheney]

Created attachment 449581 [details]
Patch

[Kate Cheney]

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.

[Radar WebKit Bug Importer]

<rdar://problem/87912445>

[Brent Fulgham]

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"]

:-)

[Kate Cheney]

Created attachment 450150 [details]
Patch for landing

[Kate Cheney]

(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"]
> 
> :-)

!!! :) !!

[Patrick Griffis]

Will need a simple rebase over r288678

[EWS]

Tools/Scripts/svn-apply failed to apply attachment 450150 [details] to trunk.
Please resolve the conflicts and upload a new patch.

[Kate Cheney]

Created attachment 450160 [details]
Patch

[EWS]

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].