Skip to content

Commit

Permalink
Fix hashing for windows paths containing a CurDir component
Browse files Browse the repository at this point in the history
* the logic only checked for / but not for \
* verbatim paths shouldn't skip items at all since they don't get normalized
* the extra branches get optimized out on unix since is_sep_byte is a trivial comparison and is_verbatim is always-false
* tests lacked windows coverage for these cases

That lead to equal paths not having equal hashes and to unnecessary collisions.
  • Loading branch information
the8472 committed Feb 6, 2022
1 parent 686663a commit 45082b0
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 9 deletions.
26 changes: 17 additions & 9 deletions library/std/src/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2920,32 +2920,40 @@ impl cmp::PartialEq for Path {
impl Hash for Path {
fn hash<H: Hasher>(&self, h: &mut H) {
let bytes = self.as_u8_slice();
let prefix_len = match parse_prefix(&self.inner) {
let (prefix_len, verbatim) = match parse_prefix(&self.inner) {
Some(prefix) => {
prefix.hash(h);
prefix.len()
(prefix.len(), prefix.is_verbatim())
}
None => 0,
None => (0, false),
};
let bytes = &bytes[prefix_len..];

let mut component_start = 0;
let mut bytes_hashed = 0;

for i in 0..bytes.len() {
if is_sep_byte(bytes[i]) {
let is_sep = if verbatim { is_verbatim_sep(bytes[i]) } else { is_sep_byte(bytes[i]) };
if is_sep {
if i > component_start {
let to_hash = &bytes[component_start..i];
h.write(to_hash);
bytes_hashed += to_hash.len();
}

// skip over separator and optionally a following CurDir item
// since components() would normalize these away
component_start = i + match bytes[i..] {
[_, b'.', b'/', ..] | [_, b'.'] => 2,
_ => 1,
};
// since components() would normalize these away.
component_start = i + 1;

let tail = &bytes[component_start..];

if !verbatim {
component_start += match tail {
[b'.'] => 1,
[b'.', sep @ _, ..] if is_sep_byte(*sep) => 1,
_ => 0,
};
}
}
}

Expand Down
35 changes: 35 additions & 0 deletions library/std/src/path/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1498,6 +1498,20 @@ pub fn test_compare() {
relative_from: Some("")
);

tc!("foo/.", "foo",
eq: true,
starts_with: true,
ends_with: true,
relative_from: Some("")
);

tc!("foo/./bar", "foo/bar",
eq: true,
starts_with: true,
ends_with: true,
relative_from: Some("")
);

tc!("foo/bar", "foo",
eq: false,
starts_with: true,
Expand Down Expand Up @@ -1541,6 +1555,27 @@ pub fn test_compare() {
ends_with: true,
relative_from: Some("")
);

tc!(r"C:\foo\.\bar.txt", r"C:\foo\bar.txt",
eq: true,
starts_with: true,
ends_with: true,
relative_from: Some("")
);

tc!(r"C:\foo\.", r"C:\foo",
eq: true,
starts_with: true,
ends_with: true,
relative_from: Some("")
);

tc!(r"\\?\C:\foo\.\bar.txt", r"\\?\C:\foo\bar.txt",
eq: false,
starts_with: false,
ends_with: false,
relative_from: None
);
}
}

Expand Down

0 comments on commit 45082b0

Please sign in to comment.