Skip to content

Fix effective directive for inline checks #3

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 15, 2025

Conversation

TimvdLippe
Copy link

A directive can imply other directives. For example, the script-src implies the script-src-elem directive. Rather than returning the script-src as violated directive, we need to return script-src-elem.

@TimvdLippe
Copy link
Author

@jdm do you mind reviewing this one so I can start a WPT try run on servo/servo#36510 ?

@jdm
Copy link
Member

jdm commented Apr 15, 2025

Oops, I assumed this was in the upstream repository!

@jdm
Copy link
Member

jdm commented Apr 15, 2025

Yikes, some kind of dependency issue blocking CI :(

@TimvdLippe
Copy link
Author

Um, how's that related to this change? Upstream CI is green. I fear I don't know enough Rust yet to figure out what's going on here.

@TimvdLippe
Copy link
Author

Wait, I think #1 already introduced this issue. Will take a look.

A directive can imply other directives. For example, the `script-src`
implies the `script-src-elem` directive. Rather than returning the
`script-src` as violated directive, we need to return `script-src-elem`.
@TimvdLippe TimvdLippe force-pushed the pr-timvdlippe-fix-effective-directive branch from b9d8bd5 to 013c596 Compare April 15, 2025 16:44
@TimvdLippe
Copy link
Author

  1. I branched from the wrong branch
  2. I then was able to locally reproduce the failure from CI
  3. I think I fixed it by correctly setting the feature, based on [Question] How to deserialize Vec<Url> rust-url#420 (comment)

@TimvdLippe TimvdLippe force-pushed the pr-timvdlippe-fix-effective-directive branch from 013c596 to ccd42e2 Compare April 15, 2025 16:47
@jdm jdm merged commit be68d50 into servo:servo-csp Apr 15, 2025
7 checks passed
github-merge-queue bot pushed a commit to servo/servo that referenced this pull request Apr 17, 2025
This also ensures that document now reports all violations and we set
the correct directive.

With these changes, all `script-src-attr-elem` WPT tests pass.

Part of #36437 

Requires servo/rust-content-security-policy#3 to land first

Signed-off-by: Tim van der Lippe <[email protected]>
github-merge-queue bot pushed a commit to servo/servo that referenced this pull request Apr 17, 2025
This also ensures that document now reports all violations and we set
the correct directive.

With these changes, all `script-src-attr-elem` WPT tests pass.

Part of #36437 

Requires servo/rust-content-security-policy#3 to land first

Signed-off-by: Tim van der Lippe <[email protected]>
github-merge-queue bot pushed a commit to servo/servo that referenced this pull request Apr 17, 2025
This also ensures that document now reports all violations and we set
the correct directive.

With these changes, all `script-src-attr-elem` WPT tests pass.

Part of #36437 

Requires servo/rust-content-security-policy#3 to land first

Signed-off-by: Tim van der Lippe <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants