[BUG] Schema-invalid serialized result when multiple licenses

[madpah]

As of cyclonedx-python-lib 4.0.0 there appears to be a serialization error when the provided (valid) Model has more than one license added.

Example:

bom = Bom()
...
bom.metadata.licenses = [LicenseChoice(license=License(
    id='Apache-2.0', text=AttachedText(
        content='VGVzdCBjb250ZW50IC0gdGhpcyBpcyBub3QgdGhlIEFwYWNoZSAyLjAgbGljZW5zZSE=', encoding=Encoding.BASE_64
    ), url=XsUri('https://www.apache.org/licenses/LICENSE-2.0.txt')
)), LicenseChoice(license=License(name='OSI_APACHE'))]

produces

...
<ns0:licenses>
    <ns0:license>
        <ns0:id>Apache-2.0</ns0:id>
        <ns0:text content-type="text/plain" encoding="base64">
            VGVzdCBjb250ZW50IC0gdGhpcyBpcyBub3QgdGhlIEFwYWNoZSAyLjAgbGljZW5zZSE=
        </ns0:text>
        <ns0:url>https://www.apache.org/licenses/LICENSE-2.0.txt</ns0:url>
    </ns0:license>
</ns0:licenses>
<ns0:licenses>
    <ns0:license>
        <ns0:name>OSI_APACHE</ns0:name>
    </ns0:license>
</ns0:licenses>
...

Which is invalid as per the CycloneDX schema.

this CDX schema discrepancy was fixed via CycloneDX/specification#204

---

important for the fix:
#365 (comment)

[madpah]

Confirmed this impacts JSON and XML serialization.

[madpah]

After some [internal] discussions with CycloneDX folks, we agree that the current Pythonic Model (use of LicenseChoice) is likely a poor representation of the CDX Specification.

Definitions such as:

def licenses(self) -> "SortedSet[LicenseChoice]":

are incorrect as it allows users of the model to supply a set with more than 1 License Expression - which is invalid according to CycloneDX Spec v1.4.

Syntactically, the above definition should more accurately be defined as:

def licenses(self) -> "Optional[Union[LicenseExpression, List[License]]]":

FYI @jkowalleck

[madpah]

After further investigation, the challenge associated with serialization for licenses is also challenging because the structure/schema is different between JSON and XML under v1.4:

JSON
licenses is an Array of either expression or license - i.e. 0 or more expression or 0 or more license see here

XML
licenses is an complex type defined as either 0 or more license OR 0 or 1 expression see here

[jkowalleck]

re: #365 (comment)
this is bad news in terms of a deserialization-conflict, because we cannot control input data. generic parser/deserializer would not do it here. we might need to write one manually.
WE can control outputdata via our own serializers - so this path has no problems.

anyway, @madpah , could you open an issue/discussion in https://github.com/CycloneDX/specification/ ?

[jkowalleck]

@madpah , i think the following is a forgiving deserializing (since a mix of expression and licenses was possible in spec<=1.4)
but also promising for serialization:

• do not change the models. lets have it a set of mixed expressions and other licenses
• do not change the deserializations -- as it was possible to mix in JSON for CDX<=1.4
• add a preprocessor for serializations:
  if there is any expression in the list of licenses: return a list that contains one only expression.
  This way the usual serialization will work with a list of exactly one item(an expression).
  Result: expressions preferred, all other items silently dropped(or add a warning).

Result:

• you can still deserialize all existing (valid) documents
• the serializer is forced(fixed) to produce valid documents

PS: here is an example implementation i drafted for TypeScript - https://github.com/CycloneDX/cyclonedx-javascript-library/blob/a43f5f1c61945b4479f3ead79d05de1db36a63f1/src/serialize/xml/normalize.ts#L488-L503
PPS: here is a draft implementation in python: #371

[gruebel]

hey, @jkowalleck and @madpah any update on this? I wanted to upgrade our project to use the latest version ,but this blocking me a bit. Any insights of potential fixes or recommendation on what to do with the multiple license case?

[jkowalleck]

I am looking into the proposed fixed.
trying to find a solution.

[jkowalleck]

@madpah impossible to create a solution with the existing solution for serialization.
The library is just not capable to model these things.

We need custom (de)serialization here, as we had previously.
I am not certain if this can be hooked into the currently used serialization framework at all.
please help.

[jkowalleck]

bugfixing this might break things downstream.
so lets put this into next major version

[jkowalleck]

with https://github.com/madpah/serializable/releases/tag/v0.13.0
i should be able to solve the issue soon.

[jkowalleck]

implementation details:

• models must be backwards-compatible: accept a mixture of exptession and others
• deserializer must be backwards-compatible: accept a mixture of exptession and others
• serializer: can print just the one appropriate value ...
• there MUST be tests to check whether "old"/invalid data can be imported

[jkowalleck]

• schema validator ready feat: validate CycloneDX documents #432
• tests snapshots ready feat: XML/JSON results more human-readable #437
• serialization lib ready https://github.com/madpah/serializable/releases/tag/v0.14.1
• test data ready [DRAFT] tests: regression issue365 #454

---

will tackle this issue soon.

[jkowalleck]

solution is implemented and will be part up upcoming major version.
regression test and schema validator in place, to assure a complete fix. 🎈

[jkowalleck]

for everybody willing to test the fix: see 5.0.0-rc.1
this is also available vie PyPI

[jkowalleck]

fix available as of https://github.com/CycloneDX/cyclonedx-python-lib/releases/tag/v5.0.0