Skip to content

Commit cc9b473

Browse files
Fix hosted NuGet lock walk: BOM and other versions (#593, #623) (#1339)
* Start refactor for #594 Assisted-by: Claude Code:claude-opus-5-5 * Wire vendored nuget.config via formats::nuget Vendored NuGet now reads the source keys and finds the <packageSources>, <packageSourceMapping> and <configuration> anchors through formats::nuget::parse_config, the reader that hosted, upstream restore and VEX already use. The private substring scanner (blank_comments, parse_config_source_keys, attr_value, self_closing_package_sources, insert_at_line) is deleted. User impact: - A close tag written with whitespace (</packageSources >) is now the section that gets extended; vendor used to append a second section NuGet ignores, so restore failed NU1100/NU1403 (#685). - An empty <packageSourceMapping /> is expanded in place instead of left beside a second mapping section. - A section opened and closed on one line receives the source inside it, not before its open tag. - Catch-all keys are written XML-encoded, so a key with & or a quote keeps its identity. - Malformed XML or a repeated section is refused with "malformed XML or a repeated section; not wired" instead of being spliced at the first substring match, as hosted already does. Output bytes for well-formed configs are unchanged. Fixes #685 Refs #594 Assisted-by: Claude Code:claude-opus-5-5 * Start NuGet fix: nuget-lock-reader Draft placeholder while the fix is written. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Pin only the patched NuGet version; read BOM locks Hosted NuGet rewrote every packages.lock.json entry of the patched id, whatever version it resolved, so a multi-targeting project silently got the patched version's bytes in another framework (#593). Vendored only pinned the matching version but still routed every version of the id to a feed serving one, so the other framework failed NU1102. Both modes now refuse such a lock (redirect_ / vendor_nuget_lock_other_version) before writing anything, and only entries at the patched version are re-pinned. A lock starting with a UTF-8 BOM, which dotnet restores fine, made hosted skip the redirect with exit 0 and vendored fail apply (#623). The lock is now read past the BOM, and the BOM and layout are kept on write. All four lock walkers (vendored pin, hosted redirect, upstream restore, VEX) now share one reader in formats::nuget::lock. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent f0d0ab0 commit cc9b473

7 files changed

Lines changed: 457 additions & 122 deletions

File tree

‎crates/socket-patch-cli/CLI_CONTRACT.md‎

Lines changed: 2 additions & 2 deletions
Large diffs are not rendered by default.
Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
//! `packages.lock.json`: the one reader every NuGet lock walker shares —
2+
//! the vendored pin, the hosted redirect, the hosted upstream restore and
3+
//! VEX discovery.
4+
//!
5+
//! A lock is `{"dependencies": {"<tfm>": {"<Id>": {"resolved": …,
6+
//! "contentHash": …}}}}`. NuGet matches ids case-insensitively, and one
7+
//! multi-targeting project can resolve the same id at a different version
8+
//! per framework, so a walker that pins a patched `<Id> <version>` must
9+
//! match the id AND the version: an entry at another version is a
10+
//! different package that the patched bytes must never replace (#593).
11+
//!
12+
//! dotnet restores a lock that starts with a UTF-8 BOM (one saved by a
13+
//! Windows editor or Windows PowerShell 5.1), so the parse reads past it
14+
//! (#623). Writers keep it: the vendored and upstream-restore edits are
15+
//! string surgery on the hash value, and the hosted rewrite re-renders in
16+
//! the original layout.
17+
18+
use serde_json::{Map, Value};
19+
20+
use crate::vendor::nuget_feed::normalize_nuget_version;
21+
22+
/// The lock NuGet writes beside a project under its default name.
23+
pub(crate) const PACKAGES_LOCK: &str = "packages.lock.json";
24+
25+
/// One `dependencies.<tfm>.<id>` entry that restores from a source: its raw
26+
/// id key, `resolved` and `contentHash` strings.
27+
#[derive(Debug, Clone, Copy)]
28+
pub(crate) struct NugetLockEntry<'a> {
29+
pub(crate) id: &'a str,
30+
pub(crate) resolved: &'a str,
31+
pub(crate) content_hash: Option<&'a str>,
32+
}
33+
34+
/// Parse a lock's text as dotnet does, reading past a leading UTF-8 BOM.
35+
pub(crate) fn parse_lock(text: &str) -> serde_json::Result<Value> {
36+
serde_json::from_str(crate::formats::text::strip_bom(text))
37+
}
38+
39+
/// Every entry of a parsed lock, target framework by target framework, in
40+
/// document-key order. Frameworks that are not objects and entries without
41+
/// a string `resolved` (`type: "Project"` references, which nothing
42+
/// restores from a source) are skipped; strings are raw (callers trim /
43+
/// normalize / compare ids as they need).
44+
pub(crate) fn nuget_lock_entries(doc: &Value) -> impl Iterator<Item = NugetLockEntry<'_>> {
45+
doc.get("dependencies")
46+
.and_then(Value::as_object)
47+
.into_iter()
48+
.flat_map(|frameworks| frameworks.values())
49+
.filter_map(Value::as_object)
50+
.flatten()
51+
.filter_map(|(id, entry)| {
52+
Some(NugetLockEntry {
53+
id,
54+
resolved: entry.get("resolved").and_then(Value::as_str)?,
55+
content_hash: entry.get("contentHash").and_then(Value::as_str),
56+
})
57+
})
58+
}
59+
60+
/// The entries of package `id` (case-insensitive) whose `resolved`
61+
/// normalizes to `version_norm` ([`normalize_nuget_version`]): the entries
62+
/// a patch of `id version_norm` replaces.
63+
pub(crate) fn locked_at<'a>(
64+
doc: &'a Value,
65+
id: &'a str,
66+
version_norm: &'a str,
67+
) -> impl Iterator<Item = NugetLockEntry<'a>> {
68+
nuget_lock_entries(doc).filter(move |e| {
69+
e.id.eq_ignore_ascii_case(id) && normalize_nuget_version(e.resolved) == version_norm
70+
})
71+
}
72+
73+
/// [`locked_at`] for editing: `(id key, entry object)` of every matching
74+
/// entry, framework by framework.
75+
pub(crate) fn locked_at_mut<'a>(
76+
doc: &'a mut Value,
77+
id: &'a str,
78+
version_norm: &'a str,
79+
) -> Vec<(&'a str, &'a mut Map<String, Value>)> {
80+
doc.get_mut("dependencies")
81+
.and_then(Value::as_object_mut)
82+
.into_iter()
83+
.flat_map(|frameworks| frameworks.values_mut())
84+
.filter_map(Value::as_object_mut)
85+
.flat_map(|fw| fw.iter_mut())
86+
.filter(|(key, _)| key.eq_ignore_ascii_case(id))
87+
.filter_map(|(key, entry)| Some((key.as_str(), entry.as_object_mut()?)))
88+
.filter(|(_, entry)| {
89+
entry
90+
.get("resolved")
91+
.and_then(Value::as_str)
92+
.is_some_and(|r| normalize_nuget_version(r) == version_norm)
93+
})
94+
.collect()
95+
}
96+
97+
/// The other versions the lock resolves `id` at, normalized, sorted and
98+
/// deduplicated. Both writers route the WHOLE id to a feed that serves only
99+
/// the patched version (`packageSourceMapping` patterns name ids, never
100+
/// versions), so a framework that resolves the id at another version could
101+
/// no longer restore it: the writers refuse such a lock rather than break
102+
/// that framework or re-pin it to the patched version.
103+
pub(crate) fn other_versions(doc: &Value, id: &str, version_norm: &str) -> Vec<String> {
104+
let mut out: Vec<String> = nuget_lock_entries(doc)
105+
.filter(|e| e.id.eq_ignore_ascii_case(id))
106+
.map(|e| normalize_nuget_version(e.resolved))
107+
.filter(|v| v != version_norm)
108+
.collect();
109+
out.sort();
110+
out.dedup();
111+
out
112+
}
113+
114+
/// The refusal detail both writers give for [`other_versions`].
115+
pub(crate) fn other_versions_detail(
116+
lock_rel: &str,
117+
id: &str,
118+
version_norm: &str,
119+
others: &[String],
120+
) -> String {
121+
format!(
122+
"{lock_rel} also resolves {id} at {}; the patch for {version_norm} would route every \
123+
version of {id} to a feed that serves only {version_norm}, so those target frameworks \
124+
could not restore. Align {id} on {version_norm} in every target framework (or patch \
125+
each version), restore, and re-run",
126+
others.join(", ")
127+
)
128+
}
129+
130+
#[cfg(test)]
131+
mod tests {
132+
use super::*;
133+
134+
const MULTI: &str = r#"{
135+
"version": 1,
136+
"dependencies": {
137+
"net6.0": {
138+
"Newtonsoft.Json": { "type": "Direct", "requested": "[12.0.3, )", "resolved": "12.0.3", "contentHash": "OLD12==" }
139+
},
140+
"net8.0": {
141+
"newtonsoft.json": { "type": "Direct", "requested": "[13.0.3, )", "resolved": "13.0.3", "contentHash": "OLD13==" },
142+
"App.Lib": { "type": "Project" }
143+
}
144+
}
145+
}"#;
146+
147+
#[test]
148+
fn bom_lock_parses_like_dotnet_reads_it() {
149+
let bom = format!("\u{feff}{MULTI}");
150+
assert!(serde_json::from_str::<Value>(&bom).is_err());
151+
assert_eq!(parse_lock(&bom).unwrap(), parse_lock(MULTI).unwrap());
152+
}
153+
154+
#[test]
155+
fn locked_at_matches_id_and_version() {
156+
let doc = parse_lock(MULTI).unwrap();
157+
let hits: Vec<_> = locked_at(&doc, "Newtonsoft.Json", "13.0.3").collect();
158+
assert_eq!(hits.len(), 1);
159+
assert_eq!(hits[0].content_hash, Some("OLD13=="));
160+
let mut doc = doc;
161+
let hits = locked_at_mut(&mut doc, "NEWTONSOFT.JSON", "13.0.3");
162+
assert_eq!(hits.len(), 1);
163+
assert_eq!(hits[0].0, "newtonsoft.json");
164+
}
165+
166+
#[test]
167+
fn other_versions_names_every_other_resolution() {
168+
let doc = parse_lock(MULTI).unwrap();
169+
assert_eq!(
170+
other_versions(&doc, "newtonsoft.json", "13.0.3"),
171+
["12.0.3"]
172+
);
173+
assert!(other_versions(&doc, "newtonsoft.json", "12.0.3") == ["13.0.3"]);
174+
assert!(other_versions(&doc, "App.Lib", "1.0.0").is_empty());
175+
// A spelling of the same version is not another version.
176+
let doc = parse_lock(&MULTI.replace("\"12.0.3\"", "\"13.0.3.0\"")).unwrap();
177+
assert!(other_versions(&doc, "Newtonsoft.Json", "13.0.3").is_empty());
178+
}
179+
}

‎crates/socket-patch-core/src/formats/nuget/mod.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010
//! and DOCTYPEs are skipped; an unterminated tag or comment, an unquoted
1111
//! attribute or a mismatched close tag makes the whole file `None`.
1212
13+
pub(crate) mod lock;
14+
1315
use std::collections::BTreeSet;
1416
use std::ops::Range;
1517

0 commit comments

Comments
 (0)