normalize domains with trailing slashes #477
Conversation
CSP3 more explicitly calls this out: > If path A consists of one character that is equal to the U+002F > SOLIDUS character (/) and path B is empty, return "Matches". A URL like `example.com/foo` will match a connect-src of `example.com`, as well as `example.com/`, so having two connect-srcs listed like this is redundant.
| source_list.map do |source| | ||
| # Normalize domains ending in a single / as without omitting the slash accomplisheg the same. | ||
| # https://www.w3.org/TR/CSP3/#match-paths § 6.6.2.10 Step 2 | ||
| if source.chomp("/").include?("/") |
There was a problem hiding this comment.
One thing this might not be normalizing is a CSP directive like https://www.example.com/ since the protocol includes /.
The quoted spec is about matching on the path-part, not the entire URL that is in the directive.
It would be good to address this, with unit tests as well.
There was a problem hiding this comment.
I've added a test and some code to fix this. The code doesn't feel great, I'd welcome more elegant solutions here!
Co-authored-by: Dusty Greif <[email protected]>
Co-authored-by: Dusty Greif <[email protected]>
|
@keithamus @vcsjones anything I can do to help get this PR ready to ship? |
| uri = URI(source) | ||
| if uri.path == "/" | ||
| next source.chomp("/") | ||
| end | ||
| rescue URI::InvalidURIError |
There was a problem hiding this comment.
@keithamus I know you wrote this code several years ago, but do you remember what edge case we are looking catch here?
It seems like we might be able to get away with something like:
source_list.map do |source|
source.chomp("/")
end
I believe .chomp will either remove the last char if it exists or return the original string unmodified.
There was a problem hiding this comment.
I don’t think that’s sufficient as it shouldn’t always remove the trailing / - only when the entire path is just /. For example “example.com/foo/“ should remain as is, but “example.com/“ should be trimmed so it becomes “example.com”.
The logic could be described as: if the entire contents of the URL after the domain name are “/“ then that is the equivalent of a URL with no path (so the contents after the domain name are “”).
All PRs:
Adding a new header
Generally, adding a new header is always OK.
Adding a new CSP directive
This PR normalises CSP source expressions to exclude trailing slashes from the Domain if there is no other information in the path.
CSP3 more explicitly calls this out in the path match algorithm:
Also a URL like
example.com/foowill match a source expression ofexample.com, as well asexample.com/, so having two source expressions listed like this is redundant.