servo / rust-url

URL parser for Rust
https://docs.rs/url/
Apache License 2.0
1.27k stars 318 forks source link

Only strip spaces from opaque path when both query and fragment are null #842

Closed ohno418 closed 8 months ago

ohno418 commented 1 year ago

In the current behavior, when url.set_query(None) is called to remove the query from a opaque path, spaces are stripped even if a fragment remains.

As the spec 1 says, strip spaces only when both the query and fragment are null. For example:

let mut url = URL::parse("data:space   ?query#hash");
url.set_query(None);
assert_eq!(url.as_str(), "data:space   #hash")

You can find the related test here: https://github.com/web-platform-tests/wpt/pull/37556/files#diff-316fd17e018fdca0e399146c072ea3a95a08fe1ccd3f8fe940d73ca2b9d26c9eR57-R63

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (0e25146) 82.44% compared to head (6ee00ad) 82.44%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #842 +/- ## ======================================= Coverage 82.44% 82.44% ======================================= Files 20 20 Lines 3343 3349 +6 ======================================= + Hits 2756 2761 +5 - Misses 587 588 +1 ``` | [Impacted Files](https://app.codecov.io/gh/servo/rust-url/pull/842?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=servo) | Coverage Δ | | |---|---|---| | [url/src/lib.rs](https://app.codecov.io/gh/servo/rust-url/pull/842?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=servo#diff-dXJsL3NyYy9saWIucnM=) | `76.00% <100.00%> (-0.07%)` | :arrow_down: | ... and [4 files with indirect coverage changes](https://app.codecov.io/gh/servo/rust-url/pull/842/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=servo)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

valenting commented 8 months ago

I think this was fixed in #879