From 56831ae9ebb803dfb96b7f6b20b305282e5ff687 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 9 Oct 2026 00:00:06 +0000 Subject: [PATCH 1/2] Start refactor for #905 Assisted-by: Claude Code:claude-opus-5-5 From 2f5cd77e80297c8f8729feaa095ab916da3784ef Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 9 Oct 2026 00:07:17 +0000 Subject: [PATCH 2/2] Move BOM handling in 4 more files to formats::text The vlt.json modifiers probe, the read-only go.mod normalizer, the vendored Gradle settings editor (appended lines and the pluginManagement insertion point) and the shared Pipfile.lock parser spelled out "skip a leading UTF-8 BOM" inline. They now call strip_bom or split_bom, so one leading BOM is encoding everywhere (#905). Zero or one leading BOM behaves as before. A Pipfile.lock that starts with two BOMs is now unparseable (the second is content), the same rule every other reader follows since #1160; it parsed before. PENDING_INLINE_BOMS drops these four files (11 remain, all in files open PRs change). Each former caller gets a 0/1/2-BOM test. Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-core/src/formats/text.rs | 4 --- .../src/patch/redirect/vlt.rs | 25 +++++++++++++++-- .../src/vendor/go_mod_edit.rs | 18 +++++++++++-- .../src/vendor/jvm/gradle.rs | 24 +++++++++++++++-- .../src/vendor/lock_inventory/pypi.rs | 27 ++++++++++++++++--- 5 files changed, 85 insertions(+), 13 deletions(-) diff --git a/crates/socket-patch-core/src/formats/text.rs b/crates/socket-patch-core/src/formats/text.rs index c96a0c46b..bd64b7d77 100644 --- a/crates/socket-patch-core/src/formats/text.rs +++ b/crates/socket-patch-core/src/formats/text.rs @@ -73,10 +73,6 @@ mod tests { "patch/redirect/npmrc.rs", "patch/redirect/upstream/npm.rs", "patch/redirect/upstream/pypi.rs", - "patch/redirect/vlt.rs", - "vendor/go_mod_edit.rs", - "vendor/jvm/gradle.rs", - "vendor/lock_inventory/pypi.rs", "vendor/yarn_classic_lock.rs", "vex/discover/npm.rs", "vex/discover/pypi_other.rs", diff --git a/crates/socket-patch-core/src/patch/redirect/vlt.rs b/crates/socket-patch-core/src/patch/redirect/vlt.rs index 0ae2d9e9d..331c31703 100644 --- a/crates/socket-patch-core/src/patch/redirect/vlt.rs +++ b/crates/socket-patch-core/src/patch/redirect/vlt.rs @@ -167,8 +167,7 @@ fn is_old_lockfile_ignored<'a>( && (dep_id.first.is_empty() || is_registry_url_segment(&dep_id.first, options)) }); let declares_modifiers = vlt_config.is_some_and(|text| { - let text = text.strip_prefix('\u{feff}').unwrap_or(text); - serde_json::from_str::(text) + serde_json::from_str::(crate::formats::text::strip_bom(text)) .ok() .and_then(|v| v.as_object().map(|o| o.contains_key("modifiers"))) .unwrap_or(false) @@ -677,6 +676,28 @@ mod tests { ))); } + /// `vlt.json`'s `modifiers` probe reads past exactly one leading BOM + /// (`formats::text::strip_bom`): a second one is content, so the file + /// is not JSON and declares nothing. + #[test] + fn modifiers_probe_reads_past_one_vlt_json_bom_only() { + let lock = "{\n \"lockfileVersion\": 0,\n \"options\": {},\n \"nodes\": {\n \"··left-pad@1.3.0\": [0,\"left-pad\",\"sha512-REGISTRY==\"]\n },\n \"edges\": {}\n}\n"; + let old_lockfile = |config: &str| { + let mut result = RewriteResult::default(); + rewrite_vlt_lock( + &files(&[(VLT_LOCK, lock), (VLT_CONFIG, config)]), + &[dep("left-pad", "1.3.0", Some(SHA))], + false, + &mut result, + ); + codes(&result).contains(&"redirect_vlt_old_lockfile_ignored") + }; + assert!(!old_lockfile("{\"modifiers\": {}}")); + assert!(!old_lockfile("\u{feff}{\"modifiers\": {}}")); + assert!(old_lockfile("\u{feff}\u{feff}{\"modifiers\": {}}")); + assert!(old_lockfile("{}")); + } + #[test] fn ledger_keys_carry_the_raw_extra_after_a_tilde() { let lock = lock_with(&[ diff --git a/crates/socket-patch-core/src/vendor/go_mod_edit.rs b/crates/socket-patch-core/src/vendor/go_mod_edit.rs index 78e3bf10f..defa4de35 100644 --- a/crates/socket-patch-core/src/vendor/go_mod_edit.rs +++ b/crates/socket-patch-core/src/vendor/go_mod_edit.rs @@ -412,8 +412,7 @@ pub fn module_path(text: &str) -> Option { /// Anything else is left verbatim (and so never parses as ours). Writers /// edit the raw text instead. pub(crate) fn normalize_for_read(text: &str) -> String { - let text = text.strip_prefix('\u{feff}').unwrap_or(text); - unquote_tokens(text) + unquote_tokens(crate::formats::text::strip_bom(text)) } fn unquote_tokens(text: &str) -> String { @@ -896,6 +895,21 @@ mod tests { use super::*; use tokio::fs; + /// The read-only go.mod parsers drop exactly one leading BOM + /// (`formats::text::strip_bom`); a second one stays content. + #[test] + fn normalize_for_read_drops_one_leading_bom_only() { + assert_eq!(normalize_for_read("module \"a/b\"\n"), "module a/b\n"); + assert_eq!( + normalize_for_read("\u{feff}module \"a/b\"\n"), + "module a/b\n" + ); + assert_eq!( + normalize_for_read("\u{feff}\u{feff}module a/b\n"), + "\u{feff}module a/b\n" + ); + } + // ── path ownership ─────────────────────────────────────────────── #[test] fn test_detect_owner() { diff --git a/crates/socket-patch-core/src/vendor/jvm/gradle.rs b/crates/socket-patch-core/src/vendor/jvm/gradle.rs index 53b082c15..c6f3df725 100644 --- a/crates/socket-patch-core/src/vendor/jvm/gradle.rs +++ b/crates/socket-patch-core/src/vendor/jvm/gradle.rs @@ -34,6 +34,7 @@ use super::{ }; use super::layout::{self, safe_coordinates}; +use crate::formats::text::{split_bom, strip_bom}; use crate::formats::xml::{self, Element}; /// The owned settings script. Its bytes change only with a CLI release. @@ -1512,7 +1513,7 @@ fn newline_of(text: &str) -> &'static str { fn append_line(text: &str, line: &str) -> String { let nl = newline_of(text); let mut out = text.to_string(); - if !out.is_empty() && !out.ends_with('\n') && out != "\u{feff}" { + if !strip_bom(&out).is_empty() && !out.ends_with('\n') { out.push_str(nl); } out.push_str(line); @@ -1858,7 +1859,7 @@ fn first_statement_offset(text: &str, toks: &[Token]) -> usize { } } // Never in front of a byte-order mark. - let bom = if text.starts_with('\u{feff}') { 3 } else { 0 }; + let bom = split_bom(text).0.len(); match toks.get(i) { Some(t) => text[..t.start].rfind('\n').map_or(0, |j| j + 1).max(bom), None => text.len(), @@ -2473,6 +2474,25 @@ mod tests { const GSON_POM: &[u8] = b"com.google.code.gsongson-parent2.10.1\n"; + /// `append_line` and `first_statement_offset` treat exactly one + /// leading BOM as encoding (`formats::text`): a file holding only a + /// BOM gets no separator line, and nothing is inserted in front of it. + #[test] + fn appended_and_inserted_lines_skip_one_leading_bom() { + assert_eq!(append_line("", "x"), "x\n"); + assert_eq!(append_line("\u{feff}", "x"), "\u{feff}x\n"); + assert_eq!(append_line("a", "x"), "a\nx\n"); + assert_eq!( + append_line("\u{feff}\u{feff}", "x"), + "\u{feff}\u{feff}\nx\n" + ); + let offset = |text: &str| first_statement_offset(text, &dsl::tokens(text, Dsl::Groovy)); + assert_eq!(offset("plugins {}\n"), 0); + assert_eq!(offset("\u{feff}plugins {}\n"), 3); + assert_eq!(offset("import a.B\nplugins {}\n"), 11); + assert_eq!(offset("\u{feff}import a.B\nplugins {}\n"), 14); + } + fn patch() -> JvmPatch<'static> { JvmPatch { group_id: "com.google.code.gson", diff --git a/crates/socket-patch-core/src/vendor/lock_inventory/pypi.rs b/crates/socket-patch-core/src/vendor/lock_inventory/pypi.rs index 4cf21711a..917d4dd66 100644 --- a/crates/socket-patch-core/src/vendor/lock_inventory/pypi.rs +++ b/crates/socket-patch-core/src/vendor/lock_inventory/pypi.rs @@ -9,6 +9,7 @@ use serde_json::Value; use toml_edit::{DocumentMut, Item, TableLike}; use crate::crawlers::python_crawler::canonicalize_pypi_name; +use crate::formats::text::strip_bom; use crate::utils::purl::{percent_decode_purl_component, pypi_purl}; use crate::utils::python_lock::{lock_package_collection, package_artifacts, UvSource}; use crate::utils::requirements::archive_filename_coords; @@ -77,11 +78,12 @@ impl<'a> PipfileLockEntry<'a> { } } -/// Parse a `Pipfile.lock`. Leading UTF-8 BOMs (Windows editors) are not -/// JSON and are skipped — the one BOM policy of every Pipfile.lock reader +/// Parse a `Pipfile.lock`. A leading UTF-8 BOM (Windows editors) is not +/// JSON and is skipped ([`strip_bom`]: one BOM is encoding, a second is +/// content) — the one BOM policy of every Pipfile.lock reader /// (this inventory, the hosted Pipenv rewriter, lockfile discovery). pub(crate) fn parse_pipfile_lock(text: &str) -> serde_json::Result { - serde_json::from_str(text.trim_start_matches('\u{feff}')) + serde_json::from_str(strip_bom(text)) } /// Every package entry of a parsed `Pipfile.lock` (pipfile-spec 6): each @@ -761,6 +763,25 @@ async fn requirements_tree(view: &ProjectView<'_>) -> Option> { Some(files) } +#[cfg(test)] +mod tests { + use super::parse_pipfile_lock; + + /// Every Pipfile.lock reader parses through here: one leading BOM is + /// encoding and skipped, a second is content (not JSON), as + /// `formats::text` rules for every reader. + #[test] + fn pipfile_lock_reads_past_one_bom_only() { + let lock = r#"{"default": {}}"#; + let plain = parse_pipfile_lock(lock).unwrap(); + assert_eq!( + parse_pipfile_lock(&format!("\u{feff}{lock}")).unwrap(), + plain + ); + assert!(parse_pipfile_lock(&format!("\u{feff}\u{feff}{lock}")).is_err()); + } +} + #[cfg(test)] #[path = "pypi_wheel_tests.rs"] mod wheel_tests;