-
Notifications
You must be signed in to change notification settings - Fork 84
Fix issue with host-matching and non-ascii hosts #597
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
Conversation
We need to filter out non-ascii hosts before treating them as strings. The note about IPv4/6 addresses can be moved to the top as a result. closes #590
|
@annevk have time to take a look? |
annevk
left a comment
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.
Do we have a test that given input %61 or some such it does not match a host that's a? I.e., that this is far more restrictive than the host parser?
Looks like we don't permit URL encoded characters in the host https://chromium-review.googlesource.com/c/chromium/src/+/4454670 |
We don't allow url encoded hosts but do permit ports with leading 0s. w3c/webappsec-csp#597 w3c/webappsec-csp#596 Bug: 1418009 Change-Id: Ie8ddc509b63e1aa9d35d4e2b989df63483bfca6a
annevk
left a comment
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, this is a lot better.
We don't allow url encoded hosts but do permit ports with leading 0s. w3c/webappsec-csp#597 w3c/webappsec-csp#596 Bug: 1418009 Change-Id: Ie8ddc509b63e1aa9d35d4e2b989df63483bfca6a
We don't allow url encoded hosts but do permit ports with leading 0s. w3c/webappsec-csp#597 w3c/webappsec-csp#596 Bug: 1418009 Change-Id: Ie8ddc509b63e1aa9d35d4e2b989df63483bfca6a Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4454670 Reviewed-by: Dustin Mitchell <[email protected]> Commit-Queue: Ari Chivukula <[email protected]> Cr-Commit-Position: refs/heads/main@{#1134816}
We don't allow url encoded hosts but do permit ports with leading 0s. w3c/webappsec-csp#597 w3c/webappsec-csp#596 Bug: 1418009 Change-Id: Ie8ddc509b63e1aa9d35d4e2b989df63483bfca6a Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4454670 Reviewed-by: Dustin Mitchell <[email protected]> Commit-Queue: Ari Chivukula <[email protected]> Cr-Commit-Position: refs/heads/main@{#1134816}
annevk
left a comment
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.
Would be good if @antosart did the final review.
Co-authored-by: Anne van Kesteren <[email protected]>
Co-authored-by: Anne van Kesteren <[email protected]>
antosart
left a comment
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 fixing this!
SHA: 71c967b Reason: push, by arichiv Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
|
Indeed, thanks @arichiv! |
…stonly Automatic update from web-platform-tests [CSP] WPTs for matching edge cases We don't allow url encoded hosts but do permit ports with leading 0s. w3c/webappsec-csp#597 w3c/webappsec-csp#596 Bug: 1418009 Change-Id: Ie8ddc509b63e1aa9d35d4e2b989df63483bfca6a Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4454670 Reviewed-by: Dustin Mitchell <[email protected]> Commit-Queue: Ari Chivukula <[email protected]> Cr-Commit-Position: refs/heads/main@{#1134816} -- wpt-commits: 4f8df7ab83ab1562214fcca2f67bb1718cc28808 wpt-pr: 39631
…stonly Automatic update from web-platform-tests [CSP] WPTs for matching edge cases We don't allow url encoded hosts but do permit ports with leading 0s. w3c/webappsec-csp#597 w3c/webappsec-csp#596 Bug: 1418009 Change-Id: Ie8ddc509b63e1aa9d35d4e2b989df63483bfca6a Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4454670 Reviewed-by: Dustin Mitchell <[email protected]> Commit-Queue: Ari Chivukula <[email protected]> Cr-Commit-Position: refs/heads/main@{#1134816} -- wpt-commits: 4f8df7ab83ab1562214fcca2f67bb1718cc28808 wpt-pr: 39631
We need to filter out non-ascii hosts before treating them as strings. The note about IPv4/6 addresses can be moved to the top as a result.
closes #590