-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Cross-Origin-Resource-Policy tests #11171
Cross-Origin-Resource-Policy tests #11171
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general this looks great, but I found a couple issues. (I annotated them in one file, but they apply to most tests.)
loadScript(remoteBaseURL + "resources/script.py?corp=same&acao=*", ko); | ||
|
||
title = "Cross-origin no-cors script load with a 'Cross-Origin-Resource-Policy: same-site' response header."; | ||
loadScript(remoteBaseURL + "resources/script.py?corp=same-site&acao=*", ko); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since remote host will be 'www1.' + ORIGINAL_HOST
, wouldn't this be ok
as it's same site?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This concern applies more generally it seems.
}, title); | ||
} | ||
|
||
title = "Same-origin script load with a 'Cross-Origin-Resource-Policy: same' response header."; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you change this to pass the title in as an argument? This seems like a rather strange style.
The other thing that'd be nice to test is ensuring the header is properly parsed. E.g., |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for addressing the nits and writing all these. I created two more small PRs that cover some remaining aspects.
Let's wait with landing this until next week so we can land all the various pieces together.
The reason this is timing out is because it changes |
This header makes it easier for sites to block unwanted "no-cors" cross-origin requests. Tests: * web-platform-tests/wpt#11171 * web-platform-tests/wpt#11427 * web-platform-tests/wpt#11428 Follow-up: #760. Fixes #687.
d591b6a
to
eab3b0e
Compare
Rebase+merge button pressed as requested by @annevk on IRC. |
This header makes it easier for sites to block unwanted "no-cors" cross-origin requests. Tests: * web-platform-tests/wpt#11171 * web-platform-tests/wpt#11427 * web-platform-tests/wpt#11428 Follow-up: #760 & #767. Fixes #687.
These are initial tests targeting whatwg/fetch#733
These are based on https://bugs.webkit.org/show_bug.cgi?id=185840.