fix(ivy,osgi): encode PURL components instead of formatting them

src/parsers/ivy.rs

@@ -196,11 +196,7 @@ fn interpret_ivy_xml(content: &str, path: &Path) -> PackageData {
     package_data.dependencies = dependencies;
 
     if let (Some(namespace), Some(name)) = (&package_data.namespace, &package_data.name) {
-        package_data.purl = Some(truncate_field(build_ivy_purl(
-            namespace,
-            name,
-            package_data.version.as_deref(),
-        )));
+        package_data.purl = build_ivy_purl(namespace, name, package_data.version.as_deref());
     }
 
     package_data
@@ -401,13 +397,15 @@ fn parse_ivy_dependency(element: &BytesStart) -> Option<Dependency> {
     let rev = attr_value(element, b"rev");
     let conf = attr_value(element, b"conf");
 
+    // An ivy dependency without an organisation has no namespace to place, so it
+    // falls back to a name-only PURL — still through the encoder.
     let purl = match &org {
         Some(org) => build_ivy_purl(org, &name, None),
-        None => format!("pkg:ivy/{}", name),
+        None => crate::parsers::utils::simple_purl("ivy", &name, None),
     };
 
     Some(Dependency {
-        purl: Some(truncate_field(purl)),
+        purl,
         extracted_requirement: rev.map(truncate_field),
         scope: conf.map(truncate_field),
         is_runtime: None,
@@ -419,11 +417,12 @@ fn parse_ivy_dependency(element: &BytesStart) -> Option<Dependency> {
     })
 }
 
-fn build_ivy_purl(organisation: &str, module: &str, revision: Option<&str>) -> String {
-    let mut purl = format!("pkg:ivy/{}/{}", organisation, module);
-    if let Some(revision) = revision.filter(|value| !value.trim().is_empty()) {
-        purl.push('@');
-        purl.push_str(revision);
-    }
-    purl
+/// Builds an ivy PURL through the encoder.
+///
+/// `organisation` and `module` come straight from XML attributes, so they can
+/// hold anything. Hand-formatting them produced strings that silently lost data
+/// on re-parse: a space in either component truncated the PURL, taking the
+/// revision with it.
+fn build_ivy_purl(organisation: &str, module: &str, revision: Option<&str>) -> Option<String> {
+    crate::parsers::utils::namespaced_purl("ivy", organisation, module, revision)
 }

src/parsers/ivy_test.rs

@@ -253,3 +253,55 @@ fn test_xml_entities_are_decoded() {
     assert_eq!(pkg.namespace.as_deref(), Some("org.example&co"));
     assert_eq!(pkg.name.as_deref(), Some("a&b"));
 }
+
+#[test]
+fn test_ivy_purls_encode_components_and_keep_the_revision() {
+    use std::io::Write;
+    use std::str::FromStr;
+
+    // `organisation`, `module` and `rev` are raw XML attributes. Hand-formatting
+    // them lost data on re-parse: a space truncated the PURL and took the
+    // revision with it, so the module silently lost its version.
+    let dir = std::env::temp_dir().join(format!(
+        "provenant-ivy-{}",
+        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("ivy.xml");
+    let mut file = std::fs::File::create(&path).expect("create ivy.xml");
+    write!(
+        file,
+        r#"<ivy-module version="2.0">
+  <info organisation="org with space" module="mod?x" revision="1.0"/>
+  <dependencies>
+    <dependency org="dep org" name="dep?name" rev="2.0"/>
+  </dependencies>
+</ivy-module>"#
+    )
+    .expect("write ivy.xml");
+
+    let package_data = IvyXmlParser::extract_first_package(&path);
+
+    let purl = package_data.purl.as_deref().expect("package purl");
+    assert_eq!(purl, "pkg:ivy/org%20with%20space/mod%3Fx@1.0");
+    let parsed = packageurl::PackageUrl::from_str(purl).expect("purl should parse");
+    assert_eq!(parsed.namespace(), Some("org with space"));
+    assert_eq!(parsed.name(), "mod?x");
+    // The revision is the part that used to disappear.
+    assert_eq!(parsed.version(), Some("1.0"));
+    assert_eq!(parsed.to_string(), purl);
+
+    let dependency = package_data
+        .dependencies
+        .first()
+        .expect("one dependency expected");
+    assert_eq!(
+        dependency.purl.as_deref(),
+        Some("pkg:ivy/dep%20org/dep%3Fname")
+    );
+
+    std::fs::remove_dir_all(&dir).ok();
+}

src/parsers/maven/manifest.rs

@@ -141,7 +141,7 @@ pub(crate) fn interpret_manifest_mf(content: &str, path: &Path) -> PackageData {
             package_data.version = bundle_version.clone();
 
             if let (Some(name), Some(version)) = (&package_data.name, &package_data.version) {
-                package_data.purl = Some(format!("pkg:osgi/{}@{}", name, version));
+                package_data.purl = crate::parsers::utils::simple_purl("osgi", name, Some(version));
             }
         }
 
@@ -291,7 +291,7 @@ pub(super) fn parse_osgi_package_list(package_list: &str, scope: &str) -> Vec<De
         let is_optional = package_entry.contains("resolution:=optional");
 
         dependencies.push(Dependency {
-            purl: Some(format!("pkg:osgi/{}", package_name)),
+            purl: crate::parsers::utils::simple_purl("osgi", package_name, None),
             extracted_requirement: version_requirement,
             scope: Some(scope.to_string()),
             is_runtime: Some(true),
@@ -335,7 +335,7 @@ pub(super) fn parse_osgi_bundle_list(bundle_list: &str, scope: &str) -> Vec<Depe
         let is_optional = bundle_entry.contains("resolution:=optional");
 
         dependencies.push(Dependency {
-            purl: Some(format!("pkg:osgi/{}", bundle_name)),
+            purl: crate::parsers::utils::simple_purl("osgi", bundle_name, None),
             extracted_requirement: version_requirement,
             scope: Some(scope.to_string()),
             is_runtime: Some(!is_optional),

src/parsers/maven/manifest_test.rs

@@ -492,3 +492,48 @@ fn test_osgi_manifest_without_implementation_version_stays_osgi() {
         Some("pkg:osgi/com.fasterxml.jackson.core.jackson-core@2.18.0".to_string())
     );
 }
+
+#[test]
+fn test_osgi_purls_encode_components_and_keep_the_version() {
+    use std::str::FromStr;
+
+    // Manifest headers are free text, so a `Bundle-SymbolicName` or an imported
+    // package name can hold anything. Hand-formatting truncated the PURL at the
+    // first space, which took the bundle version with it.
+    let temp_dir = TempDir::new().expect("create temp dir");
+    let meta_inf = temp_dir.path().join("META-INF");
+    fs::create_dir_all(&meta_inf).expect("create META-INF");
+    let manifest_path = meta_inf.join("MANIFEST.MF");
+    fs::write(
+        &manifest_path,
+        "Bundle-SymbolicName: my bundle?x\nBundle-Version: 1.0.0\nImport-Package: com.foo bar\nRequire-Bundle: req bundle#z\n",
+    )
+    .expect("write MANIFEST.MF");
+
+    let package_data = MavenParser::extract_first_package(&manifest_path);
+
+    let purl = package_data.purl.as_deref().expect("package purl");
+    assert_eq!(purl, "pkg:osgi/my%20bundle%3Fx@1.0.0");
+    let parsed = packageurl::PackageUrl::from_str(purl).expect("purl should parse");
+    assert_eq!(parsed.name(), "my bundle?x");
+    // The version is the part that used to be lost.
+    assert_eq!(parsed.version(), Some("1.0.0"));
+    assert_eq!(parsed.to_string(), purl);
+
+    let purls: Vec<&str> = package_data
+        .dependencies
+        .iter()
+        .filter_map(|dependency| dependency.purl.as_deref())
+        .collect();
+    assert!(purls.contains(&"pkg:osgi/com.foo%20bar"), "got {purls:?}");
+    // `#` would otherwise become a subpath rather than part of the name.
+    assert!(
+        purls.contains(&"pkg:osgi/req%20bundle%23z"),
+        "got {purls:?}"
+    );
+    for purl in &purls {
+        let parsed = packageurl::PackageUrl::from_str(purl).expect("purl should parse");
+        assert_eq!(parsed.subpath(), None);
+        assert_eq!(parsed.to_string(), *purl);
+    }
+}

src/parsers/opam.rs

@@ -658,7 +658,7 @@ fn extract_parties(authors: &[String], maintainers: &[String]) -> Vec<Party> {
 fn extract_dependencies(deps: &[(String, String)]) -> Vec<Dependency> {
     deps.iter()
         .map(|(name, version_constraint)| Dependency {
-            purl: crate::parsers::utils::simple_purl("opam", name, None).map(truncate_field),
+            purl: crate::parsers::utils::simple_purl("opam", name, None),
             extracted_requirement: Some(truncate_field(version_constraint.clone())),
             scope: Some("dependency".to_string()),
             is_runtime: Some(true),

src/parsers/utils.rs

@@ -307,18 +307,25 @@ pub fn url_authority_host(authority: &str) -> &str {
 /// when the components cannot form a PURL, which is the honest outcome: the
 /// declared text is still reported in the fields that carry it.
 ///
+/// Components are bounded before construction, never after: truncating an
+/// assembled PURL can cut a percent escape in half or drop a trailing component,
+/// turning a valid PURL into one that no longer parses. Callers must not apply
+/// `truncate_field` to the result.
+///
 /// 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<String> {
-    let name = name.trim();
+    let name = truncate_field(name.trim().to_string());
     if name.is_empty() {
         return None;
     }
 
-    let mut package_url = PackageUrl::new(package_type.to_string(), name.to_string()).ok()?;
+    let mut package_url = PackageUrl::new(package_type.to_string(), name).ok()?;
     if let Some(version) = version.map(str::trim).filter(|value| !value.is_empty()) {
-        package_url.with_version(version.to_string()).ok()?;
+        package_url
+            .with_version(truncate_field(version.to_string()))
+            .ok()?;
     }
     Some(package_url.to_string())
 }
@@ -336,15 +343,18 @@ pub fn namespaced_purl(
     name: &str,
     version: Option<&str>,
 ) -> Option<String> {
-    let (namespace, name) = (namespace.trim(), name.trim());
+    let namespace = truncate_field(namespace.trim().to_string());
+    let name = truncate_field(name.trim().to_string());
     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()?;
+    let mut package_url = PackageUrl::new(package_type.to_string(), name).ok()?;
+    package_url.with_namespace(namespace).ok()?;
     if let Some(version) = version.map(str::trim).filter(|value| !value.is_empty()) {
-        package_url.with_version(version.to_string()).ok()?;
+        package_url
+            .with_version(truncate_field(version.to_string()))
+            .ok()?;
     }
     Some(package_url.to_string())
 }