Skip to content

Commit 8fdac0c

Browse files
weihanglobmwill
authored andcommitted
fix(patch_set): multi-byte char boundary panic
The old byte-level reverse cmp could produce a suffix offset inside a multi-byte UTF-8 character and panic. Rewrite to track `/` positions during the reverse scan and only split at those. (`/` is ASCII so no boundary check needed)
1 parent bd8ee5e commit 8fdac0c

2 files changed

Lines changed: 29 additions & 15 deletions

File tree

‎src/patch_set/parse.rs‎

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -698,30 +698,42 @@ fn longest_common_path_suffix<'a, T: Text + ?Sized>(a: &'a T, b: &T) -> Option<&
698698
return None;
699699
}
700700

701-
let suffix_len = a
701+
let mut last_slash = None;
702+
let mut matched = 0;
703+
704+
for (i, (x, y)) in a
702705
.as_bytes()
703706
.iter()
704707
.rev()
705708
.zip(b.as_bytes().iter().rev())
706-
.take_while(|(x, y)| x == y)
707-
.count();
709+
.enumerate()
710+
{
711+
if x != y {
712+
break;
713+
}
714+
// `/` is ASCII,
715+
// so this index is always a valid split point UTF-8 strings.
716+
// No char boundary check needed.
717+
if *x == b'/' {
718+
last_slash = Some(i + 1);
719+
}
720+
matched = i + 1;
721+
}
708722

709-
if suffix_len == 0 {
723+
if matched == 0 {
710724
return None;
711725
}
712726

713727
// Identical strings
714-
if suffix_len == a.len() && a.len() == b.len() {
728+
if matched == a.len() && a.len() == b.len() {
715729
return Some(a);
716730
}
717731

718-
// Find first '/' in suffix and return path after it
719-
let suffix_start = a.len() - suffix_len;
720-
let (_, suffix) = a.split_at(suffix_start);
721-
suffix
722-
.split_at_exclusive("/")
723-
.map(|(_, path)| path)
724-
.filter(|p| !p.is_empty())
732+
// Return the path after the outermost `/` in the common suffix.
733+
let suffix_len = last_slash?;
734+
let start = a.len() - suffix_len + 1; // skip the '/'
735+
let (_, path) = a.split_at(start);
736+
(!path.is_empty()).then_some(path)
725737
}
726738

727739
/// Extracts the file operation for a binary patch from git headers.

‎src/patch_set/tests.rs‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -482,14 +482,16 @@ mod patchset_gitdiff {
482482
/// Paths sharing trailing UTF-8 continuation bytes
483483
/// but belonging to different codepoints must not panic
484484
#[test]
485-
#[should_panic(expected = "not a char boundary")]
486485
fn multibyte_char_boundary_in_diff_git_path() {
487486
// U+00BF INVERTED QUESTION MARK (¿) = 0xC2 0xBF
488487
// U+00FF LATIN SMALL LETTER Y WITH DIAERESIS (ÿ) = 0xC3 0xBF
489488
// Both share trailing byte 0xBF.
490489
let input = "diff --git a/\u{bf} b/\u{ff}\n";
491-
let _ = PatchSet::parse(input, ParseOptions::gitdiff())
492-
.collect::<Result<Vec<_>, _>>();
490+
let result: Result<Vec<_>, _> = PatchSet::parse(input, ParseOptions::gitdiff()).collect();
491+
assert_eq!(
492+
result.unwrap_err().kind,
493+
PatchSetParseErrorKind::InvalidDiffGitPath
494+
);
493495
}
494496

495497
/// `parse_one` must stop at `diff --git` boundaries so that

0 commit comments

Comments
 (0)