Skip to content

Commit 6e60f09

Browse files
authored
Fixes comma support in the lockfile (#279)
The lockfile format doesn't currently escape commas in ranges, which leads to lockfile corruption when they contain some (since we already split on commas entries with multiple descriptors). This diff addresses that by making sure we escape commas when serializing multikeys. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes lockfile key serialization/deserialization rules to introduce escaping for commas/backslashes, which could impact reading/writing lockfiles across versions if edge cases aren’t handled correctly. > > **Overview** > Prevents lockfile corruption when descriptor strings contain commas (e.g., comma-separated PyPI ranges) by **escaping `,` and `\\`** when serializing `MultiKey` values and updating deserialization to correctly unescape them while preserving unknown escapes for backward compatibility. > > Adds an acceptance test that runs `install` twice with a comma-separated `pypi:` range to ensure lockfile round-trips cleanly. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 33eb8f8. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 2281b5c commit 6e60f09

2 files changed

Lines changed: 61 additions & 5 deletions

File tree

packages/zpm/src/lockfile.rs

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,14 @@ impl<T> Serialize for MultiKey<T> where T: ToFileString {
205205
string.push_str(", ");
206206
}
207207

208-
string.push_str(&item.to_file_string());
208+
let serialized = item.to_file_string();
209+
for ch in serialized.chars() {
210+
if ch == ',' || ch == '\\' {
211+
string.push('\\');
212+
}
213+
214+
string.push(ch);
215+
}
209216
}
210217

211218
serializer.serialize_str(&string)
@@ -226,10 +233,37 @@ impl<'de, T: FromFileString> Deserialize<'de> for MultiKey<T> where <T as FromFi
226233
}
227234

228235
fn visit_str<E>(self, value: &str) -> Result<Vec<T>, E> where E: de::Error {
229-
let result = value
230-
.split(',')
231-
.map(str::trim)
232-
.map(|s| T::from_file_string(s))
236+
let mut chunks = Vec::new();
237+
let mut current = String::new();
238+
let mut chars = value.chars().peekable();
239+
240+
while let Some(ch) = chars.next() {
241+
if ch == ',' {
242+
chunks.push(current);
243+
current = String::new();
244+
continue;
245+
}
246+
247+
if ch != '\\' {
248+
current.push(ch);
249+
continue;
250+
}
251+
252+
match chars.peek() {
253+
Some(',') | Some('\\') => {
254+
current.push(chars.next().expect("peeked character should be present"));
255+
}
256+
_ => {
257+
// Keep unknown escapes as-is to stay backward-compatible with legacy lockfiles.
258+
current.push('\\');
259+
}
260+
}
261+
}
262+
263+
chunks.push(current);
264+
265+
let result = chunks.into_iter()
266+
.map(|s| T::from_file_string(s.trim()))
233267
.collect::<Result<Vec<_>, _>>()
234268
.map_err(de::Error::custom)?;
235269

tests/acceptance-tests/pkg-tests-specs/sources/basic/python.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,5 +86,27 @@ describe(`Protocols`, () => {
8686
},
8787
),
8888
);
89+
90+
test(
91+
`it should support running install twice with a comma-separated PyPI range`,
92+
makeTemporaryEnv(
93+
{
94+
dependencies: {
95+
[`pypi-no-deps`]: `pypi:>=1.0.0,<2.0.0`,
96+
},
97+
},
98+
async ({run}) => {
99+
const registryUrl = await tests.startPackageServer();
100+
101+
for (let i = 0; i < 2; i++) {
102+
await run(`install`, {
103+
env: {
104+
ZPM_PYPI_REGISTRY: registryUrl,
105+
},
106+
});
107+
}
108+
},
109+
),
110+
);
89111
});
90112
});

0 commit comments

Comments
 (0)