CycloneDX v1.3 JSON Schema - canonical (duplicate) `$id` errors #123

mrutkows · 2022-01-18T20:58:25Z

General Problem

Within the latest, published v1.3 JSON schema (i.e., https://github.com/CycloneDX/specification/blob/master/schema/bom-1.3.schema.json)

The same local $id value being used within 2 different definitions (i.e., both at top-level and under the "definitions" key) which cause "canonical id" or similar errors using several JSON schema validators referenced by the JSON schema project (ignored to unknown results by some others). Output of errors shown below for two tools.

Notes:

Appears that draft v1.4 JSON schema removes all local $Id keys altogether as well as the problematic duplicate $id occurrences listed below.
- https://github.com/CycloneDX/specification/blob/master/schema/bom-1.4.schema.json
Use of "$ref" key/value correct in call cases

JSON Schema reference (applicable to problem space)

Declared schema:

  "$schema": "http://json-schema.org/draft-07/schema#",

Draft 7: https://datatracker.ietf.org/doc/html/draft-handrews-json-schema-01
- Specifically "Schema identification examples": https://datatracker.ietf.org/doc/html/draft-handrews-json-schema-01#section-8.2.4
Draft 7 release notes which may contain applicable comments under release notes:
- See: https://json-schema.org/draft/2019-09/release-notes.html
- Incompatible Changes:
  - Plain name fragments are no longer defined with $id, but instead with the new keyword $anchor (which has a different syntax).
  - $id cannot contain a fragment anymore (except possibly an empty fragment, although that is discouraged).
- Semi-incompatible Changes
  - The old syntax for these keywords is not an error (and the default meta-schema still validates them), so implementations can therefore offer a compatibility mode. However, migrating to the new keywords is straightforward and should be preferred.
  - definitions is now $defs

Reproducible in tools

Go: https://github.com/xeipuuv/gojsonschema

errors.errorString {s: "Reference #/definitions/hash must be canonical"}
errors.errorString {s: "Reference #/definitions/component must be canonical"}
errors.errorString {s: "Reference #/definitions/service must be canonical"}

Online: https://www.liquid-technologies.com/online-json-schema-validator

line	file	message
397:23	schema.json	Duplicate schema id 'http://cyclonedx.org/schema/bom-1.3.schema.json#/properties/components' encountered. Path 'definitions.component.properties.components', line 397, position 23.
920:21	schema.json	Duplicate schema id 'http://cyclonedx.org/schema/bom-1.3.schema.json#/properties/services' encountered. Path 'definitions.service.properties.services', line 920, position 21.
160:19	schema.json	Duplicate schema id 'http://cyclonedx.org/schema/bom-1.3.schema.json#/properties/hashes' encountered. Path 'definitions.tool.properties.hashes', line 160, position 19.

Investigation

Performing a manual inspection of schema...

Base URI

  "$id": "http://cyclonedx.org/schema/bom-1.3.schema.json"

Specific instances of problem

Duplicate "$id": "#/properties/hashes"

within "tool"

specification/schema/bom-1.3.schema.json

Line 161 in c710388

"$id": "#/properties/hashes",

    "tool": {
      "type": "object",
      ...
      "properties": {
       ...
        "hashes": {
          "$id": "#/properties/hashes",
          "type": "array",
          "items": {"$ref": "#/definitions/hash"},
          "title": "Hashes",
          "description": "The hashes of the tool (if applicable)."
        }
      }
    },

within "externalReference":

specification/schema/bom-1.3.schema.json

Line 812 in c710388

"$id": "#/properties/hashes",

    "externalReference": {
      "type": "object",
       ...
      "properties": {
        ...
        "hashes": {
          "$id": "#/properties/hashes",
          "type": "array",
          "items": {"$ref": "#/definitions/hash"},
          "title": "Hashes",
          "description": "The hashes of the external reference (if applicable)."
        }
      }
    },

both reference the same "hash" definition property with the $ref key:

  "definitions": {
    ...
    "hash": {
      "type": "object",
      "title": "Hash Objects",
      "required": [
        "alg",
        "content"
      ],
      "properties": {
        "alg": {
          "$ref": "#/definitions/hash-alg"
        },
        "content": {
          "$ref": "#/definitions/hash-content"
        }
      }
    },

both resolve to the same "$id" value of "http://cyclonedx.org/schema/bom-1.3.schema.json#/properties/hashes"

Duplicate "$id": "#/properties/components",

within top-level:

specification/schema/bom-1.3.schema.json

Line 52 in c710388

"$id": "#/properties/components",
within "component":

specification/schema/bom-1.3.schema.json

Line 398 in c710388

"$id": "#/properties/components",

Duplicate "$id": "#/properties/services",

top-level:

specification/schema/bom-1.3.schema.json

Line 59 in c710388

"$id": "#/properties/services",
within "service":

specification/schema/bom-1.3.schema.json

Line 921 in c710388

"$id": "#/properties/services",

Notes/Observation

Unsure of use case for declaring an $id in these fragments (i.e., why do these $id" declarations exist at all)
Cut/paste error (copied JSON fragment from top-level to "tools"?

for example, these are the only "definitions" that have an "$id" (again $ref" usage in call cases is fine):

        "hashes": {
          "$id": "#/tools/properties/hashes",
          "type": "array",
          "items": {"$ref": "#/definitions/hash"},
          "title": "Hashes",
          "description": "The hashes of the tool (if applicable)."
        }

        "hashes": {
          "$id": "#/externalReference/properties/hashes",
          "type": "array",
          "items": {"$ref": "#/definitions/hash"},
          "title": "Hashes",
          "description": "The hashes of the external reference (if applicable)."
        }

        "components": {
          "$id": "#/component/properties/components",
          "type": "array",
          "items": {"$ref": "#/definitions/component"},
          "uniqueItems": true,
          "title": "Components"
        },

        "services": {
          "$id": "#service/properties/services",
          "type": "array",
          "items": {"$ref": "#service/definitions/service"},
          "uniqueItems": true,
          "title": "Services"
        },

The text was updated successfully, but these errors were encountered:

jkowalleck · 2022-01-18T23:40:12Z

Thanks for the detailed report, @mrutkows .

Do you see similarities to #83?
Do you see any relations to #84?

How do you see the removal of the all the $îds in schema that happened in CycloneDX spec transition from version 1.3 to 1.4?
is it making things better? making things worse?

How would you describe an ideal solution?
Do you see any risks with the solution you would propose?
Are there any side effects to consider?

mrutkows · 2022-01-19T00:17:15Z

@jkowalleck

Do you see similarities to #83? Do you see any relations to #84?

If #83 and #84 were combined (with subsequent discussions included) and netted out eventually identify all 3 $id duplication are identified (but still do not call out all 6 specific $id usages that get called into question) in order to have a comprehensive discussion...

Discussion in #83 identifies the duplicate $id values for the 2 appearances of "hashes", but does not include "tools" or "services". I see #83 is closed and seems to point to v1.4 for a fix?
Discussion in #84 identifies the duplicate $id values for the 2 appearances in "tools" and "services". It also seems to have a PR that deletes all $id declarations.

This issue summarizes the problem along with a comprehensive list of all occurrences of duplicate $id usage (along with line numbers and snippets).

There are 2 distinct cases that I call out here:

"components" and "services" $id values are declared within top-level bom "properties" (and would be correct if a use case exists for them) which means you only need to change the 2nd occurrence (to add a path). This might look like:

        "components": {
          "$id": "#/component/properties/components",
          "type": "array",
          "items": {"$ref": "#/definitions/component"},
          "uniqueItems": true,
          "title": "Components"
        },

        "services": {
          "$id": "#service/properties/services",
          "type": "array",
          "items": {"$ref": "#/definitions/service"},
          "uniqueItems": true,
          "title": "Services"
        },

In the case of "hashes", both $id declarations are not top-level, but their values indicate they are top-level "properties" and likely both need an additional relative path added to their URI fragments to include the parent schema to provide uniqueness; for example:

        "hashes": {
          "$id": "#/tools/properties/hashes",
          "type": "array",
          "items": {"$ref": "#/definitions/hash"},
          "title": "Hashes",
          "description": "The hashes of the tool (if applicable)."
        }

        "hashes": {
          "$id": "#/externalReference/properties/hashes",
          "type": "array",
          "items": {"$ref": "#/definitions/hash"},
          "title": "Hashes",
          "description": "The hashes of the external reference (if applicable)."
        }

Again, with the proper use of $ref (in each of these cases) you could simply delete the $id declaration; however, it then begs the question of removing all $id declarations in the top-level "properties" (and perhaps what lead us to v1.4 current state).

As to fixing in v1.4 (e.g., as #83 indicates perhaps)... I would say also that (IMO) it is important for discussing a possible solution within the v1.3 schema revisions (assuming some documents "finalized" and will always declare "1.3" as their version and cannot simply re-declare for 1.4)

mrutkows · 2022-01-19T00:47:31Z

Ideally, my approach would be to remove the problematic $ids in favor of $ref; specifically:

Case 1: remove the (duplicate) second occurrence of the $id declaration (since there is a valid $ref) in the case of "components" and "services" (i.e., case 1 above). This results in:

        "components": {
          "type": "array",
          "items": {"$ref": "#/definitions/component"},
          "uniqueItems": true,
          "title": "Components"
        },

        "services": {
          "type": "array",
          "items": {"$ref": "#/definitions/service"},
          "uniqueItems": true,
          "title": "Services"
        },

Case 2: remove the $id reference from both "hashes" object declarations as both are incorrect in their paths decleared on their URI fragments .

Do you see any risks with the solution you would propose?
Are there any side effects to consider?

Risks...
Any external reference that might be "working" today are likely working "by accident" of their tooling implementation and likely getting different "JSON pointers" depending on manner of parsing/searching identifiers (e.g., HashMap would overwrite entry with last $id parsed) or "fast" (streaming) parsers would short-circuit on first match (or others simply not find the fragment at all as is the case with browsers like Mozilla but not indicate he condition as an error).

My primary concern is leaving in top-level $id declarations in case they (for some unknown/historically discussed use case) are being referenced directly by an external schema (which by my recent reading is possible, but typically not encouraged) to not break external schemas.

If you want further risk reduction...

Preserve all $ids, but fix all paths as shown in my previous comment (this is what I did locally in a local schema copy my Go code uses as a minimal fix). This does still beg the use case as to when $id values would be used of course. Happy to discuss my thoughts...

stevespringett · 2022-01-20T04:34:51Z

@jkowalleck Does #84 address all the necessary changes required to fix the 1.2 and 1.3 JSON schemas? If not, would it be possible to do so?

jkowalleck · 2022-01-20T08:42:00Z

@mrutkows
could you pull-request the "fixed" files, so we could spot your recommended changes easily?

#123 (comment)

Preserve all $ids, but fix all paths as shown in my previous comment (this is what I did locally in a local schema copy my Go code uses as a minimal fix). This does still beg the use case as to when $id values would be used of course. Happy to discuss my thoughts...

jkowalleck · 2022-01-20T08:45:29Z

@jkowalleck Does #84 address all the necessary changes required to fix the 1.2 and 1.3 JSON schemas? If not, would it be possible to do so?

@stevespringett basically, yes. BUT
i want to see how @mrutkows fixed the issues, and compare the solutions. There are multiple options to fix/address the issue - and considering the risks/backward-compatibility one solution might be "better" than others.

jkowalleck · 2022-01-22T22:08:50Z

my rule of thumb for fixing: if a $id is just the same value a JSON pointer has, we could drop the $id without any risk
my point of view: the wrong $ids are not needed, as long as JSON pointers are usable - so instead of renaming the $id we could simply drop them.
This is the way how #84 was prepared.

Therefore i would prefer #84 over #125. Maybe i am not seeing something.

@mrutkows Do you see a benefit in #125 over #84?

regarding #125 I have some questions, @mrutkows :

do you see any benefit in adding new / fixing $id values? they don't help anybody at the moment, because they are new and not used anywhere, yet. The elements can be accessed via JSON pointers, so a $id is not needed.
Furthermore the $ids you introduced are the same values the JSON pointer has, are they? Don't they shadow the JSON pointer and therefore do not have any benefit ?
Do you see a benefit in keeping $ids that are the same value the JSON pointer has?

mrutkows · 2022-01-24T15:11:30Z

@jkowalleck Please see PR opened to show what could be a minimally invasive "fix" that reduces risk if adopted in the existing 1.3 release line. Again, I am in full support that with a new miner release, all $id usage beyond the base URI should be removed as I see is the case for current v1.4 schema.

jkowalleck · 2022-01-24T21:52:16Z

@stevespringett as far as i understand it, we have two options ready to fix/address this issues:

Guess it is time for the working groups to make a decision if and how to address the topic...
And they might need to decide how "{bug}fixes" will affect already published schema versions (1.2.2? 1.2b? )

mrutkows · 2022-01-24T22:11:15Z

@stevespringett as far as i understand it, we have two options ready to fix/address this issues:

* [Preserve  keys, but fix potential JSON pointers to reflect actual DOM… #125](https://github.com/CycloneDX/specification/pull/125)

* [fixed JSON schema id  #84](https://github.com/CycloneDX/specification/pull/84)

@jkowalleck Please know I did not produce PR 125 to "compete" in an either/or manner with PR 84, but was only providing what I was asked for in terms of "minimally invasive" (least risk). Hope you have an informed discussion.

stevespringett · 2022-01-24T23:10:33Z

Thank you so much @mrutkows. A least risk approach is likely what the Core Working Group would move forward with. I'm going to get clarity on it this week from the other members. I'd like to push updates out which we've already agreed to do. That's being tracked in #124 .

jkowalleck · 2022-02-04T14:02:12Z

updated the proposed CI/CT in #110

stevespringett · 2022-03-06T23:53:37Z

Thanks for all the work on this. PR #125 has been merged that fixes schema versions 1.2 and 1.3 in a minimally invasive way. All official CycloneDX tools will be updated with newer versions of the schemas.

- CycloneDX/specification#123 - CycloneDX/specification#84 - CycloneDX/specification#125 Signed-off-by: Paul Horton <[email protected]>

stevespringett added the defect label Jan 20, 2022

stevespringett mentioned this issue Jan 20, 2022

Schema Maintenance Releases #124

Closed

jkowalleck mentioned this issue Jan 20, 2022

fixed JSON schema id #84

Closed

mrutkows mentioned this issue Jan 20, 2022

Preserve keys, but fix potential JSON pointers to reflect actual DOM… #125

Merged

jkowalleck mentioned this issue Feb 4, 2022

add GH-workflow: php ci #110

Merged

stevespringett closed this as completed Mar 6, 2022

jkowalleck mentioned this issue Mar 8, 2022

update bundled schema files CycloneDX/cyclonedx-python-lib#186

Closed

madpah mentioned this issue Apr 11, 2022

feat: Update CycloneDX Schemas to latest patch versions CycloneDX/cyclonedx-python-lib#207

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CycloneDX v1.3 JSON Schema - canonical (duplicate) `$id` errors #123

CycloneDX v1.3 JSON Schema - canonical (duplicate) `$id` errors #123

mrutkows commented Jan 18, 2022 •

edited

Loading

jkowalleck commented Jan 18, 2022

mrutkows commented Jan 19, 2022 •

edited

Loading

mrutkows commented Jan 19, 2022 •

edited

Loading

stevespringett commented Jan 20, 2022

jkowalleck commented Jan 20, 2022

jkowalleck commented Jan 20, 2022 •

edited

Loading

jkowalleck commented Jan 22, 2022 •

edited

Loading

mrutkows commented Jan 24, 2022

jkowalleck commented Jan 24, 2022

mrutkows commented Jan 24, 2022

stevespringett commented Jan 24, 2022

jkowalleck commented Feb 4, 2022

stevespringett commented Mar 6, 2022

CycloneDX v1.3 JSON Schema - canonical (duplicate) $id errors #123

CycloneDX v1.3 JSON Schema - canonical (duplicate) $id errors #123

Comments

mrutkows commented Jan 18, 2022 • edited Loading

General Problem

JSON Schema reference (applicable to problem space)

Reproducible in tools

Investigation

Base URI

Specific instances of problem

jkowalleck commented Jan 18, 2022

mrutkows commented Jan 19, 2022 • edited Loading

mrutkows commented Jan 19, 2022 • edited Loading

stevespringett commented Jan 20, 2022

jkowalleck commented Jan 20, 2022

jkowalleck commented Jan 20, 2022 • edited Loading

jkowalleck commented Jan 22, 2022 • edited Loading

mrutkows commented Jan 24, 2022

jkowalleck commented Jan 24, 2022

mrutkows commented Jan 24, 2022

stevespringett commented Jan 24, 2022

jkowalleck commented Feb 4, 2022

stevespringett commented Mar 6, 2022

CycloneDX v1.3 JSON Schema - canonical (duplicate) `$id` errors #123

CycloneDX v1.3 JSON Schema - canonical (duplicate) `$id` errors #123

mrutkows commented Jan 18, 2022 •

edited

Loading

mrutkows commented Jan 19, 2022 •

edited

Loading

mrutkows commented Jan 19, 2022 •

edited

Loading

jkowalleck commented Jan 20, 2022 •

edited

Loading

jkowalleck commented Jan 22, 2022 •

edited

Loading