node_runtime: Tolerate non-string values in npm time metadata (#57765)
> [!NOTE] > Authored by Claude, but fixes a real issue I encountered; I have reviewed the diff Some npm registries return `npm info --json` with non-string values in the `time` map. JFrog Artifactory emits `"unpublished": null`, and npm itself represents `unpublished` as an object for packages that have had versions unpublished. Because `NpmInfo.time` was typed `HashMap<String, String>`, serde aborted the entire deserialization with `invalid type: null, expected a string`. As a result `select_npm_package_version` never ran, and language servers installed via npm (for example the tsgo extension, `tailwindcss-language-server`, and `json-language-server`) failed to start with an error like: ``` Failed to start language server "tsgo": invalid type: null, expected a string at line 100 column 23 ``` Only version keys in `time` are ever read (to honor npm's `before` cutoff), so this deserializes the map leniently: keep the string-valued entries and drop the rest. The field type and all downstream logic are unchanged. Added a regression test covering a `time` map containing `"unpublished": null`. Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content is consistent with the UI/UX checklist - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Release Notes: - Fixed language servers failing to install when the npm registry returns non-string values (such as `"unpublished": null`) in package `time` metadata
This commit is contained in:
1 parent
130470597b
commit
e5e2ba0709
1 file changed
+45
-1
@@ -408,7 +408,7 @@ pub struct NpmInfo {
|
||||
#[serde(default)]
|
||||
dist_tags: NpmInfoDistTags,
|
||||
versions: Vec<Version>,
|
||||
#[serde(default)]
|
||||
#[serde(default, deserialize_with = "deserialize_npm_info_time")]
|
||||
time: HashMap<String, String>,
|
||||
}
|
||||
|
||||
@@ -417,6 +417,25 @@ pub struct NpmInfoDistTags {
|
||||
latest: Option<Version>,
|
||||
}
|
||||
|
||||
// Some registries put non-string values in the `time` map: JFrog Artifactory emits
|
||||
// `"unpublished": null`, and npm itself reports `unpublished` as an object when a
|
||||
// package has had versions unpublished. Only version keys map to the RFC 3339 strings
|
||||
// we read, so keep the string entries and drop the rest rather than failing to parse
|
||||
// the entire `npm info` response (which would block language server installation).
|
||||
fn deserialize_npm_info_time<'de, D>(deserializer: D) -> Result<HashMap<String, String>, D::Error>
|
||||
where
|
||||
D: serde::Deserializer<'de>,
|
||||
{
|
||||
let entries = HashMap::<String, serde_json::Value>::deserialize(deserializer)?;
|
||||
Ok(entries
|
||||
.into_iter()
|
||||
.filter_map(|(key, value)| match value {
|
||||
serde_json::Value::String(value) => Some((key, value)),
|
||||
_ => None,
|
||||
})
|
||||
.collect())
|
||||
}
|
||||
|
||||
#[derive(Debug, Deserialize)]
|
||||
struct NpmConfig {
|
||||
#[serde(default)]
|
||||
@@ -1212,6 +1231,31 @@ mod tests {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_npm_info_skips_non_string_time_entries() -> Result<()> {
|
||||
// Registries such as JFrog Artifactory include `"unpublished": null` in `time`;
|
||||
// parsing must tolerate this rather than rejecting the whole response.
|
||||
let info: NpmInfo = serde_json::from_str(
|
||||
r#"{
|
||||
"dist-tags": { "latest": "2.0.0" },
|
||||
"versions": ["1.0.0", "2.0.0"],
|
||||
"time": {
|
||||
"unpublished": null,
|
||||
"created": "2024-01-01T00:00:00.000Z",
|
||||
"modified": "2024-02-01T00:00:00.000Z",
|
||||
"1.0.0": "2024-01-01T00:00:00.000Z",
|
||||
"2.0.0": "2024-02-01T00:00:00.000Z"
|
||||
}
|
||||
}"#,
|
||||
)?;
|
||||
|
||||
assert_eq!(
|
||||
select_npm_package_version("test-package", info, Some("2024-02-15T00:00:00.000Z"))?,
|
||||
Version::parse("2.0.0")?
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_select_npm_package_version_uses_latest_before_npm_before_config() -> Result<()> {
|
||||
let info: NpmInfo = serde_json::from_str(
|
||||
|
||||
Reference in new issue
Block a user