Skip to content
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

Revert "Normalize URL paths: convert /.//p, /..//p, and //p to p (#943)" #999

Merged
merged 3 commits into from
Nov 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ jobs:
strategy:
matrix:
os: [ubuntu-latest, macos-latest, windows-latest]
rust: [1.57.0, 1.67.0, stable, beta, nightly]
rust: [1.63.0, 1.67.0, stable, beta, nightly]
exclude:
- os: macos-latest
rust: 1.67.0
Expand All @@ -37,9 +37,9 @@ jobs:
toolchain: ${{ matrix.rust }}
# Add toolchain for no_std tests
- run: rustup toolchain install nightly
- name: Downgrade idna_adapter on Rust 1.57.0
- name: Downgrade idna_adapter on Rust 1.63.0
if: |
matrix.rust == '1.57.0'
matrix.rust == '1.63.0'
run: cargo update -p idna_adapter --precise 1.1.0
- name: Add `aarch64-unknown-none` toolchain for `no_std` tests
if: |
Expand All @@ -58,7 +58,7 @@ jobs:
- name: Run debugger_visualizer tests
if: |
matrix.os == 'windows-latest' &&
matrix.rust != '1.57.0' &&
matrix.rust != '1.63.0' &&
matrix.rust != '1.67.0'
run: cargo test --test debugger_visualizer --features "url/debugger_visualizer,url_debug_tests/debugger_visualizer" -- --test-threads=1 || echo "debugger test failed"
continue-on-error: true # Fails on GH actions, but not locally.
Expand Down
2 changes: 1 addition & 1 deletion url/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ categories = ["parser-implementations", "web-programming", "encoding", "no-std"]
license = "MIT OR Apache-2.0"
include = ["src/**/*", "LICENSE-*", "README.md", "tests/**"]
edition = "2018"
rust-version = "1.57" # From idna
rust-version = "1.63" # From libc

[dev-dependencies]
serde = { version = "1.0", features = ["derive"] }
Expand Down
54 changes: 1 addition & 53 deletions url/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1757,39 +1757,6 @@ impl Url {
let old_after_path_pos = to_u32(self.serialization.len()).unwrap();
let cannot_be_a_base = self.cannot_be_a_base();
let scheme_type = SchemeType::from(self.scheme());
let mut path_empty = false;

// Check ':' and then see if the next character is '/'
let mut has_host = if let Some(index) = self.serialization.find(":") {
if self.serialization.len() > index + 1
&& self.serialization.as_bytes().get(index + 1) == Some(&b'/')
{
let rest = &self.serialization[(index + ":/".len())..];
let host_part = rest.split('/').next().unwrap_or("");
path_empty = rest.is_empty();
!host_part.is_empty() && !host_part.contains('@')
} else {
false
}
} else {
false
};

// Ensure the path length is greater than 1 to account
// for cases where "/." is already appended from serialization
// If we set path, then we already checked the other two conditions:
// https://url.spec.whatwg.org/#url-serializing
// 1. The host is null
// 2. the first segment of the URL's path is an empty string
if path.len() > 1 {
if let Some(index) = self.serialization.find(":") {
let removal_start = index + ":".len();
if self.serialization[removal_start..].starts_with("/.") {
self.path_start -= "/.".len() as u32;
}
}
}

self.serialization.truncate(self.path_start as usize);
self.mutate(|parser| {
if cannot_be_a_base {
Expand All @@ -1799,33 +1766,14 @@ impl Url {
}
parser.parse_cannot_be_a_base_path(parser::Input::new_no_trim(path));
} else {
let mut has_host = true; // FIXME
parser.parse_path_start(
scheme_type,
&mut has_host,
parser::Input::new_no_trim(path),
);
}
});

// For cases where normalization is applied across both the serialization and the path.
// Append "/." immediately after the scheme (up to ":")
// This is done if three conditions are met.
// https://url.spec.whatwg.org/#url-serializing
// 1. The host is null
// 2. The url's path length is greater than 1
// 3. the first segment of the URL's path is an empty string
if !has_host && path.len() > 1 && path_empty {
if let Some(index) = self.serialization.find(":") {
if self.serialization.len() > index + 2
&& self.serialization.as_bytes().get(index + 1) == Some(&b'/')
&& self.serialization.as_bytes().get(index + 2) == Some(&b'/')
{
self.serialization.insert_str(index + ":".len(), "/.");
self.path_start += "/.".len() as u32;
}
}
}

self.restore_after_path(old_after_path_pos, &after_path);
}

Expand Down
4 changes: 4 additions & 0 deletions url/tests/expected_failures.txt
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,7 @@
<file://monkey/> set pathname to <\\\\>
<file:///unicorn> set pathname to <//\\/>
<file:///unicorn> set pathname to <//monkey/..//>
<non-spec:/> set pathname to </.//p>
<non-spec:/> set pathname to </..//p>
<non-spec:/> set pathname to <//p>
<non-spec:/.//> set pathname to <p>
Loading