-
Notifications
You must be signed in to change notification settings - Fork 859
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
Relax path validation (#2355) #2356
Conversation
@@ -1247,4 +1247,23 @@ mod tests { | |||
0 | |||
); | |||
} | |||
|
|||
#[tokio::test] | |||
async fn filesystem_filename_with_percent() { |
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 is @jccampagne 's test case from #2351
4f5629d
to
2911464
Compare
let decoded: Cow<'a, [u8]> = percent_decode(segment.as_bytes()).into(); | ||
let part = PathPart::from(decoded.as_ref()); | ||
if segment != part.as_ref() { | ||
if segment == "." || segment == ".." { |
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.
Could some percent_validate
be upstreamed to percent_encoding
?
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.
It would certainly be helpful if AsciiSet had a public interface to check a byte
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.
Filed a ticket servo/rust-url#784
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.
Upstreaming would be nice, otherwise this looks good.
Benchmark runs are scheduled for baseline = ce2bd1e and contender = 0c828a9. 0c828a9 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Which issue does this PR close?
Closes #2355
Closes #2349
Rationale for this change
See tickets
What changes are included in this PR?
This relaxes the validation in
PathPart::parse
to be less conservative about what constitutes a safe path. In particular it should accept any "safe" path, even if it would not generate such a path itself.Are there any user-facing changes?
No