CycloneDX / CycloneDX/cyclonedx-rust-cargo

XML de/serialization of `normalizedstring` fields is incorrect

Open
#737 0 comments 1 reaction 0 assignees View on GitHub
Dominant language
Rust
Stars
178
Forks
66
PR merge metrics
No merged PRs in 30d

Description

Based on the findings in #733 it was concluded that the XML serialization code does not parse the XML type `normalizedstring` correctly. This does not only affect the license name field, but all occurrences of this string type.

There is a `NormalizedString` type in `/external_models` available that replaces a set of forbidden characters, e.g. line feed or tab, in `NormalizedString::new` with white space characters. All XML schemas in the [specification](https://github.com/CycloneDX/specification) repository use this standard XML type `normalizedstring`, while the JSON schemas do not use such a string type or put any associated restrictions on them. Therefore this type should only apply to the XML serialization code.

The `cyclonedx-bom` code handles the `normalizedstring` XML type as a `String`, only when converting the spec types to their model representations the `NormalizedString` type is used, but for both JSON and XML parsed objects. That is wrong, the `normalizedstring` XML type & therefore the `NormalizedString` model type are only relevant as part of the XML specification & should be used in this context only.

To fix the issue, the code should be adapted in the following way.

* [ ] introduce a NewType struct that encapsulates a `String`, which is handled as a normal string with `serde_json`, most likely under folder `/spec`, but applies replacing chars in XML serialization code
* [ ] check the XML specification to see which types in the `/spec` folder need to change their type from `String` to the NewType struct, therefore documenting types closer to the XML specification. This needs to be done for all available spec versions
* [ ] remove the `NormalizedString` type from the `models` folder, the model types that currently declare it, e.g. `Option`, should declare them with `String` only
* [ ] implement `FromXml` & `ToXml` traits for the NewType struct that replaces the forbidden white space characters
* [ ] update all conversion functions between model & spec types accordingly
* [ ] update serialization & deserialization code of the string fields in the XML logic, for example instead of using `read_simple_tag` to read a `String` the `NewType::read_xml_element` trait method is called
* [ ] update & fix tests, there are lot of specific tests to validate `NormalizedString` fields, which should become unnecessary, likely to eliminate a lot of unnecessary validations?
* [ ] remove the `validate_normalized_string` function, it's very likely not needed anymore, which should simplify validation logic for a bunch of fields

These are a lot of changes, but the outcome is

* all string types that define `normalizedstring` in the XML schemas are correctly parsed afterwards, similar to the behavior of other XML libraries in other languages
* JSON & XML string types are handled the same way afterwards, the forbidden white space characters are allowed in JSON, but replaced in XML correctly
* validation logic becomes simpler, same for a lot of tests
* #733 will be fixed, so are all other fields with the `normalizedstring` type
* finally the XML serialization code follows the specifcation more closely, & therefore `cyclonedx-bom` gets closer to other BOM libraries of the CycloneDX family

cc @Shnatsel, @lfrancke (thanks & also thanks to all others on Discord for their input)

Contributor guide

Open the contributing guide

Research direction

Start by tracing the XML and JSON serialization paths in the /spec and /models folders, including the existing NormalizedString, FromXml, ToXml, read_simple_tag, and validate_normalized_string code. Compare all available spec versions and their XML schemas before updating conversions and tests. Done means normalizedstring fields are replaced only during XML serialization, JSON preserves whitespace, and the obsolete model validation is removed.

Written by the indexing model from the issue text.

Assessment

Tech stack
json, rust, xml
Domain
tooling
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
32/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.