fix: properly sort components based on all properties (#599)

reverts #587 - as this one introduced errors fixes #598 fixes #586 --------- Signed-off-by: Jan Kowalleck <jan.kowalleck@gmail.com> Signed-off-by: Paul Horton <paul.horton@owasp.org> Co-authored-by: Paul Horton <paul.horton@owasp.org>
CycloneDX · Apr 26, 2024 · 8df488c · 8df488c
1 parent 25ea611
commit 8df488c
Show file tree

Hide file tree

Showing 27 changed files with 835 additions and 23 deletions.
diff --git a/.github/workflows/python.yml b/.github/workflows/python.yml
@@ -115,7 +115,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        os: ['ubuntu-latest', 'windows-latest', 'macos-latest']
+        os: ['ubuntu-latest', 'windows-latest', 'macos-13']
         python-version:
           - "3.12" # highest supported
           - "3.11"

diff --git a/cyclonedx/_internal/compare.py b/cyclonedx/_internal/compare.py
@@ -19,9 +19,11 @@
 Everything might change without any notice.
 """
 
-
 from itertools import zip_longest
-from typing import Any, Optional, Tuple
+from typing import TYPE_CHECKING, Any, Dict, Optional, Tuple
+
+if TYPE_CHECKING:  # pragma: no cover
+    from packageurl import PackageURL
 
 
 class ComparableTuple(Tuple[Optional[Any], ...]):
@@ -52,3 +54,42 @@ def __gt__(self, other: Any) -> bool:
                 return False
             return True if s > o else False
         return False
+
+
+class ComparableDict:
+    """
+    Allows comparison of dictionaries, allowing for missing/None values.
+    """
+
+    def __init__(self, dict_: Dict[Any, Any]) -> None:
+        self._dict = dict_
+
+    def __lt__(self, other: Any) -> bool:
+        if not isinstance(other, ComparableDict):
+            return True
+        keys = sorted(self._dict.keys() | other._dict.keys())
+        return ComparableTuple(self._dict.get(k) for k in keys) \
+            < ComparableTuple(other._dict.get(k) for k in keys)
+
+    def __gt__(self, other: Any) -> bool:
+        if not isinstance(other, ComparableDict):
+            return False
+        keys = sorted(self._dict.keys() | other._dict.keys())
+        return ComparableTuple(self._dict.get(k) for k in keys) \
+            > ComparableTuple(other._dict.get(k) for k in keys)
+
+
+class ComparablePackageURL(ComparableTuple):
+    """
+    Allows comparison of PackageURL, allowing for qualifiers.
+    """
+
+    def __new__(cls, purl: 'PackageURL') -> 'ComparablePackageURL':
+        return super().__new__(
+            ComparablePackageURL, (
+                purl.type,
+                purl.namespace,
+                purl.version,
+                ComparableDict(purl.qualifiers) if isinstance(purl.qualifiers, dict) else purl.qualifiers,
+                purl.subpath
+            ))
diff --git a/cyclonedx/model/component.py b/cyclonedx/model/component.py
@@ -14,6 +14,7 @@
 #
 # SPDX-License-Identifier: Apache-2.0
 # Copyright (c) OWASP Foundation. All Rights Reserved.
+
 import re
 from enum import Enum
 from os.path import exists
@@ -25,7 +26,7 @@
 from packageurl import PackageURL
 from sortedcontainers import SortedSet
 
-from .._internal.compare import ComparableTuple as _ComparableTuple
+from .._internal.compare import ComparablePackageURL as _ComparablePackageURL, ComparableTuple as _ComparableTuple
 from .._internal.hash import file_sha1sum as _file_sha1sum
 from ..exception.model import InvalidOmniBorIdException, InvalidSwhidException, NoPropertiesProvidedException
 from ..exception.serialization import (
@@ -42,7 +43,7 @@
     SchemaVersion1Dot5,
     SchemaVersion1Dot6,
 )
-from ..serialization import BomRefHelper, LicenseRepositoryHelper, PackageUrl
+from ..serialization import BomRefHelper, LicenseRepositoryHelper, PackageUrl as PackageUrlSH
 from . import (
     AttachedText,
     Copyright,
@@ -1406,7 +1407,7 @@ def cpe(self, cpe: Optional[str]) -> None:
         self._cpe = cpe
 
     @property
-    @serializable.type_mapping(PackageUrl)
+    @serializable.type_mapping(PackageUrlSH)
     @serializable.xml_sequence(15)
     def purl(self) -> Optional[PackageURL]:
         """
@@ -1699,29 +1700,42 @@ def __eq__(self, other: object) -> bool:
     def __lt__(self, other: Any) -> bool:
         if isinstance(other, Component):
             return _ComparableTuple((
-                self.type, self.mime_type, self.supplier, self.author, self.publisher, self.group, self.name,
-                self.version, self.description, self.scope, _ComparableTuple(self.hashes),
-                _ComparableTuple(self.licenses), self.copyright, self.cpe, self.purl, self.swid, self.pedigree,
+                self.type, self.group, self.name, self.version,
+                self.mime_type, self.supplier, self.author, self.publisher,
+                self.description, self.scope, _ComparableTuple(self.hashes),
+                _ComparableTuple(self.licenses), self.copyright, self.cpe,
+                None if self.purl is None else _ComparablePackageURL(self.purl),
+                self.swid, self.pedigree,
                 _ComparableTuple(self.external_references), _ComparableTuple(self.properties),
                 _ComparableTuple(self.components), self.evidence, self.release_notes, self.modified,
-                _ComparableTuple(self.authors), _ComparableTuple(self.omnibor_ids),
+                _ComparableTuple(self.authors), _ComparableTuple(self.omnibor_ids), self.manufacturer,
+                _ComparableTuple(self.swhids), self.crypto_properties, _ComparableTuple(self.tags)
             )) < _ComparableTuple((
-                other.type, other.mime_type, other.supplier, other.author, other.publisher, other.group, other.name,
-                other.version, other.description, other.scope, _ComparableTuple(other.hashes),
-                _ComparableTuple(other.licenses), other.copyright, other.cpe, other.purl, other.swid, other.pedigree,
+                other.type, other.group, other.name, other.version,
+                other.mime_type, other.supplier, other.author, other.publisher,
+                other.description, other.scope, _ComparableTuple(other.hashes),
+                _ComparableTuple(other.licenses), other.copyright, other.cpe,
+                None if other.purl is None else _ComparablePackageURL(other.purl),
+                other.swid, other.pedigree,
                 _ComparableTuple(other.external_references), _ComparableTuple(other.properties),
                 _ComparableTuple(other.components), other.evidence, other.release_notes, other.modified,
-                _ComparableTuple(other.authors), _ComparableTuple(other.omnibor_ids),
+                _ComparableTuple(other.authors), _ComparableTuple(other.omnibor_ids), other.manufacturer,
+                _ComparableTuple(other.swhids), other.crypto_properties, _ComparableTuple(other.tags)
             ))
         return NotImplemented
 
     def __hash__(self) -> int:
         return hash((
-            self.type, self.mime_type, self.supplier, self.author, self.publisher, self.group, self.name,
-            self.version, self.description, self.scope, tuple(self.hashes), tuple(self.licenses), self.copyright,
-            self.cpe, self.purl, self.swid, self.pedigree, tuple(self.external_references), tuple(self.properties),
-            tuple(self.components), self.evidence, self.release_notes, self.modified, tuple(self.authors),
-            tuple(self.omnibor_ids),
+            self.type, self.group, self.name, self.version,
+            self.mime_type, self.supplier, self.author, self.publisher,
+            self.description, self.scope, tuple(self.hashes),
+            tuple(self.licenses), self.copyright, self.cpe,
+            self.purl,
+            self.swid, self.pedigree,
+            tuple(self.external_references), tuple(self.properties),
+            tuple(self.components), self.evidence, self.release_notes, self.modified,
+            tuple(self.authors), tuple(self.omnibor_ids), self.manufacturer,
+            tuple(self.swhids), self.crypto_properties, tuple(self.tags)
         ))
 
     def __repr__(self) -> str:

diff --git a/tests/_data/models.py b/tests/_data/models.py
@@ -954,8 +954,12 @@ def get_bom_with_licenses() -> Bom:
                                                    url=XsUri('https://www.apache.org/licenses/LICENSE-2.0.html'),
                                                    acknowledgement=LicenseAcknowledgement.CONCLUDED)]),
             Component(name='c-with-name', type=ComponentType.LIBRARY, bom_ref='C3',
-                      licenses=[DisjunctiveLicense(name='some commercial license',
-                                                   text=AttachedText(content='this is a license text'))]),
+                      licenses=[
+                          DisjunctiveLicense(name='some commercial license',
+                                             text=AttachedText(content='this is a license text')),
+                          DisjunctiveLicense(name='some additional',
+                                             text=AttachedText(content='this is additional license text')),
+                      ]),
         ],
         services=[
             Service(name='s-with-expression', bom_ref='S1',
@@ -966,8 +970,12 @@ def get_bom_with_licenses() -> Bom:
                                                  url=XsUri('https://www.apache.org/licenses/LICENSE-2.0.html'),
                                                  acknowledgement=LicenseAcknowledgement.DECLARED)]),
             Service(name='s-with-name', bom_ref='S3',
-                    licenses=[DisjunctiveLicense(name='some commercial license',
-                                                 text=AttachedText(content='this is a license text'))]),
+                    licenses=[
+                        DisjunctiveLicense(name='some commercial license',
+                                           text=AttachedText(content='this is a license text')),
+                        DisjunctiveLicense(name='some additional',
+                                           text=AttachedText(content='this is additional license text')),
+                    ]),
         ])
 
 
@@ -1064,6 +1072,30 @@ def get_bom_for_issue_497_urls() -> Bom:
     ])
 
 
+def get_bom_for_issue_598_multiple_components_with_purl_qualifiers() -> Bom:
+    """regression test for issue #598
+    see https://github.com/CycloneDX/cyclonedx-python-lib/issues/598
+    """
+    return _make_bom(components=[
+        Component(
+            name='dummy', version='2.3.5', bom_ref='dummy-a',
+            purl=PackageURL(
+                type='pypi', namespace=None, name='pathlib2', version='2.3.5', subpath=None,
+                qualifiers={}
+            )
+        ),
+        Component(
+            name='dummy', version='2.3.5', bom_ref='dummy-b',
+            purl=PackageURL(
+                type='pypi', namespace=None, name='pathlib2', version='2.3.5', subpath=None,
+                qualifiers={
+                    'vcs_url': 'git+https://github.com/jazzband/pathlib2.git@5a6a88db3cc1d08dbc86fbe15edfb69fb5f5a3d6'
+                }
+            )
+        )
+    ])
+
+
 def bom_all_same_bomref() -> Tuple[Bom, int]:
     bom = Bom()
     bom.metadata.component = Component(name='root', bom_ref='foo', components=[
@@ -1113,5 +1145,6 @@ def bom_all_same_bomref() -> Tuple[Bom, int]:
     get_bom_with_licenses,
     get_bom_with_multiple_licenses,
     get_bom_for_issue_497_urls,
+    get_bom_for_issue_598_multiple_components_with_purl_qualifiers,
     get_bom_with_component_setuptools_with_v16_fields,
 }
diff --git a/...data/snapshots/get_bom_for_issue_598_multiple_components_with_purl_qualifiers-1.0.xml.bin b/...data/snapshots/get_bom_for_issue_598_multiple_components_with_purl_qualifiers-1.0.xml.bin
@@ -0,0 +1,17 @@
+<?xml version="1.0" ?>
+<bom xmlns="http://cyclonedx.org/schema/bom/1.0" version="1">
+  <components>
+    <component type="library">
+      <name>dummy</name>
+      <version>2.3.5</version>
+      <purl>pkg:pypi/pathlib2@2.3.5?vcs_url=git%2Bhttps://github.com/jazzband/pathlib2.git%405a6a88db3cc1d08dbc86fbe15edfb69fb5f5a3d6</purl>
+      <modified>false</modified>
+    </component>
+    <component type="library">
+      <name>dummy</name>
+      <version>2.3.5</version>
+      <purl>pkg:pypi/pathlib2@2.3.5</purl>
+      <modified>false</modified>
+    </component>
+  </components>
+</bom>
diff --git a/...data/snapshots/get_bom_for_issue_598_multiple_components_with_purl_qualifiers-1.1.xml.bin b/...data/snapshots/get_bom_for_issue_598_multiple_components_with_purl_qualifiers-1.1.xml.bin
@@ -0,0 +1,15 @@
+<?xml version="1.0" ?>
+<bom xmlns="http://cyclonedx.org/schema/bom/1.1" serialNumber="urn:uuid:1441d33a-e0fc-45b5-af3b-61ee52a88bac" version="1">
+  <components>
+    <component type="library" bom-ref="dummy-b">
+      <name>dummy</name>
+      <version>2.3.5</version>
+      <purl>pkg:pypi/pathlib2@2.3.5?vcs_url=git%2Bhttps://github.com/jazzband/pathlib2.git%405a6a88db3cc1d08dbc86fbe15edfb69fb5f5a3d6</purl>
+    </component>
+    <component type="library" bom-ref="dummy-a">
+      <name>dummy</name>
+      <version>2.3.5</version>
+      <purl>pkg:pypi/pathlib2@2.3.5</purl>
+    </component>
+  </components>
+</bom>
diff --git a/...ata/snapshots/get_bom_for_issue_598_multiple_components_with_purl_qualifiers-1.2.json.bin b/...ata/snapshots/get_bom_for_issue_598_multiple_components_with_purl_qualifiers-1.2.json.bin
@@ -0,0 +1,41 @@
+{
+  "components": [
+    {
+      "bom-ref": "dummy-b",
+      "name": "dummy",
+      "purl": "pkg:pypi/pathlib2@2.3.5?vcs_url=git%2Bhttps://github.com/jazzband/pathlib2.git%405a6a88db3cc1d08dbc86fbe15edfb69fb5f5a3d6",
+      "type": "library",
+      "version": "2.3.5"
+    },
+    {
+      "bom-ref": "dummy-a",
+      "name": "dummy",
+      "purl": "pkg:pypi/pathlib2@2.3.5",
+      "type": "library",
+      "version": "2.3.5"
+    }
+  ],
+  "dependencies": [
+    {
+      "ref": "dummy-a"
+    },
+    {
+      "ref": "dummy-b"
+    }
+  ],
+  "metadata": {
+    "timestamp": "2023-01-07T13:44:32.312678+00:00",
+    "tools": [
+      {
+        "name": "cyclonedx-python-lib",
+        "vendor": "CycloneDX",
+        "version": "TESTING"
+      }
+    ]
+  },
+  "serialNumber": "urn:uuid:1441d33a-e0fc-45b5-af3b-61ee52a88bac",
+  "version": 1,
+  "$schema": "http://cyclonedx.org/schema/bom-1.2b.schema.json",
+  "bomFormat": "CycloneDX",
+  "specVersion": "1.2"
+}
diff --git a/...data/snapshots/get_bom_for_issue_598_multiple_components_with_purl_qualifiers-1.2.xml.bin b/...data/snapshots/get_bom_for_issue_598_multiple_components_with_purl_qualifiers-1.2.xml.bin
@@ -0,0 +1,29 @@
+<?xml version="1.0" ?>
+<bom xmlns="http://cyclonedx.org/schema/bom/1.2" serialNumber="urn:uuid:1441d33a-e0fc-45b5-af3b-61ee52a88bac" version="1">
+  <metadata>
+    <timestamp>2023-01-07T13:44:32.312678+00:00</timestamp>
+    <tools>
+      <tool>
+        <vendor>CycloneDX</vendor>
+        <name>cyclonedx-python-lib</name>
+        <version>TESTING</version>
+      </tool>
+    </tools>
+  </metadata>
+  <components>
+    <component type="library" bom-ref="dummy-b">
+      <name>dummy</name>
+      <version>2.3.5</version>
+      <purl>pkg:pypi/pathlib2@2.3.5?vcs_url=git%2Bhttps://github.com/jazzband/pathlib2.git%405a6a88db3cc1d08dbc86fbe15edfb69fb5f5a3d6</purl>
+    </component>
+    <component type="library" bom-ref="dummy-a">
+      <name>dummy</name>
+      <version>2.3.5</version>
+      <purl>pkg:pypi/pathlib2@2.3.5</purl>
+    </component>
+  </components>
+  <dependencies>
+    <dependency ref="dummy-a"/>
+    <dependency ref="dummy-b"/>
+  </dependencies>
+</bom>
diff --git a/...ata/snapshots/get_bom_for_issue_598_multiple_components_with_purl_qualifiers-1.3.json.bin b/...ata/snapshots/get_bom_for_issue_598_multiple_components_with_purl_qualifiers-1.3.json.bin
@@ -0,0 +1,41 @@
+{
+  "components": [
+    {
+      "bom-ref": "dummy-b",
+      "name": "dummy",
+      "purl": "pkg:pypi/pathlib2@2.3.5?vcs_url=git%2Bhttps://github.com/jazzband/pathlib2.git%405a6a88db3cc1d08dbc86fbe15edfb69fb5f5a3d6",
+      "type": "library",
+      "version": "2.3.5"
+    },
+    {
+      "bom-ref": "dummy-a",
+      "name": "dummy",
+      "purl": "pkg:pypi/pathlib2@2.3.5",
+      "type": "library",
+      "version": "2.3.5"
+    }
+  ],
+  "dependencies": [
+    {
+      "ref": "dummy-a"
+    },
+    {
+      "ref": "dummy-b"
+    }
+  ],
+  "metadata": {
+    "timestamp": "2023-01-07T13:44:32.312678+00:00",
+    "tools": [
+      {
+        "name": "cyclonedx-python-lib",
+        "vendor": "CycloneDX",
+        "version": "TESTING"
+      }
+    ]
+  },
+  "serialNumber": "urn:uuid:1441d33a-e0fc-45b5-af3b-61ee52a88bac",
+  "version": 1,
+  "$schema": "http://cyclonedx.org/schema/bom-1.3a.schema.json",
+  "bomFormat": "CycloneDX",
+  "specVersion": "1.3"
+}