Skip to content

Commit 2174c8d

Browse files
authored
patch: accept CRLF line endings in the ---/+++ headers (#87)
A carriage return before the newline terminating a `--- `/`+++ ` line was treated as part of the filename. Since a carriage return has to be quoted in a filename, every patch generated on Windows failed to parse with `InvalidCharInUnquotedFilename`. Strip a single trailing carriage return from the filename field. A raw carriage return can never occur inside a filename: unquoted filenames reject it, and quoted ones write it as the `\r` escape. A carriage return anywhere else on the line is still rejected as before. Signed-off-by: Teddy Tennant <teddytennant@icloud.com>
1 parent e5cc468 commit 2174c8d

2 files changed

Lines changed: 100 additions & 1 deletion

File tree

‎src/patch/parse.rs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,11 @@ fn parse_filename<'a, T: Text + ?Sized>(prefix: &str, line: &'a T) -> Result<Cow
181181
let filename = if let Some((filename, _)) = line.split_at_exclusive("\t") {
182182
filename
183183
} else if let Some((filename, _)) = line.split_at_exclusive("\n") {
184-
filename
184+
// Accept CRLF line endings, as produced by tools on Windows.
185+
// A carriage return is never part of the filename itself: it has
186+
// to be quoted when unquoted filenames are used, and is written
187+
// as the `\r` escape inside a quoted one. See `byte_needs_quoting`.
188+
filename.strip_suffix("\r").unwrap_or(filename)
185189
} else {
186190
return Err(ParsePatchErrorKind::FilenameUnterminated.into());
187191
};

‎src/patch/tests.rs‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -638,6 +638,101 @@ fn non_utf8_escaped_filename_returns_error_on_str_parse() {
638638
);
639639
}
640640

641+
// Patches produced on Windows terminate their header lines with CRLF.
642+
// The carriage return belongs to the line ending, not to the filename.
643+
#[test]
644+
fn crlf_patch_header() {
645+
let s = "\
646+
--- original\r
647+
+++ modified\r
648+
@@ -1,2 +1,3 @@\r
649+
Allomancy\r
650+
Feruchemy\r
651+
+Hemalurgy\r
652+
";
653+
let p = parse(s).unwrap();
654+
assert_eq!(p.original(), Some("original"));
655+
assert_eq!(p.modified(), Some("modified"));
656+
assert_eq!(p.hunks().len(), 1);
657+
658+
let b = parse_bytes(s.as_ref()).unwrap();
659+
assert_eq!(b.original(), Some(&b"original"[..]));
660+
assert_eq!(b.modified(), Some(&b"modified"[..]));
661+
662+
// The same patch with LF line endings yields the same filenames.
663+
let lf = "\
664+
--- original
665+
+++ modified
666+
@@ -1,2 +1,3 @@
667+
Allomancy
668+
Feruchemy
669+
+Hemalurgy
670+
";
671+
let lf = parse(lf).unwrap();
672+
assert_eq!(p.original(), lf.original());
673+
assert_eq!(p.modified(), lf.modified());
674+
}
675+
676+
// A timestamp is separated from the filename by a tab, so the carriage
677+
// return of a CRLF ending trails the timestamp rather than the filename.
678+
#[test]
679+
fn crlf_patch_header_with_timestamp() {
680+
let s = "\
681+
--- original\t2022-10-18 22:41:24.000000000 +0000\r
682+
+++ modified\t2022-10-18 22:41:24.000000000 +0000\r
683+
@@ -1,0 +1,1 @@\r
684+
+Oathbringer\r
685+
";
686+
let p = parse(s).unwrap();
687+
assert_eq!(p.original(), Some("original"));
688+
assert_eq!(p.modified(), Some("modified"));
689+
}
690+
691+
// Quoted filenames escape a carriage return as `\r`, so the only raw
692+
// carriage return on the line is the one from the line ending.
693+
#[test]
694+
fn crlf_patch_header_quoted_filename() {
695+
let s = "\
696+
--- \"ori\\rginal\"\r
697+
+++ \"mo\\rdified\"\r
698+
@@ -1,0 +1,1 @@\r
699+
+Oathbringer\r
700+
";
701+
let p = parse(s).unwrap();
702+
assert_eq!(p.original(), Some("ori\rginal"));
703+
assert_eq!(p.modified(), Some("mo\rdified"));
704+
}
705+
706+
// A carriage return that isn't part of the line ending still has to be
707+
// quoted, exactly as before.
708+
#[test]
709+
fn unquoted_carriage_return_in_filename_still_rejected() {
710+
let s = "\
711+
--- ori\rginal
712+
+++ modified
713+
@@ -1,0 +1,1 @@
714+
+Oathbringer
715+
";
716+
assert_eq!(
717+
parse(s).unwrap_err().kind,
718+
ParsePatchErrorKind::InvalidCharInUnquotedFilename,
719+
);
720+
parse_bytes(s.as_ref()).unwrap_err();
721+
722+
// Same, with CRLF line endings.
723+
let s = "\
724+
--- ori\rginal\r
725+
+++ modified\r
726+
@@ -1,0 +1,1 @@\r
727+
+Oathbringer\r
728+
";
729+
assert_eq!(
730+
parse(s).unwrap_err().kind,
731+
ParsePatchErrorKind::InvalidCharInUnquotedFilename,
732+
);
733+
parse_bytes(s.as_ref()).unwrap_err();
734+
}
735+
641736
#[test]
642737
fn hunk_range_overflow() {
643738
let s = format!(

0 commit comments

Comments
 (0)