diff --git a/crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs b/crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs index 8500a5afc..d2b2fd4a0 100644 --- a/crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs +++ b/crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs @@ -4769,3 +4769,93 @@ async fn gem_vendored_unwind_names_a_patched_archive_in_vendor_cache() { vendored_unwind_names_the_patched_cached_archive(&fx, command); } } + +/// #1831: a hosted redirect over a CRLF Gemfile (a Windows checkout, or +/// `core.autocrlf=true`) keeps it CRLF throughout, for a plain top-level +/// declaration and for one inside the user's own `source … do` block; a +/// real bundler converges the result, a re-scan is a no-op, and `rollback` +/// hands back a CRLF Gemfile that still declares the gem. +#[tokio::test(flavor = "multi_thread")] +#[ignore = "host capstone: shells out to a real ruby/gem/bundler (>= 1.17); \ + the unpinned `test` job skips it, an e2e job with a pinned toolchain runs it via --ignored"] +async fn gem_hosted_redirect_keeps_a_crlf_gemfile_crlf() { + fn all_crlf(text: &str) -> bool { + let crlf = text.matches("\r\n").count(); + crlf > 0 && crlf == text.matches('\n').count() && crlf == text.matches('\r').count() + } + for (tag, driver) in [ + ("crlf-gemfile", Driver::ScanVex), + ("crlf-gemfile-source-block", Driver::ScanVexSourceBlock), + ] { + let Some(fx) = + redirect_scanned_project(tag, Spelling::Gemfile, false, true, None, driver).await + else { + return; + }; + let api = fx._server.uri(); + let gemfile_path = fx.proj.join(fx.gemfile_name); + let lock_path = fx.proj.join(fx.lock_name); + // Start over from the pristine pair, its Gemfile in CRLF. + let pristine = String::from_utf8(fx.pristine_gemfile.clone()).unwrap(); + let crlf = pristine.replace("\r\n", "\n").replace('\n', "\r\n"); + let _ = std::fs::remove_dir_all(fx.proj.join(".socket")); + std::fs::write(&gemfile_path, &crlf).unwrap(); + std::fs::write(&lock_path, &fx.pristine_lock).unwrap(); + + let (code, out, err) = run_hosted_scan(&fx.proj, &api); + assert_eq!(code, 0, "{tag}: hosted scan: {out}\n{err}"); + let gemfile = std::fs::read_to_string(&gemfile_path).unwrap(); + assert!( + gemfile.contains(&fx.index_url), + "{tag}: the Gemfile is redirected: {gemfile:?}" + ); + assert!( + all_crlf(&gemfile), + "{tag}: the redirected Gemfile mixes line endings: {gemfile:?}" + ); + + let install = bundle(&fx.proj, &["install"]); + assert!( + install.status.success(), + "{tag}: converging install of the CRLF Gemfile:\n{}", + String::from_utf8_lossy(&install.stderr) + ); + let (code, out, err) = run_hosted_scan(&fx.proj, &api); + assert_eq!(code, 0, "{tag}: re-scan: {out}\n{err}"); + assert_eq!( + std::fs::read_to_string(&gemfile_path).unwrap(), + gemfile, + "{tag}: a re-scan leaves the Gemfile byte-identical" + ); + + let upstream = format!("{api}/upstream"); + let cwd = fx.proj.to_str().expect("utf8 tmp path"); + let (code, stdout, stderr) = run_socket_env( + &fx.proj, + &[ + "rollback", + PURL, + "--json", + "--cwd", + cwd, + "--api-url", + &api, + "--org", + ORG, + "--api-token", + "fake", + "--patch-server-url", + &api, + ], + &[("SOCKET_RUBYGEMS_URL", &upstream)], + ); + assert_eq!(code, 0, "{tag}: rollback: {stdout}\nstderr:\n{stderr}"); + let restored = std::fs::read_to_string(&gemfile_path).unwrap(); + assert!( + !restored.contains("patch-registry") + && restored.contains(&format!("gem \"{DEP}\"")) + && all_crlf(&restored), + "{tag}: rollback restores a CRLF Gemfile declaring the gem: {restored:?}" + ); + } +} diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index ccb8bb226..f209cd7a1 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -25,6 +25,7 @@ use serde_json::{json, Value}; use crate::formats::yarn::berry_gates::{self, Yarnrc}; use crate::utils::digest::is_hex64_lower; +use crate::utils::line_endings; #[cfg(test)] use crate::utils::line_endings::LineEndings; use crate::vendor::common::{parse_json_text, JsonLayout}; @@ -7649,7 +7650,8 @@ pub(crate) fn gem_index_url_pattern(dep: &DepOverride, index_url: &str) -> Strin /// divergence the rewriter itself created. Trailing whitespace is trimmed (a /// block appended to a newline-less file adds a final newline the other /// spelling never had). `\r?` mirrors the block recognizer in `rewrite_gem`: -/// a `core.autocrlf` checkout rewrites run 1's LF block to CRLF, and a block +/// a CRLF manifest gets a CRLF block (#1831) and a `core.autocrlf` checkout +/// rewrites run 1's LF block to CRLF, and a block /// the recognizer accepts must also be erased here or the re-run is trapped /// behind the divergence warning before it can reach the recognizer. fn gem_spelling_residue(content: &str, deps: &[&DepOverride]) -> String { @@ -7950,10 +7952,11 @@ fn rewrite_gem( // Wildcard the rotating segments instead (mirrors the CHECKSUMS // at-target guard below). let url_pat = gem_index_url_pattern(dep, &ov.index_url); - // `\r?\n`: the rewriter emits LF, but a `core.autocrlf` checkout - // rewrites the working tree to CRLF — the guard must still - // recognize the block there, or the indented `gem` line inside - // it falls through to `gem_line_re` and gets wrapped again. + // `\r?\n`: the rewriter writes a CRLF Gemfile's block in CRLF + // (#1831), and a `core.autocrlf` checkout rewrites an LF one to + // CRLF — the guard must recognize the block either way, or the + // indented `gem` line inside it falls through to `gem_line_re` + // and gets wrapped again. let block_re = Regex::new( &(String::from(r#"(?m)^source "("#) + &url_pat @@ -8004,8 +8007,24 @@ fn rewrite_gem( let raw_tail = m .get(2) .expect("gem_line_re always captures group 2 (tail)") - .as_str() - .to_string(); + .as_str(); + // The CR of a CRLF line is its ending, not part of the + // declaration: the block's own lines take that ending + // and the CR goes back at its end (#1831). An + // unterminated last line (a lone trailing CR included) + // takes the file's. + let terminated = range.end < gf.len(); + let cr = if terminated && raw_tail.ends_with('\r') { + "\r" + } else { + "" + }; + let raw_tail = raw_tail.strip_suffix('\r').unwrap_or(raw_tail).to_string(); + let nl = match (cr, terminated) { + ("\r", _) => "\r\n", + (_, true) => "\n", + _ => line_endings::terminator(gf), + }; // A parenthesized call keeps its closing `)` in the tail: // strip it (dropping any comment with it), or fail closed // when it is absent (the call continues past this line). @@ -8075,12 +8094,12 @@ fn rewrite_gem( let opts = gemfile::trailing_options(&tail); let block = if opts.is_empty() { format!( - "source \"{}\" do\n gem \"{}\", \"{}\"\nend", + "source \"{}\" do{nl} gem \"{}\", \"{}\"{nl}end", ov.index_url, dep.name, dep.version ) } else { format!( - "source \"{}\" do\n gem \"{}\", \"{}\", {opts}\nend", + "source \"{}\" do{nl} gem \"{}\", \"{}\", {opts}{nl}end", ov.index_url, dep.name, dep.version ) }; @@ -8089,6 +8108,7 @@ fn rewrite_gem( // duplicate earlier in the file and corrupt it (and the // block may carry user text a regex replacement would // `$`-expand). + let block = format!("{block}{cr}"); gf.replace_range(range, &block); gemfile_changed = true; result.edits.push(FileEdit { @@ -8131,9 +8151,11 @@ fn rewrite_gem( }); continue; } else { - // Genuinely undeclared (a transitive dep): append a block. + // Genuinely undeclared (a transitive dep): append a block + // in the file's line endings (#1831). + let eol = line_endings::terminator(gf); let block = format!( - "source \"{}\" do\n gem \"{}\", \"{}\"\nend", + "source \"{}\" do{eol} gem \"{}\", \"{}\"{eol}end", ov.index_url, dep.name, dep.version ); // Always one blank line before the block: the in-place @@ -8141,13 +8163,12 @@ fn rewrite_gem( // declaration, so this is what lets `rollback` / // `remove` prove the block is this append and drop it // instead of restoring a direct exact pin (#457). - let eol = if gf.contains("\r\n") { "\r\n" } else { "\n" }; let sep = if gf.is_empty() || gf.ends_with('\n') { eol.to_string() } else { eol.repeat(2) }; - *gf = format!("{gf}{sep}{block}\n"); + *gf = format!("{gf}{sep}{block}{eol}"); gemfile_changed = true; result.edits.push(FileEdit { path: gemfile_name.into(), @@ -18019,6 +18040,124 @@ mod tests { ); } + /// #1831: the Socket source block takes the CRLF endings of the Gemfile + /// it is written into, for a rewritten direct declaration (plain, with + /// options, parenthesized, inside a user `source … do` block, on an + /// unterminated last line) and for the block appended for a transitive + /// gem. The re-run over the result is a no-op. + #[test] + fn gemfile_source_block_keeps_crlf_line_endings() { + let direct = gem_lock_resolving(&[("rails", "7.0.0", &[])], &["rails"]); + let transitive = gem_lock_resolving( + &[ + ("puma", "6.0.0", &["rails (>= 0)"]), + ("rails", "7.0.0", &[]), + ], + &["puma"], + ); + let head = "source \"https://rubygems.org\"\r\n\r\n"; + let block = "source \"https://patch.test/gem/tok/uuid/\" do\r\n gem \"rails\", \"7.0.0\""; + for (gemfile, lock, want) in [ + ( + format!("{head}gem \"rails\"\r\n"), + &direct, + format!("{block}\r\nend\r\n"), + ), + ( + format!("{head}gem \"rails\", require: false # boot\r\n"), + &direct, + format!("{block}, require: false # boot\r\nend\r\n"), + ), + ( + format!("{head}gem(\"rails\")\r\n"), + &direct, + format!("{block}\r\nend\r\n"), + ), + ( + format!("{head}source \"https://gems.example\" do\r\n gem \"rails\"\r\nend\r\n"), + &direct, + format!("do\r\n{block}\r\nend\r\nend\r\n"), + ), + ( + format!("{head}gem \"rails\""), + &direct, + format!("{block}\r\nend"), + ), + // A lone CR at the end of the file is not a line ending. + ( + format!("{head}gem \"rails\"\r"), + &direct, + format!("{block}\r\nend"), + ), + ( + format!("{head}gem \"puma\"\r\n"), + &transitive, + format!("gem \"puma\"\r\n\r\n{block}\r\nend\r\n"), + ), + ( + format!("{head}gem \"puma\""), + &transitive, + format!("gem \"puma\"\r\n\r\n{block}\r\nend\r\n"), + ), + ] { + let mut files = BTreeMap::new(); + files.insert("Gemfile".to_string(), gemfile.clone()); + files.insert("Gemfile.lock".to_string(), lock.to_string()); + let res = rewrite_registry_redirect(&files, &[gem_override("rails", "7.0.0")]); + let out = res + .files + .get("Gemfile") + .unwrap_or_else(|| panic!("{gemfile:?} is redirected: {:?}", res.warnings)); + assert_eq!( + LineEndings::of(out), + LineEndings::Crlf, + "{gemfile:?}: mixed line endings: {out:?}" + ); + assert!(out.ends_with(&want), "{gemfile:?}: {out:?}"); + + files.insert("Gemfile".to_string(), out.clone()); + files.insert( + "Gemfile.lock".to_string(), + res.files + .get("Gemfile.lock") + .cloned() + .unwrap_or_else(|| lock.to_string()), + ); + let again = rewrite_registry_redirect(&files, &[gem_override("rails", "7.0.0")]); + assert!( + !again.files.contains_key("Gemfile"), + "{gemfile:?}: re-run rewrote the Gemfile: {:?}", + again.files.get("Gemfile") + ); + } + } + + /// #1831: an LF Gemfile still gets an LF block, and a line ending in LF + /// inside a mostly-CRLF file keeps its own LF (the block takes the + /// ending of the line it replaces). + #[test] + fn gemfile_source_block_keeps_lf_line_endings() { + let direct = gem_lock_resolving(&[("rails", "7.0.0", &[])], &["rails"]); + for (gemfile, want) in [ + ( + "source \"https://rubygems.org\"\n\ngem \"rails\"\n", + "source \"https://patch.test/gem/tok/uuid/\" do\n gem \"rails\", \"7.0.0\"\nend\n", + ), + ( + "source \"https://rubygems.org\"\r\n\r\ngem \"rails\"\n", + "source \"https://patch.test/gem/tok/uuid/\" do\n gem \"rails\", \"7.0.0\"\nend\n", + ), + ] { + let mut files = BTreeMap::new(); + files.insert("Gemfile".to_string(), gemfile.to_string()); + files.insert("Gemfile.lock".to_string(), direct.clone()); + let res = rewrite_registry_redirect(&files, &[gem_override("rails", "7.0.0")]); + let out = res.files.get("Gemfile").expect("redirected"); + assert!(out.ends_with(want), "{gemfile:?}: {out:?}"); + assert!(!out.contains("\r\r"), "{out:?}"); + } + } + /// A gem-level source option (`git:` / `path:` / `github:` / `source:`) /// preserved into the Socket source block OVERRIDES it in bundler's DSL, /// leaving the redirect a silent no-op that still gets attested. Fail @@ -18762,10 +18901,12 @@ mod tests { r.warnings ); let out = r.files.get("Gemfile").expect("declaration rewritten"); + // A CRLF declaration's block takes its CRLF (#1831). + let nl = if decl.ends_with('\r') { "\r\n" } else { "\n" }; let want = if opts.is_empty() { - " gem \"vuln-gem\", \"1.0.0\"\nend".to_string() + format!(" gem \"vuln-gem\", \"1.0.0\"{nl}end") } else { - format!(" gem \"vuln-gem\", \"1.0.0\", {opts}\nend") + format!(" gem \"vuln-gem\", \"1.0.0\", {opts}{nl}end") }; assert!(out.contains(&want), "{decl:?}: {out}"); } diff --git a/crates/socket-patch-core/src/patch/redirect/upstream/gem.rs b/crates/socket-patch-core/src/patch/redirect/upstream/gem.rs index c49eaf4ec..8a425c34e 100644 --- a/crates/socket-patch-core/src/patch/redirect/upstream/gem.rs +++ b/crates/socket-patch-core/src/patch/redirect/upstream/gem.rs @@ -62,7 +62,7 @@ use super::{Ctx, FormatResult, HostedPin, UpstreamClient, View}; use crate::formats::gem::{ bundler_manifest_for, parse_spec, same_remote, split_checksum_entry, BUNDLER_LOCKS, }; -use crate::utils::line_endings::{to_lf, LineEndings}; +use crate::utils::line_endings::{self, to_lf, LineEndings}; use crate::vendor::gem::{gem_declaration_any, quoted_literal}; /// The default upstream `GEM` remote. @@ -1178,7 +1178,14 @@ fn line_before_start(text: &str, at: usize) -> usize { } fn restore_manifest(text: &str, block: &Block, gem: &Gem<'_>, decl: &Decl) -> String { - let eol = if text.contains("\r\n") { "\r\n" } else { "\n" }; + // The restored line takes the block's ending: the rewriter writes it in + // the ending of the line it replaced (#1831). A block in LF, the only + // spelling before #1831, takes the ending of the lines around it. + let eol = if text[block.start..block.end].contains("\r\n") { + "\r\n" + } else { + line_endings::terminator(&format!("{}{}", &text[..block.start], &text[block.end..])) + }; let mut start = block.start; let replacement = match decl { // The append's own blank separator goes with it. @@ -1785,7 +1792,7 @@ mod tests { run(&t, &direct), "group :test do\n gem \"rails\", \"7.0.0\", require: false\nend\n" ); - // CRLF manifest (the rewriter wrote its block in LF). + // CRLF manifest, its block in LF (written before #1831). let t = format!("source \"https://rubygems.org\"\r\ngem \"puma\"\r\n{block}\n"); assert_eq!( run(&t, &direct),