diff --git a/src/parsers/conan.rs b/src/parsers/conan.rs index 2f79d1184..4ba63e7bc 100644 --- a/src/parsers/conan.rs +++ b/src/parsers/conan.rs @@ -30,7 +30,6 @@ use std::path::Path; use crate::parser_warn as warn; -use packageurl::PackageUrl; use ruff_python_ast as ast; use ruff_python_parser::parse_module; use serde_json::Value; @@ -477,16 +476,9 @@ fn parse_conan_reference(ref_str: &str) -> Option { // A range constraint is not a PURL version, so ranged and bare references // both fall through to a name-only PURL and keep the constraint in - // `extracted_requirement`. - let purl = version - .as_deref() - .and_then(|v| { - PackageUrl::new("conan", name).ok().map(|mut p| { - let _ = p.with_version(v); - p.to_string() - }) - }) - .unwrap_or_else(|| format!("pkg:conan/{}", name)); + // `extracted_requirement`. Both forms go through the encoder — the + // hand-formatted fallback left a name like `my pkg` unencoded. + let purl = crate::parsers::utils::simple_purl("conan", name, version.as_deref()); let is_pinned = version_spec .as_ref() @@ -494,7 +486,7 @@ fn parse_conan_reference(ref_str: &str) -> Option { .unwrap_or(false); Some(Dependency { - purl: Some(truncate_field(purl)), + purl: purl.map(truncate_field), extracted_requirement: version_spec, scope: Some("install".to_string()), is_runtime: Some(true), diff --git a/src/parsers/conan_test.rs b/src/parsers/conan_test.rs index dab45dd73..0456e9c53 100644 --- a/src/parsers/conan_test.rs +++ b/src/parsers/conan_test.rs @@ -6,6 +6,7 @@ use crate::models::{DatasourceId, PackageType}; use std::path::PathBuf; +use std::str::FromStr; use super::PackageParser; use super::conan::{ConanFilePyParser, ConanLockParser, ConanfileTxtParser}; @@ -195,6 +196,45 @@ fn test_conanfile_txt_basic() { assert_eq!(result.datasource_id, Some(DatasourceId::ConanConanFileTxt)); } +#[test] +fn test_conan_reference_purls_are_encoded() { + use std::io::Write; + + // A reference with a range constraint yields no PURL version, so it took the + // hand-formatted path where a name was spliced in unencoded. + let dir = std::env::temp_dir().join(format!( + "provenant-conan-{}", + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .expect("system time should be after epoch") + .as_nanos() + )); + std::fs::create_dir_all(&dir).expect("create temp dir"); + let path = dir.join("conanfile.txt"); + let mut file = std::fs::File::create(&path).expect("create conanfile.txt"); + writeln!(file, "[requires]\nmy pkg/[>=1.0]\nplain/1.0").expect("write conanfile.txt"); + + let result = ConanfileTxtParser::extract_first_package(&path); + let purls: Vec<&str> = result + .dependencies + .iter() + .filter_map(|dependency| dependency.purl.as_deref()) + .collect(); + + assert!( + purls.contains(&"pkg:conan/my%20pkg"), + "the ranged reference should encode its name, got {purls:?}" + ); + assert!(purls.contains(&"pkg:conan/plain@1.0")); + + for purl in &purls { + let parsed = packageurl::PackageUrl::from_str(purl).expect("purl should parse"); + assert_eq!(parsed.to_string(), *purl, "{purl} should round-trip"); + } + + std::fs::remove_dir_all(&dir).ok(); +} + #[test] fn test_conan_lock_parser_is_match() { assert!(ConanLockParser::is_match(&PathBuf::from("conan.lock"))); diff --git a/src/parsers/opam.rs b/src/parsers/opam.rs index 5781b456e..8cab1d853 100644 --- a/src/parsers/opam.rs +++ b/src/parsers/opam.rs @@ -228,11 +228,9 @@ fn build_opam_urls( _ => None, }; - let purl = match (name, version) { - (Some(n), Some(v)) => Some(format!("pkg:opam/{}@{}", n, v)), - (Some(n), None) => Some(format!("pkg:opam/{}", n)), - _ => None, - }; + let purl = name + .as_deref() + .and_then(|n| crate::parsers::utils::simple_purl("opam", n, version.as_deref())); (repository_homepage_url, api_data_url, purl) } @@ -660,7 +658,7 @@ fn extract_parties(authors: &[String], maintainers: &[String]) -> Vec { fn extract_dependencies(deps: &[(String, String)]) -> Vec { deps.iter() .map(|(name, version_constraint)| Dependency { - purl: Some(truncate_field(format!("pkg:opam/{}", name))), + purl: crate::parsers::utils::simple_purl("opam", name, None).map(truncate_field), extracted_requirement: Some(truncate_field(version_constraint.clone())), scope: Some("dependency".to_string()), is_runtime: Some(true), @@ -696,6 +694,41 @@ mod tests { assert!(!OpamParser::is_match(path)); } + #[test] + fn test_opam_purls_are_encoded_rather_than_formatted() { + // Names come from a quoted opam field, so anything but a quote reaches the + // PURL. Splicing them in unencoded produced strings that either failed to + // parse or silently changed meaning: a `/` became a namespace separator + // and text after a `#` became a subpath. + use std::str::FromStr; + + for (name, version, expected) in [ + ("conf gmp", None, "pkg:opam/conf%20gmp"), + ("ocaml/evil", None, "pkg:opam/ocaml%2Fevil"), + ("sharp#frag", None, "pkg:opam/sharp%23frag"), + // Already-percent-encoded text is data, not encoding: the real name is + // the literal six characters, so it must survive a round trip. + ("pct%20", None, "pkg:opam/pct%2520"), + ("my pkg", Some("1.0 beta"), "pkg:opam/my%20pkg@1.0%20beta"), + ] { + let purl = crate::parsers::utils::simple_purl("opam", name, version) + .expect("a non-empty name should yield a purl"); + assert_eq!(purl, expected); + + let parsed = packageurl::PackageUrl::from_str(&purl).expect("purl should parse"); + assert_eq!(parsed.name(), name); + assert_eq!(parsed.namespace(), None); + assert_eq!(parsed.subpath(), None); + assert_eq!(parsed.version(), version); + assert_eq!(parsed.to_string(), purl, "purl should round-trip"); + } + + assert_eq!( + crate::parsers::utils::simple_purl("opam", " ", None), + None + ); + } + #[test] fn test_parse_key_value() { let (key, value) = parse_key_value("name: \"js_of_ocaml\"").unwrap(); diff --git a/src/parsers/swift_show_dependencies.rs b/src/parsers/swift_show_dependencies.rs index 98a74e186..8a6fcd3f6 100644 --- a/src/parsers/swift_show_dependencies.rs +++ b/src/parsers/swift_show_dependencies.rs @@ -248,12 +248,7 @@ fn build_dependency( fn create_dependency_purl(dep: &SwiftDependency, version: Option<&str>) -> Option { let url = dep.url.as_deref()?; let (namespace, name) = parse_url_namespace_and_name(url)?; - let mut purl = format!("pkg:swift/{}/{}", namespace, name); - if let Some(version) = version { - purl.push('@'); - purl.push_str(version); - } - Some(purl) + crate::parsers::utils::namespaced_purl("swift", &namespace, &name, version) } /// As with dependencies, the root package needs a namespace derived from its @@ -268,13 +263,7 @@ fn create_root_purl( return None; } let (namespace, _) = parse_url_namespace_and_name(url?)?; - - let mut purl = format!("pkg:swift/{}/{}", namespace, name); - if let Some(version) = version { - purl.push('@'); - purl.push_str(version); - } - Some(purl) + crate::parsers::utils::namespaced_purl("swift", &namespace, name, version) } fn normalize_version(version: Option) -> Option { diff --git a/src/parsers/utils.rs b/src/parsers/utils.rs index 92df48796..f025dfa41 100644 --- a/src/parsers/utils.rs +++ b/src/parsers/utils.rs @@ -297,6 +297,58 @@ pub fn url_authority_host(authority: &str) -> &str { .unwrap_or(authority) } +/// Builds a PURL for a type that takes no namespace, running the name and version +/// through the crate's encoder. +/// +/// Parsers that assemble a PURL with `format!` splice unvalidated text straight +/// into it, so a name carrying a space, `?`, `#` or `/` yields a string that +/// either fails to parse or silently reinterprets — a `/` in the name becomes a +/// namespace separator, and text after a `#` becomes a subpath. Returns `None` +/// when the components cannot form a PURL, which is the honest outcome: the +/// declared text is still reported in the fields that carry it. +/// +/// Only for types the crate does not rewrite. It lowercases names for +/// `bitbucket`, `deb`, `github`, `hex`, `npm` and `pypi`, so those need a +/// deliberate decision about case rather than this helper. +pub fn simple_purl(package_type: &str, name: &str, version: Option<&str>) -> Option { + let name = name.trim(); + if name.is_empty() { + return None; + } + + let mut package_url = PackageUrl::new(package_type.to_string(), name.to_string()).ok()?; + if let Some(version) = version.map(str::trim).filter(|value| !value.is_empty()) { + package_url.with_version(version.to_string()).ok()?; + } + Some(package_url.to_string()) +} + +/// Builds a PURL for a type that carries a namespace, running every component +/// through the crate's encoder. +/// +/// The namespace keeps its `/` separators — its segments are path parts — while +/// the name and version are encoded, which is what hand-formatting missed. +/// +/// Same caveat as [`simple_purl`]: not for the types the crate rewrites. +pub fn namespaced_purl( + package_type: &str, + namespace: &str, + name: &str, + version: Option<&str>, +) -> Option { + let (namespace, name) = (namespace.trim(), name.trim()); + if namespace.is_empty() || name.is_empty() { + return None; + } + + let mut package_url = PackageUrl::new(package_type.to_string(), name.to_string()).ok()?; + package_url.with_namespace(namespace.to_string()).ok()?; + if let Some(version) = version.map(str::trim).filter(|value| !value.is_empty()) { + package_url.with_version(version.to_string()).ok()?; + } + Some(package_url.to_string()) +} + /// Creates a correctly-formatted npm Package URL for scoped or regular packages. /// /// Handles namespace encoding for scoped packages (e.g., `@babel/core`) and ensures