From 5810c1b13ea9be29cc05dd042d53a332c3ca3919 Mon Sep 17 00:00:00 2001 From: Dylan Hurd Date: Tue, 10 Feb 2026 22:50:43 -0800 Subject: [PATCH] fix(apply-patch) Handle CRLF gracefully --- codex-rs/apply-patch/src/parser.rs | 54 ++++++++++++++----- codex-rs/apply-patch/tests/suite/scenarios.rs | 35 ++++++++++++ 2 files changed, 76 insertions(+), 13 deletions(-) diff --git a/codex-rs/apply-patch/src/parser.rs b/codex-rs/apply-patch/src/parser.rs index 8785b38519..cf8ff8dab8 100644 --- a/codex-rs/apply-patch/src/parser.rs +++ b/codex-rs/apply-patch/src/parser.rs @@ -152,7 +152,7 @@ enum ParseMode { } fn parse_patch_text(patch: &str, mode: ParseMode) -> Result { - let lines: Vec<&str> = patch.trim().lines().collect(); + let lines: Vec<&str> = patch.trim().split('\n').collect(); let lines: &[&str] = match check_patch_boundaries_strict(&lines) { Ok(()) => &lines, Err(e) => match mode { @@ -206,7 +206,9 @@ fn check_patch_boundaries_lenient<'a>( ) -> Result<&'a [&'a str], ParseError> { match original_lines { [first, .., last] => { - if (first == &"<= 4 { @@ -282,9 +284,11 @@ fn parse_one_hunk(lines: &[&str], line_number: usize) -> Result<(Hunk, usize), P let mut parsed_lines = 1; // Optional: move file line - let move_path = remaining_lines - .first() - .and_then(|x| x.strip_prefix(MOVE_TO_MARKER)); + let move_path = remaining_lines.first().and_then(|x| { + x.strip_suffix('\r') + .unwrap_or(x) + .strip_prefix(MOVE_TO_MARKER) + }); if move_path.is_some() { remaining_lines = &remaining_lines[1..]; @@ -353,9 +357,10 @@ fn parse_update_file_chunk( } // If we see an explicit context marker @@ or @@ , consume it; otherwise, optionally // allow treating the chunk as starting directly with diff lines. - let (change_context, start_index) = if lines[0] == EMPTY_CHANGE_CONTEXT_MARKER { + let first_line = lines[0].strip_suffix('\r').unwrap_or(lines[0]); + let (change_context, start_index) = if first_line == EMPTY_CHANGE_CONTEXT_MARKER { (None, 1) - } else if let Some(context) = lines[0].strip_prefix(CHANGE_CONTEXT_MARKER) { + } else if let Some(context) = first_line.strip_prefix(CHANGE_CONTEXT_MARKER) { (Some(context.to_string()), 1) } else { if !allow_missing_context { @@ -383,7 +388,8 @@ fn parse_update_file_chunk( }; let mut parsed_lines = 0; for line in &lines[start_index..] { - match *line { + let line_contents = line.strip_suffix('\r').unwrap_or(line); + match line_contents { EOF_MARKER => { if parsed_lines == 0 { return Err(InvalidHunkError { @@ -395,7 +401,7 @@ fn parse_update_file_chunk( parsed_lines += 1; break; } - line_contents => { + _ => { match line_contents.chars().next() { None => { // Interpret this as an empty line. @@ -403,14 +409,14 @@ fn parse_update_file_chunk( chunk.new_lines.push(String::new()); } Some(' ') => { - chunk.old_lines.push(line_contents[1..].to_string()); - chunk.new_lines.push(line_contents[1..].to_string()); + chunk.old_lines.push(line[1..].to_string()); + chunk.new_lines.push(line[1..].to_string()); } Some('+') => { - chunk.new_lines.push(line_contents[1..].to_string()); + chunk.new_lines.push(line[1..].to_string()); } Some('-') => { - chunk.old_lines.push(line_contents[1..].to_string()); + chunk.old_lines.push(line[1..].to_string()); } _ => { if parsed_lines == 0 { @@ -669,6 +675,28 @@ fn test_parse_patch_lenient() { ); } +#[test] +fn test_parse_patch_preserves_crlf() { + let patch_text = "*** Begin Patch\r\n*** Update File: sample.txt\r\n@@\r\n-one\r\n+uno\r\n two\r\n*** End Patch\r\n"; + assert_eq!( + parse_patch_text(patch_text, ParseMode::Strict), + Ok(ApplyPatchArgs { + hunks: vec![UpdateFile { + path: PathBuf::from("sample.txt"), + move_path: None, + chunks: vec![UpdateFileChunk { + change_context: None, + old_lines: vec!["one\r".to_string(), "two\r".to_string()], + new_lines: vec!["uno\r".to_string(), "two\r".to_string()], + is_end_of_file: false, + }], + }], + patch: patch_text.trim().to_string(), + workdir: None, + }) + ); +} + #[test] fn test_parse_one_hunk() { assert_eq!( diff --git a/codex-rs/apply-patch/tests/suite/scenarios.rs b/codex-rs/apply-patch/tests/suite/scenarios.rs index 3beb7d652d..5f8285b931 100644 --- a/codex-rs/apply-patch/tests/suite/scenarios.rs +++ b/codex-rs/apply-patch/tests/suite/scenarios.rs @@ -124,3 +124,38 @@ fn copy_dir_recursive(src: &Path, dst: &Path) -> anyhow::Result<()> { } Ok(()) } + +#[test] +fn test_apply_patch_preserves_crlf_for_update_hunks() -> anyhow::Result<()> { + let tmp = tempdir()?; + let file_path = tmp.path().join("sample.txt"); + fs::write(&file_path, b"one\r\ntwo\r\n")?; + + let patch = "*** Begin Patch\r\n*** Update File: sample.txt\r\n@@\r\n-one\r\n+uno\r\n two\r\n*** End Patch\r\n"; + + Command::new(codex_utils_cargo_bin::cargo_bin("apply_patch")?) + .arg(patch) + .current_dir(tmp.path()) + .output()?; + + assert_eq!(fs::read(&file_path)?, b"uno\r\ntwo\r\n"); + Ok(()) +} + +#[test] +fn test_apply_patch_preserves_crlf_for_add_file_hunks() -> anyhow::Result<()> { + let tmp = tempdir()?; + let patch = + "*** Begin Patch\r\n*** Add File: created.txt\r\n+hello\r\n+world\r\n*** End Patch\r\n"; + + Command::new(codex_utils_cargo_bin::cargo_bin("apply_patch")?) + .arg(patch) + .current_dir(tmp.path()) + .output()?; + + assert_eq!( + fs::read(tmp.path().join("created.txt"))?, + b"hello\r\nworld\r\n" + ); + Ok(()) +}