fix(root): resolve version comparison crashes and PURL collisions

osv/ecosystems/root.py

@@ -14,7 +14,11 @@
 """Root ecosystem helper."""
 
 import re
+import packaging_legacy.version
 from .ecosystems_base import OrderedEcosystem
+from .maven import Version as MavenVersion
+from ..third_party.univers.alpine import AlpineLinuxVersion
+from ..third_party.univers.debian import Version as DebianVersion
 
 
 class Root(OrderedEcosystem):
@@ -37,79 +41,91 @@ class Root(OrderedEcosystem):
   def _sort_key(self, version: str):
     """Generate sort key for Root version strings.
 
-    Handles multiple version formats:
-    - Alpine: 1.0.0-r10071
-    - Python: 1.0.0+root.io.1
-    - Others: 1.0.0.root.io.1
+    Delegates to the appropriate ecosystem version parser based on the
+    ecosystem suffix (e.g., :Alpine:3.18, :Debian:12, :npm).
 
     Args:
       version: Version string to parse
 
     Returns:
-      Tuple suitable for sorting
+      Tuple with (version_object, root_patch) for sorting
     """
-    # Try Alpine format: <version>-r<number>
-    alpine_match = re.match(r'^(.+?)-r(\d+)$', version)
-    if alpine_match:
-      upstream = alpine_match.group(1)
-      root_patch = int(alpine_match.group(2))
-      return self._parse_upstream_version(upstream) + (root_patch,)
+    upstream_version = version
+    root_patch = 0
 
-    # Try Python format: <version>+root.io.<number>
+    # Extract Root-specific suffixes
+    # Python format: <version>+root.io.<number>
     python_match = re.match(r'^(.+?)\+root\.io\.(\d+)$', version)
     if python_match:
-      upstream = python_match.group(1)
+      upstream_version = python_match.group(1)
       root_patch = int(python_match.group(2))
-      return self._parse_upstream_version(upstream) + (root_patch,)
 
-    # Try other format: <version>.root.io.<number>
+    # Generic format: <version>.root.io.<number>
     other_match = re.match(r'^(.+?)\.root\.io\.(\d+)$', version)
     if other_match:
-      upstream = other_match.group(1)
+      upstream_version = other_match.group(1)
       root_patch = int(other_match.group(2))
-      return self._parse_upstream_version(upstream) + (root_patch,)
 
-    # Fallback: treat as generic version
-    return self._parse_upstream_version(version)
+    # Alpine format with Root suffix: <version>-r<number>
+    # Note: Alpine naturally uses -r<revision>
+    alpine_match = re.match(r'^(.+?)-r(\d+)$', upstream_version)
+    if alpine_match:
+      root_patch = int(alpine_match.group(2))
 
-  def _parse_upstream_version(self, version: str):
-    """Parse upstream version component.
+    # Determine the sub-ecosystem from the suffix
+    sub_ecosystem = self._get_sub_ecosystem()
 
-    Attempts to extract numeric and string components for sorting.
+    # Parse the upstream version using the appropriate version class
+    return self._parse_upstream_version(upstream_version,
+                                        sub_ecosystem) + (root_patch,)
 
-    Args:
-      version: Upstream version string
+  def _get_sub_ecosystem(self) -> str:
+    """Extract the sub-ecosystem from the suffix.
 
     Returns:
-      Tuple of parsed components
+      Sub-ecosystem name (e.g., 'Alpine', 'Debian', 'npm', 'PyPI')
     """
-    parts = []
-
-    # Split on common delimiters
-    components = re.split(r'[.-]', version)
-
-    for component in components:
-      # Try to parse as integer
-      try:
-        parts.append(int(component))
-      except ValueError:
-        # If not numeric, use string comparison
-        # Convert to tuple of character codes for consistent sorting
-        parts.append(component)
+    if not self.suffix:
+      return 'unknown'
 
-    return tuple(parts)
+    # Parse suffix like ":Alpine:3.18" -> "Alpine"
+    # or ":npm" -> "npm"
+    parts = self.suffix.strip(':').split(':')
+    if parts:
+      return parts[0]
+    return 'unknown'
 
-  def sort_key(self, version: str):
-    """Public sort key method.
+  def _parse_upstream_version(self, version: str, sub_ecosystem: str):
+    """Parse upstream version using ecosystem-specific parser.
 
     Args:
-      version: Version string
+      version: Upstream version string
+      sub_ecosystem: Sub-ecosystem name (e.g., 'Alpine', 'Debian', 'npm')
 
     Returns:
-      Tuple for sorting
+      Tuple with version object for comparison
+
+    Raises:
+      ValueError: If the version cannot be parsed by the appropriate parser
     """
-    try:
-      return self._sort_key(version)
-    except Exception:
-      # Fallback to string comparison if parsing fails
-      return (version,)
+    match sub_ecosystem.lower():
+      case 'alpine':
+        if not AlpineLinuxVersion.is_valid(version):
+          raise ValueError(f'Invalid Alpine version: {version}')
+        return (AlpineLinuxVersion(version),)
+
+      case 'debian' | 'ubuntu':
+        if not DebianVersion.is_valid(version):
+          raise ValueError(f'Invalid Debian/Ubuntu version: {version}')
+        return (DebianVersion.from_string(version),)
+
+      case 'pypi' | 'python':
+        # packaging_legacy.version.parse handles invalid versions gracefully
+        # by returning LegacyVersion, so we don't need explicit validation
+        return (packaging_legacy.version.parse(version),)
+
+      case 'maven':
+        return (MavenVersion.from_string(version),)
+
+      case _:
+        return (packaging_legacy.version.parse(version),)

osv/ecosystems/root_test.py

@@ -0,0 +1,172 @@
+# Copyright 2025 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+"""Root ecosystem helper tests."""
+
+import unittest
+
+from . import root
+
+
+class RootEcosystemTest(unittest.TestCase):
+  """Root ecosystem helper tests."""
+
+  def test_alpine_versions(self):
+    """Test Root:Alpine version comparison."""
+    ecosystem = root.Root(suffix=':Alpine:3.18')
+
+    # Basic Alpine version ordering
+    self.assertGreater(
+        ecosystem.sort_key('1.51.0-r20072'),
+        ecosystem.sort_key('1.51.0-r20071'))
+    self.assertGreater(
+        ecosystem.sort_key('1.0.0-r2'), ecosystem.sort_key('1.0.0-r1'))
+
+    # Check the 0 sentinel value
+    self.assertLess(ecosystem.sort_key('0'), ecosystem.sort_key('1.0.0-r1'))
+
+    # Check equality
+    self.assertEqual(
+        ecosystem.sort_key('1.51.0-r20071'),
+        ecosystem.sort_key('1.51.0-r20071'))
+
+  def test_debian_versions(self):
+    """Test Root:Debian version comparison."""
+    ecosystem = root.Root(suffix=':Debian:12')
+
+    # Basic Debian version ordering with Root suffix
+    self.assertGreater(
+        ecosystem.sort_key('22.12.0-2+deb12u1.root.io.5'),
+        ecosystem.sort_key('22.12.0-2.root.io.1'))
+
+    self.assertGreater(
+        ecosystem.sort_key('1.18.0-6+deb11u3-r20072'),
+        ecosystem.sort_key('1.18.0-6+deb11u3-r20071'))
+
+    # Check equality
+    self.assertEqual(
+        ecosystem.sort_key('1.18.0-6+deb11u3-r20071'),
+        ecosystem.sort_key('1.18.0-6+deb11u3-r20071'))
+
+  def test_ubuntu_versions(self):
+    """Test Root:Ubuntu version comparison."""
+    ecosystem = root.Root(suffix=':Ubuntu:22.04')
+
+    # Ubuntu version ordering
+    self.assertGreater(
+        ecosystem.sort_key('1.2.3-4ubuntu2'),
+        ecosystem.sort_key('1.2.3-4ubuntu1'))
+
+  def test_pypi_versions(self):
+    """Test Root:PyPI version comparison."""
+    ecosystem = root.Root(suffix=':PyPI')
+
+    # Python version ordering with Root suffix
+    self.assertGreater(
+        ecosystem.sort_key('1.0.0+root.io.5'),
+        ecosystem.sort_key('1.0.0+root.io.1'))
+
+    # PEP440 version ordering
+    self.assertGreater(ecosystem.sort_key('2.0.0'), ecosystem.sort_key('1.9.9'))
+    self.assertGreater(
+        ecosystem.sort_key('1.0.0'), ecosystem.sort_key('1.0.0rc1'))
+
+  def test_npm_versions(self):
+    """Test Root:npm version comparison."""
+    ecosystem = root.Root(suffix=':npm')
+
+    # npm semver ordering with Root suffix
+    self.assertGreater(
+        ecosystem.sort_key('1.0.0.root.io.5'),
+        ecosystem.sort_key('1.0.0.root.io.1'))
+
+    # Basic semver ordering
+    self.assertGreater(ecosystem.sort_key('2.0.0'), ecosystem.sort_key('1.9.9'))
+    self.assertGreater(ecosystem.sort_key('1.0.1'), ecosystem.sort_key('1.0.0'))
+
+  def test_maven_versions(self):
+    """Test Root:Maven version comparison."""
+    ecosystem = root.Root(suffix=':Maven')
+
+    # Maven version ordering
+    self.assertGreater(ecosystem.sort_key('2.0'), ecosystem.sort_key('1.0'))
+    self.assertGreater(
+        ecosystem.sort_key('1.0'), ecosystem.sort_key('1.0-SNAPSHOT'))
+
+  def test_unknown_ecosystem_fallback(self):
+    """Test fallback behavior for unknown ecosystems."""
+    ecosystem = root.Root(suffix=None)
+
+    # Should still work with Alpine-like versions
+    self.assertGreater(
+        ecosystem.sort_key('1.0.0-r2'), ecosystem.sort_key('1.0.0-r1'))
+
+    # Should work with generic versions
+    self.assertGreater(ecosystem.sort_key('2.0.0'), ecosystem.sort_key('1.0.0'))
+
+  def test_github_issue_4396(self):
+    """Test the specific versions from GitHub issue #4396."""
+    ecosystem = root.Root(suffix=':Debian:12')
+
+    # The problematic comparison that used to crash
+    key1 = ecosystem.sort_key('22.12.0-2.root.io.1')
+    key2 = ecosystem.sort_key('22.12.0-2+deb12u1.root.io.5')
+
+    # Should not crash and should compare correctly
+    self.assertLess(key1, key2)
+
+  def test_root_suffix_extraction(self):
+    """Test extraction of Root-specific version suffixes."""
+    ecosystem = root.Root(suffix=':PyPI')
+
+    # Python format: +root.io.<number>
+    key = ecosystem.sort_key('1.0.0+root.io.5')
+    self.assertIsNotNone(key)
+
+    # Generic format: .root.io.<number>
+    key = ecosystem.sort_key('1.0.0.root.io.5')
+    self.assertIsNotNone(key)
+
+  def test_invalid_versions(self):
+    """Test that invalid versions raise appropriate errors."""
+    # Alpine ecosystem with invalid version
+    ecosystem_alpine = root.Root(suffix=':Alpine:3.18')
+    with self.assertRaises(ValueError) as context:
+      ecosystem_alpine.sort_key('invalid-version!@#')
+    self.assertIn('Invalid Alpine version', str(context.exception))
+
+    # Debian ecosystem with empty version
+    ecosystem_debian = root.Root(suffix=':Debian:12')
+    with self.assertRaises(ValueError) as context:
+      ecosystem_debian.sort_key('')
+    self.assertIn('Invalid Debian/Ubuntu version', str(context.exception))
+
+  def test_sub_ecosystem_extraction(self):
+    """Test _get_sub_ecosystem method."""
+    # Test various suffix formats
+    # pylint: disable=protected-access
+    ecosystem = root.Root(suffix=':Alpine:3.18')
+    self.assertEqual(ecosystem._get_sub_ecosystem(), 'Alpine')
+
+    ecosystem = root.Root(suffix=':Debian:12')
+    self.assertEqual(ecosystem._get_sub_ecosystem(), 'Debian')
+
+    ecosystem = root.Root(suffix=':npm')
+    self.assertEqual(ecosystem._get_sub_ecosystem(), 'npm')
+
+    ecosystem = root.Root(suffix=None)
+    self.assertEqual(ecosystem._get_sub_ecosystem(), 'unknown')
+
+
+if __name__ == '__main__':
+  unittest.main()

osv/purl_helpers.py

@@ -87,8 +87,9 @@
         EcosystemPURL('rpm', 'redhat'),
     'Rocky Linux':
         EcosystemPURL('rpm', 'rocky-linux'),
-    'Root':
-        EcosystemPURL('generic', 'root'),
+    # Note: Root ecosystem does not generate PURLs as Root packages are not
+    # published to public registries (npm, PyPI, Maven Central, etc.).
+    # Users can query Root vulnerabilities using ecosystem and package name.
     'RubyGems':
         EcosystemPURL('gem', None),
     'SUSE':
@@ -142,8 +143,10 @@ def package_to_purl(ecosystem: str, package_name: str) -> str | None:
                                           'BellSoft Hardened Containers'):
     suffix = '?arch=source'
 
-  # Encode package name: preserve '/' only when no namespace is defined
-  safe_chars = '' if purl_namespace else '/'
+  # Encode package name: preserve '/' in specific cases
+  # - When no namespace is defined
+  # - For maven type (uses / to separate group ID and artifact ID)
+  safe_chars = '' if (purl_namespace and purl_type != 'maven') else '/'
   encoded_name = quote(package_name, safe=safe_chars)
 
   return f'pkg:{purl_ecosystem}/{encoded_name}{suffix}'