Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Defect]: Inconsistency of formats: can data in data components be repeated? #518

Closed
andreas-hilti opened this issue Sep 4, 2024 · 6 comments · Fixed by #530
Closed

Comments

@andreas-hilti
Copy link
Contributor

andreas-hilti commented Sep 4, 2024

Describe the defect

In JSON, the data member of a (data) component is an array: https://cyclonedx.org/docs/1.6/json/#components_items_data
In protobuf, it is not repeated:

// This object SHOULD be specified for any component of type `data` and must not be specified for other component types.
optional ComponentData data = 26;

Also in xml it is not repeated:

<xs:element name="data" type="bom:componentDataType" minOccurs="0" maxOccurs="1">
<xs:annotation>
<xs:documentation>This object SHOULD be specified for any component of type `data` and must not be
specified for other component types.</xs:documentation>
</xs:annotation>
</xs:element>

for instance this is not valid:

<?xml version="1.0"?>
<bom serialNumber="urn:uuid:44dabf30-74d9-4949-9bef-1da4ef41a531" version="1" xmlns="http://cyclonedx.org/schema/bom/1.5">
	<components>
		<component type="data">
			<name>acme-library data</name>
			<data>
				<type>source-code</type>				
				<contents>
					<url>http://acme.org/src</url>					
				</contents>
				<classification>public</classification>
			</data>			
			<data>
				<type>other</type>				
				<description>Description of other data</description>
			</data>			
		</component>
	</components>
</bom>

Additional context

@stevespringett
Copy link
Member

Thanks for reporting. Looks like a defect in the XML Schema. The correct fix should be to make maxOccurs unbounded.

@jkowalleck
Copy link
Member

changing the current ProtoBuff item from optional to repeated would be a breaking change in the implementation

@andreas-hilti
Copy link
Contributor Author

changing the current ProtoBuff item from optional to repeated would be a breaking change in the implementation

Why is this? Is proto2 supported as well?

For proto3:
https://protobuf.dev/programming-guides/proto3/#updating

For string, bytes, and message fields, optional is compatible with repeated. Given serialized data of a repeated field as input, clients that expect this field to be optional will take the last input value if it’s a primitive type field or merge all input elements if it’s a message type field.

Unless I'm mistaken, all messages that contained only a single "data" would continue to work, and this is the kind of compatibility that you are looking for, isn't it?

@jkowalleck
Copy link
Member

jkowalleck commented Nov 3, 2024

For string, bytes, and message fields, optional is compatible with repeated. [...]

the field in question is a complex type - a message.
so there should be no issue fixing this, I guess.

@nscuro what do you think?


the XML could be fixed in an upcoming dot release, too.
the proto buff - I will clarify anc check.

@stevespringett,

i would pull this fix into the upcoming 1.6.1 if this is okay.

@jkowalleck jkowalleck modified the milestones: 1.7, 1.6.1 Nov 3, 2024
@stevespringett
Copy link
Member

1.6.1 is ok @jkowalleck

@jkowalleck
Copy link
Member

@andreas-hilti ,

may I ask you for a code review of the proposed fix? #530

jkowalleck added a commit that referenced this issue Nov 7, 2024
fixes <#518>

---------

Signed-off-by: Jan Kowalleck <[email protected]>
Signed-off-by: Jan Kowalleck <[email protected]>
Co-authored-by: andreas-hilti <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants