From ee3bb4b5868be9bb371cf79c6e8bb5c8e5c44dc2 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Mon, 3 Aug 2026 22:18:22 +0200 Subject: [PATCH] fix(lockfile): read a CRLF or BOM-prefixed multi-document lockfile (#13609) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `extract_main_document` / `extract_env_document` split the combined `pnpm-lock.yaml` on literal `---\n` and `\n---\n`. A lockfile pacquet did not write may carry CRLF line endings — a `core.autocrlf` checkout on Windows — or a UTF-8 BOM, and neither marker then matches. The main extractor fell through to returning the whole file, which serde rejected as `multiple YAML documents detected`, so the lockfile was discarded and every dependency re-resolved from the registry. Normalize the whole file (strip BOM, CRLF to LF) inside both extractors, which covers every caller. `save_value_to_path` already normalized the same way inline and now shares the helper. The TypeScript stack needs no counterpart change: `pnpm11/lockfile/fs/src/yamlDocuments.ts` already normalizes CRLF in `extractMainDocument` / `extractEnvDocument` and strips the BOM at each read site. Closes https://github.com/pnpm/pnpm/issues/13606 --- .../read-crlf-multi-document-lockfile.md | 5 ++ pnpm/crates/lockfile/src/env_lockfile.rs | 2 +- .../crates/lockfile/src/env_lockfile/tests.rs | 13 +++++ pnpm/crates/lockfile/src/load_lockfile.rs | 2 +- .../lockfile/src/load_lockfile/tests.rs | 23 ++++++++ pnpm/crates/lockfile/src/save_lockfile.rs | 9 ++-- pnpm/crates/lockfile/src/yaml_documents.rs | 53 ++++++++++++++----- .../lockfile/src/yaml_documents/tests.rs | 23 +++++++- 8 files changed, 111 insertions(+), 19 deletions(-) create mode 100644 .changeset/read-crlf-multi-document-lockfile.md diff --git a/.changeset/read-crlf-multi-document-lockfile.md b/.changeset/read-crlf-multi-document-lockfile.md new file mode 100644 index 0000000000..d4c9192af0 --- /dev/null +++ b/.changeset/read-crlf-multi-document-lockfile.md @@ -0,0 +1,5 @@ +--- +"pacquet": patch +--- + +Fixed `pnpm install` ignoring a `pnpm-lock.yaml` that carries a leading env lockfile document when the file has CRLF line endings or a UTF-8 byte order mark, as a `core.autocrlf` checkout on Windows produces. The lockfile was reported as broken with `multiple YAML documents detected` and every dependency was re-resolved from the registry [#13606](https://github.com/pnpm/pnpm/issues/13606). diff --git a/pnpm/crates/lockfile/src/env_lockfile.rs b/pnpm/crates/lockfile/src/env_lockfile.rs index 33c3ec6c0e..b86241fb63 100644 --- a/pnpm/crates/lockfile/src/env_lockfile.rs +++ b/pnpm/crates/lockfile/src/env_lockfile.rs @@ -116,7 +116,7 @@ impl EnvLockfile { let Some(env_doc) = extract_env_document(&content) else { return Ok(None); }; - let mut env: EnvLockfile = serde_saphyr::from_str(env_doc) + let mut env: EnvLockfile = serde_saphyr::from_str(&env_doc) .map_err(|source| LoadLockfileError::parse_yaml(&path, &source))?; env.root_importer_mut(); Ok(Some(env)) diff --git a/pnpm/crates/lockfile/src/env_lockfile/tests.rs b/pnpm/crates/lockfile/src/env_lockfile/tests.rs index 559e72d13a..12f2ad33fb 100644 --- a/pnpm/crates/lockfile/src/env_lockfile/tests.rs +++ b/pnpm/crates/lockfile/src/env_lockfile/tests.rs @@ -69,6 +69,19 @@ fn reads_non_numeric_lockfile_version() { ); } +#[test] +fn reads_a_crlf_combined_lockfile() { + let dir = TempDir::new().unwrap(); + let env = sample_env_lockfile(); + env.write(dir.path()).unwrap(); + let path = dir.path().join(Lockfile::FILE_NAME); + let crlf = std::fs::read_to_string(&path).unwrap().replace('\n', "\r\n"); + std::fs::write(&path, crlf).unwrap(); + + let read_back = EnvLockfile::read(dir.path()).unwrap().expect("env document present"); + assert_eq!(read_back, env); +} + #[test] fn write_preserves_existing_main_document() { let dir = TempDir::new().unwrap(); diff --git a/pnpm/crates/lockfile/src/load_lockfile.rs b/pnpm/crates/lockfile/src/load_lockfile.rs index 5e6c3b2c45..f44407a67f 100644 --- a/pnpm/crates/lockfile/src/load_lockfile.rs +++ b/pnpm/crates/lockfile/src/load_lockfile.rs @@ -118,7 +118,7 @@ impl Lockfile { return Ok(None); } serde_saphyr::from_str_with_options::( - main, + &main, serde_saphyr::options! { // Every size-proportional budget is raised to the document's // byte length: none of these dimensions can exceed the size of diff --git a/pnpm/crates/lockfile/src/load_lockfile/tests.rs b/pnpm/crates/lockfile/src/load_lockfile/tests.rs index 4bb665cb0e..be7066860e 100644 --- a/pnpm/crates/lockfile/src/load_lockfile/tests.rs +++ b/pnpm/crates/lockfile/src/load_lockfile/tests.rs @@ -87,6 +87,29 @@ fn parses_main_document_from_combined_yaml() { assert_eq!(combined_loaded, main_only_loaded); } +/// Regression test for : a +/// combined lockfile checked out with CRLF line endings was handed to +/// serde whole, failing as "multiple YAML documents detected" and +/// making every install re-resolve from the registry. +#[test] +fn parses_main_document_from_crlf_combined_yaml() { + let combined = format!("---\n{ENV_DOC}\n---\n{MAIN_DOC}").replace('\n', "\r\n"); + let tmp = write_lockfile(&combined); + let virtual_store_dir = tmp.path().join("node_modules").join(".pacquet"); + + let crlf_loaded = Lockfile::load_current_from_virtual_store_dir(&virtual_store_dir) + .expect("load CRLF combined lockfile") + .expect("CRLF combined lockfile should be present"); + + let tmp_main = write_lockfile(MAIN_DOC); + let main_only_dir = tmp_main.path().join("node_modules").join(".pacquet"); + let main_only_loaded = Lockfile::load_current_from_virtual_store_dir(&main_only_dir) + .expect("load main-only lockfile") + .expect("main-only lockfile should be present"); + + assert_eq!(crlf_loaded, main_only_loaded); +} + #[test] fn env_only_lockfile_loads_as_none() { let env_only = format!("---\n{ENV_DOC}\n"); diff --git a/pnpm/crates/lockfile/src/save_lockfile.rs b/pnpm/crates/lockfile/src/save_lockfile.rs index ef5f3be702..7a27aa712d 100644 --- a/pnpm/crates/lockfile/src/save_lockfile.rs +++ b/pnpm/crates/lockfile/src/save_lockfile.rs @@ -1,6 +1,9 @@ use crate::{ Lockfile, serialize_yaml, - yaml_documents::{YAML_DOCUMENT_SEPARATOR, YAML_DOCUMENT_START, extract_env_document}, + yaml_documents::{ + YAML_DOCUMENT_SEPARATOR, YAML_DOCUMENT_START, extract_env_document, + normalize_lockfile_content, + }, }; use derive_more::{Display, Error}; use pacquet_diagnostics::miette::{self, Diagnostic}; @@ -79,9 +82,7 @@ pub fn save_value_to_path( ) -> Result<(), SaveLockfileError> { let content = serialize_yaml::to_string(value).map_err(SaveLockfileError::SerializeYaml)?; let existing = match fs::read_to_string(path) { - Ok(existing) => { - Some(existing.strip_prefix('\u{feff}').unwrap_or(&existing).replace("\r\n", "\n")) - } + Ok(existing) => Some(normalize_lockfile_content(&existing).into_owned()), Err(error) if error.kind() == io::ErrorKind::NotFound => None, Err(error) => return Err(SaveLockfileError::WriteFile(error)), }; diff --git a/pnpm/crates/lockfile/src/yaml_documents.rs b/pnpm/crates/lockfile/src/yaml_documents.rs index bb38efea31..bdf1fe9a8d 100644 --- a/pnpm/crates/lockfile/src/yaml_documents.rs +++ b/pnpm/crates/lockfile/src/yaml_documents.rs @@ -6,6 +6,13 @@ //! and the second document is the regular project lockfile. Pacquet //! only consumes the second document, so this module strips the //! leading env document before handing the content to serde. +//! +//! Every entry point here takes the *whole* file and normalizes it +//! first: a lockfile pacquet did not write may carry a UTF-8 BOM or +//! CRLF line endings (a `core.autocrlf` checkout on Windows), and the +//! document markers below would then match nothing. + +use std::borrow::Cow; /// Document-stream marker that ends one YAML document and starts the /// next. @@ -14,16 +21,25 @@ pub(crate) const YAML_DOCUMENT_SEPARATOR: &str = "\n---\n"; /// Document-stream marker at the very start of a file. pub(crate) const YAML_DOCUMENT_START: &str = "---\n"; +/// Strip a leading UTF-8 BOM and rewrite CRLF as LF, so the rest of the +/// lockfile machinery only ever sees the byte shape pacquet writes. +#[must_use] +pub fn normalize_lockfile_content(content: &str) -> Cow<'_, str> { + let content = content.strip_prefix('\u{feff}').unwrap_or(content); + if content.contains("\r\n") { + Cow::Owned(content.replace("\r\n", "\n")) + } else { + Cow::Borrowed(content) + } +} + /// Extract the main lockfile document (second YAML document) from a /// combined file. #[must_use] -pub fn extract_main_document(content: &str) -> &str { - let Some(rest) = content.strip_prefix(YAML_DOCUMENT_START) else { - return content; - }; - match rest.find(YAML_DOCUMENT_SEPARATOR) { - Some(idx) => &rest[idx + YAML_DOCUMENT_SEPARATOR.len()..], - None => "", +pub fn extract_main_document(content: &str) -> Cow<'_, str> { + match normalize_lockfile_content(content) { + Cow::Borrowed(content) => Cow::Borrowed(main_document_of(content)), + Cow::Owned(content) => Cow::Owned(main_document_of(&content).to_string()), } } @@ -35,12 +51,25 @@ pub fn extract_main_document(content: &str) -> &str { /// - Returns the slice between the leading `---\n` and the next /// `\n---\n` separator. A leading `---\n` with no following separator /// (an env-only file with no main document) also yields `None`. -/// -/// pacquet reads the whole lockfile into memory rather than streaming, -/// so this skips chunked BOM/CRLF handling — the only callers pass -/// content pacquet itself wrote with LF line endings. #[must_use] -pub fn extract_env_document(content: &str) -> Option<&str> { +pub fn extract_env_document(content: &str) -> Option> { + match normalize_lockfile_content(content) { + Cow::Borrowed(content) => env_document_of(content).map(Cow::Borrowed), + Cow::Owned(content) => env_document_of(&content).map(|doc| Cow::Owned(doc.to_string())), + } +} + +fn main_document_of(content: &str) -> &str { + let Some(rest) = content.strip_prefix(YAML_DOCUMENT_START) else { + return content; + }; + match rest.find(YAML_DOCUMENT_SEPARATOR) { + Some(idx) => &rest[idx + YAML_DOCUMENT_SEPARATOR.len()..], + None => "", + } +} + +fn env_document_of(content: &str) -> Option<&str> { let rest = content.strip_prefix(YAML_DOCUMENT_START)?; rest.find(YAML_DOCUMENT_SEPARATOR).map(|idx| &rest[..idx]) } diff --git a/pnpm/crates/lockfile/src/yaml_documents/tests.rs b/pnpm/crates/lockfile/src/yaml_documents/tests.rs index ca08d78168..e798859b02 100644 --- a/pnpm/crates/lockfile/src/yaml_documents/tests.rs +++ b/pnpm/crates/lockfile/src/yaml_documents/tests.rs @@ -1,4 +1,4 @@ -use super::extract_main_document; +use super::{extract_env_document, extract_main_document}; #[test] fn returns_entire_content_when_it_does_not_start_with_separator() { @@ -18,3 +18,24 @@ fn returns_the_second_document_from_a_combined_file() { let combined = format!("---\nfoo: bar\n---\n{main}"); assert_eq!(extract_main_document(&combined), main); } + +#[test] +fn splits_a_crlf_combined_file() { + let combined = "---\r\nfoo: bar\r\n---\r\nlockfileVersion: 9.0\r\npackages: {}\r\n"; + assert_eq!(extract_main_document(combined), "lockfileVersion: 9.0\npackages: {}\n"); + assert_eq!(extract_env_document(combined).as_deref(), Some("foo: bar")); +} + +#[test] +fn splits_a_combined_file_behind_a_byte_order_mark() { + let combined = "\u{feff}---\nfoo: bar\n---\nlockfileVersion: 9.0\n"; + assert_eq!(extract_main_document(combined), "lockfileVersion: 9.0\n"); + assert_eq!(extract_env_document(combined).as_deref(), Some("foo: bar")); +} + +#[test] +fn normalizes_a_single_document_file() { + let content = "\u{feff}lockfileVersion: 9.0\r\npackages: {}\r\n"; + assert_eq!(extract_main_document(content), "lockfileVersion: 9.0\npackages: {}\n"); + assert_eq!(extract_env_document(content), None); +}