From 096f25ed06950ecb530c7cfd75851b37780837e5 Mon Sep 17 00:00:00 2001 From: Souta Kawahara Date: Sat, 1 Nov 2025 16:19:10 +0900 Subject: [PATCH 1/5] feat: Add DocumentID type and replace string-based DocumentRefID (#268) Signed-off-by: Souta Kawahara --- spdx/v2/common/identifier.go | 67 ++++++++++++++++++++++++++++++------ 1 file changed, 57 insertions(+), 10 deletions(-) diff --git a/spdx/v2/common/identifier.go b/spdx/v2/common/identifier.go index 929e42fa..5e979e98 100644 --- a/spdx/v2/common/identifier.go +++ b/spdx/v2/common/identifier.go @@ -14,6 +14,52 @@ const ( documentRefPrefix = "DocumentRef-" ) +// DocumentID represents the identifier string portion of an SPDX document +// identifier. DocumentID should be used for reference to a SPDX document. +// DocumentIDs should NOT contain the mandatory 'DocumentRef-' portion. +type DocumentID string + +// MarshalJSON returns an DocumentRef- prefixed JSON string +func (d DocumentID) MarshalJSON() ([]byte, error) { + return marshal.JSON(prefixDocumentId(d)) +} + +// UnmarshalJSON validates DocumentRef- prefixes and removes them when processing DocumentIDs +func (d *DocumentID) UnmarshalJSON(data []byte) error { + // SPDX identifier will simply be a string + idStr := string(data) + idStr = strings.Trim(idStr, "\"") + + e, err := trimDocumentIdPrefix(idStr) + if err != nil { + return err + } + *d = e + return nil +} + +// prefixDocumentId adds the DocumentRef- prefix to an document ID if it does not have one +func prefixDocumentId(id DocumentID) string { + val := string(id) + if !strings.HasPrefix(val, spdxRefPrefix) { + return documentRefPrefix + val + } + return val +} + +// trimDocumentIdPrefix removes the DocumentRef- prefix from an document ID string or returns an error if it +// does not start with DocumentRef- +func trimDocumentIdPrefix(id string) (DocumentID, error) { + // handle DocumentRef- + idFields := strings.SplitN(id, documentRefPrefix, 2) + if len(idFields) != 2 { + return "", fmt.Errorf("failed to parse SPDX identifier '%s'", id) + } + + e := DocumentID(idFields[1]) + return e, nil +} + // ElementID represents the identifier string portion of an SPDX element // identifier. DocElementID should be used for any attributes which can // contain identifiers defined in a different SPDX document. @@ -76,7 +122,7 @@ func trimElementIdPrefix(id string) (ElementID, error) { // "NOASSERTION" for the right-hand side of Relationships. If SpecialID // is set, DocumentRefID and ElementRefID should be empty (and vice versa). type DocElementID struct { - DocumentRefID string + DocumentRefID DocumentID ElementRefID ElementID SpecialID string } @@ -85,8 +131,9 @@ type DocElementID struct { // This function is also used when marshalling to YAML func (d DocElementID) MarshalJSON() ([]byte, error) { if d.DocumentRefID != "" && d.ElementRefID != "" { - idStr := prefixElementId(d.ElementRefID) - return marshal.JSON(fmt.Sprintf("%s%s:%s", documentRefPrefix, d.DocumentRefID, idStr)) + dIdStr := prefixDocumentId(d.DocumentRefID) + eIdStr := prefixElementId(d.ElementRefID) + return marshal.JSON(fmt.Sprintf("%s:%s", dIdStr, eIdStr)) } else if d.ElementRefID != "" { return marshal.JSON(prefixElementId(d.ElementRefID)) } else if d.SpecialID != "" { @@ -112,13 +159,13 @@ func (d *DocElementID) UnmarshalJSON(data []byte) (err error) { var idFields []string // handle DocumentRef- if present if strings.HasPrefix(idStr, documentRefPrefix) { - // strip out the "DocumentRef-" so we can get the value - idFields = strings.SplitN(idStr, documentRefPrefix, 2) - idStr = idFields[1] - // an SPDXRef can appear after a DocumentRef, separated by a colon idFields = strings.SplitN(idStr, ":", 2) - d.DocumentRefID = idFields[0] + + d.DocumentRefID, err = trimDocumentIdPrefix(idFields[0]) + if err != nil { + return err + } if len(idFields) == 2 { idStr = idFields[1] @@ -139,7 +186,7 @@ func (d *DocElementID) UnmarshalJSON(data []byte) (err error) { // present document. func MakeDocElementID(docRef string, eltRef string) DocElementID { return DocElementID{ - DocumentRefID: docRef, + DocumentRefID: DocumentID(docRef), ElementRefID: ElementID(eltRef), } } @@ -168,7 +215,7 @@ func RenderDocElementID(deID DocElementID) string { } prefix := "" if deID.DocumentRefID != "" { - prefix = documentRefPrefix + deID.DocumentRefID + ":" + prefix = documentRefPrefix + string(deID.DocumentRefID) + ":" } return prefix + spdxRefPrefix + string(deID.ElementRefID) } From 24bc6eb11c187578f9cbd73d664a058b928a399b Mon Sep 17 00:00:00 2001 From: Souta Kawahara Date: Sat, 1 Nov 2025 16:22:05 +0900 Subject: [PATCH 2/5] fix: update utilities for DocumentID type change And fix broken tagvalue example (#267) Signed-off-by: Souta Kawahara --- .../sample-docs/tv/SPDXTagExample-v2.3.spdx | 2 +- spdx/v2/v2_1/document.go | 2 +- .../tagvalue/reader/parse_creation_info.go | 13 ++++++----- .../reader/parse_creation_info_test.go | 5 ++-- spdx/v2/v2_1/tagvalue/reader/util.go | 2 +- spdx/v2/v2_1/tagvalue/reader/util_test.go | 2 +- spdx/v2/v2_2/document.go | 2 +- spdx/v2/v2_2/example/example.go | 2 +- .../v2/v2_2/rdf/reader/parse_spdx_document.go | 2 +- spdx/v2/v2_2/rdf/reader/utils.go | 2 +- spdx/v2/v2_2/rdf/reader/utils_test.go | 2 +- .../tagvalue/reader/parse_creation_info.go | 23 ++++++++++--------- .../reader/parse_creation_info_test.go | 5 ++-- spdx/v2/v2_2/tagvalue/reader/util.go | 2 +- spdx/v2/v2_2/tagvalue/reader/util_test.go | 4 ++-- spdx/v2/v2_3/document.go | 2 +- spdx/v2/v2_3/example/example.go | 2 +- .../v2/v2_3/rdf/reader/parse_spdx_document.go | 2 +- spdx/v2/v2_3/rdf/reader/utils.go | 2 +- spdx/v2/v2_3/rdf/reader/utils_test.go | 2 +- .../tagvalue/reader/parse_creation_info.go | 23 ++++++++++--------- .../reader/parse_creation_info_test.go | 5 ++-- spdx/v2/v2_3/tagvalue/reader/util.go | 2 +- spdx/v2/v2_3/tagvalue/reader/util_test.go | 4 ++-- 24 files changed, 60 insertions(+), 54 deletions(-) diff --git a/examples/sample-docs/tv/SPDXTagExample-v2.3.spdx b/examples/sample-docs/tv/SPDXTagExample-v2.3.spdx index 8d4bb61a..5c32e830 100644 --- a/examples/sample-docs/tv/SPDXTagExample-v2.3.spdx +++ b/examples/sample-docs/tv/SPDXTagExample-v2.3.spdx @@ -3,7 +3,7 @@ DataLicense: CC0-1.0 SPDXID: SPDXRef-DOCUMENT DocumentName: SPDX-Tools-v2.0 DocumentNamespace: http://spdx.org/spdxdocs/spdx-example-444504E0-4F89-41D3-9A0C-0305E82C3301 -ExternalDocumentRef: DocumentRef-DocumentRef-spdx-tool-1.2 http://spdx.org/spdxdocs/spdx-tools-v1.2-3F2504E0-4F89-41D3-9A0C-0305E82C3301 SHA1:d6a770ba38583ed4bb4525bd96e50461655d2759 +ExternalDocumentRef: DocumentRef-spdx-tool-1.2 http://spdx.org/spdxdocs/spdx-tools-v1.2-3F2504E0-4F89-41D3-9A0C-0305E82C3301 SHA1:d6a770ba38583ed4bb4525bd96e50461655d2759 DocumentComment: This document was created using SPDX 2.0 using licenses from the web site. LicenseListVersion: 3.9 Creator: Tool: LicenseFind-1.0 diff --git a/spdx/v2/v2_1/document.go b/spdx/v2/v2_1/document.go index bd6b41d4..4d853083 100644 --- a/spdx/v2/v2_1/document.go +++ b/spdx/v2/v2_1/document.go @@ -18,7 +18,7 @@ type ExternalDocumentRef struct { // DocumentRefID is the ID string defined in the start of the // reference. It should _not_ contain the "DocumentRef-" part // of the mandatory ID string. - DocumentRefID string `json:"externalDocumentId"` + DocumentRefID common.DocumentID `json:"externalDocumentId"` // URI is the URI defined for the external document URI string `json:"spdxDocument"` diff --git a/spdx/v2/v2_1/tagvalue/reader/parse_creation_info.go b/spdx/v2/v2_1/tagvalue/reader/parse_creation_info.go index 14c315c0..9e3f8e9e 100644 --- a/spdx/v2/v2_1/tagvalue/reader/parse_creation_info.go +++ b/spdx/v2/v2_1/tagvalue/reader/parse_creation_info.go @@ -103,7 +103,7 @@ func (parser *tvParser) parsePairFromCreationInfo(tag string, value string) erro // ===== Helper functions ===== -func extractExternalDocumentReference(value string) (string, string, string, string, error) { +func extractExternalDocumentReference(value string) (common.DocumentID, string, string, string, error) { sp := strings.Split(value, " ") // remove any that are just whitespace keepSp := []string{} @@ -114,12 +114,13 @@ func extractExternalDocumentReference(value string) (string, string, string, str } } - var documentRefID, uri, alg, checksum string + var documentRefID common.DocumentID + var strDocRefID, uri, alg, checksum string // now, should have 4 items (or 3, if Alg and Checksum were joined) // and should be able to map them if len(keepSp) == 4 { - documentRefID = keepSp[0] + strDocRefID = keepSp[0] uri = keepSp[1] alg = keepSp[2] // check that colon is present for alg, and remove it @@ -129,7 +130,7 @@ func extractExternalDocumentReference(value string) (string, string, string, str alg = strings.TrimSuffix(alg, ":") checksum = keepSp[3] } else if len(keepSp) == 3 { - documentRefID = keepSp[0] + strDocRefID = keepSp[0] uri = keepSp[1] // split on colon into alg and checksum parts := strings.SplitN(keepSp[2], ":", 2) @@ -144,10 +145,10 @@ func extractExternalDocumentReference(value string) (string, string, string, str // additionally, we should be able to parse the first element as a // DocumentRef- ID string, and we should remove that prefix - if !strings.HasPrefix(documentRefID, "DocumentRef-") { + if !strings.HasPrefix(strDocRefID, "DocumentRef-") { return "", "", "", "", fmt.Errorf("expected first element to have DocumentRef- prefix") } - documentRefID = strings.TrimPrefix(documentRefID, "DocumentRef-") + documentRefID = common.DocumentID(strings.TrimPrefix(strDocRefID, "DocumentRef-")) if documentRefID == "" { return "", "", "", "", fmt.Errorf("document identifier has nothing after prefix") } diff --git a/spdx/v2/v2_1/tagvalue/reader/parse_creation_info_test.go b/spdx/v2/v2_1/tagvalue/reader/parse_creation_info_test.go index 94ab57f3..816a9881 100644 --- a/spdx/v2/v2_1/tagvalue/reader/parse_creation_info_test.go +++ b/spdx/v2/v2_1/tagvalue/reader/parse_creation_info_test.go @@ -5,6 +5,7 @@ import ( "testing" spdx "github.com/spdx/tools-golang/spdx/v2/v2_1" + "github.com/spdx/tools-golang/spdx/v2/common" ) // ===== Parser creation info state change tests ===== @@ -360,7 +361,7 @@ func TestParserCICreatesAnnotation(t *testing.T) { func TestCanExtractExternalDocumentReference(t *testing.T) { refstring := "DocumentRef-spdx-tool-1.2 http://spdx.org/spdxdocs/spdx-tools-v1.2-3F2504E0-4F89-41D3-9A0C-0305E82C3301 SHA1:d6a770ba38583ed4bb4525bd96e50461655d2759" - wantDocumentRefID := "spdx-tool-1.2" + wantDocumentRefID := common.DocumentID("spdx-tool-1.2") wantURI := "http://spdx.org/spdxdocs/spdx-tools-v1.2-3F2504E0-4F89-41D3-9A0C-0305E82C3301" wantAlg := "SHA1" wantChecksum := "d6a770ba38583ed4bb4525bd96e50461655d2759" @@ -385,7 +386,7 @@ func TestCanExtractExternalDocumentReference(t *testing.T) { func TestCanExtractExternalDocumentReferenceWithExtraWhitespace(t *testing.T) { refstring := " DocumentRef-spdx-tool-1.2 \t http://spdx.org/spdxdocs/spdx-tools-v1.2-3F2504E0-4F89-41D3-9A0C-0305E82C3301 \t SHA1: \t d6a770ba38583ed4bb4525bd96e50461655d2759" - wantDocumentRefID := "spdx-tool-1.2" + wantDocumentRefID := common.DocumentID("spdx-tool-1.2") wantURI := "http://spdx.org/spdxdocs/spdx-tools-v1.2-3F2504E0-4F89-41D3-9A0C-0305E82C3301" wantAlg := "SHA1" wantChecksum := "d6a770ba38583ed4bb4525bd96e50461655d2759" diff --git a/spdx/v2/v2_1/tagvalue/reader/util.go b/spdx/v2/v2_1/tagvalue/reader/util.go index 817660b8..28a29d61 100644 --- a/spdx/v2/v2_1/tagvalue/reader/util.go +++ b/spdx/v2/v2_1/tagvalue/reader/util.go @@ -70,7 +70,7 @@ func extractDocElementID(value string) (common.DocElementID, error) { } // we're good - return common.DocElementID{DocumentRefID: docRefID, ElementRefID: common.ElementID(eltRefID)}, nil + return common.DocElementID{DocumentRefID: common.DocumentID(docRefID), ElementRefID: common.ElementID(eltRefID)}, nil } // used to extract SPDXRef values only from an SPDX Identifier which can point diff --git a/spdx/v2/v2_1/tagvalue/reader/util_test.go b/spdx/v2/v2_1/tagvalue/reader/util_test.go index 6043bf9f..b0a3c111 100644 --- a/spdx/v2/v2_1/tagvalue/reader/util_test.go +++ b/spdx/v2/v2_1/tagvalue/reader/util_test.go @@ -58,7 +58,7 @@ func helperForExtractDocElementID(t *testing.T, tst string, wantErr bool, wantDo if err == nil && wantErr == true { t.Errorf("testing %v: expected non-nil error, got nil", tst) } - if deID.DocumentRefID != wantDoc { + if deID.DocumentRefID != common.DocumentID(wantDoc) { if wantDoc == "" { t.Errorf("testing %v: want empty string for DocumentRefID, got %v", tst, deID.DocumentRefID) } else { diff --git a/spdx/v2/v2_2/document.go b/spdx/v2/v2_2/document.go index 5fcd33aa..0d6678a7 100644 --- a/spdx/v2/v2_2/document.go +++ b/spdx/v2/v2_2/document.go @@ -21,7 +21,7 @@ type ExternalDocumentRef struct { // DocumentRefID is the ID string defined in the start of the // reference. It should _not_ contain the "DocumentRef-" part // of the mandatory ID string. - DocumentRefID string `json:"externalDocumentId"` + DocumentRefID common.DocumentID `json:"externalDocumentId"` // URI is the URI defined for the external document URI string `json:"spdxDocument"` diff --git a/spdx/v2/v2_2/example/example.go b/spdx/v2/v2_2/example/example.go index 501eb773..39bda9d3 100644 --- a/spdx/v2/v2_2/example/example.go +++ b/spdx/v2/v2_2/example/example.go @@ -41,7 +41,7 @@ var example = spdx.Document{ DocumentComment: "This document was created using SPDX 2.0 using licenses from the web site.", ExternalDocumentReferences: []spdx.ExternalDocumentRef{ { - DocumentRefID: "DocumentRef-spdx-tool-1.2", + DocumentRefID: "spdx-tool-1.2", URI: "http://spdx.org/spdxdocs/spdx-tools-v1.2-3F2504E0-4F89-41D3-9A0C-0305E82C3301", Checksum: common.Checksum{ Algorithm: common.SHA1, diff --git a/spdx/v2/v2_2/rdf/reader/parse_spdx_document.go b/spdx/v2/v2_2/rdf/reader/parse_spdx_document.go index 7f16cc28..7629a926 100644 --- a/spdx/v2/v2_2/rdf/reader/parse_spdx_document.go +++ b/spdx/v2/v2_2/rdf/reader/parse_spdx_document.go @@ -96,7 +96,7 @@ func (parser *rdfParser2_2) getExternalDocumentRefFromNode(node *gordfParser.Nod switch triple.Predicate.ID { case SPDX_EXTERNAL_DOCUMENT_ID: // cardinality: exactly 1 - edr.DocumentRefID = triple.Object.ID + edr.DocumentRefID = common.DocumentID(triple.Object.ID) case SPDX_SPDX_DOCUMENT: // cardinality: exactly 1 // assumption: "spdxDocument" property of an external document diff --git a/spdx/v2/v2_2/rdf/reader/utils.go b/spdx/v2/v2_2/rdf/reader/utils.go index aa68dfa7..25db1fcc 100644 --- a/spdx/v2/v2_2/rdf/reader/utils.go +++ b/spdx/v2/v2_2/rdf/reader/utils.go @@ -112,7 +112,7 @@ func ExtractDocElementID(value string) (common.DocElementID, error) { } // we're good - return common.DocElementID{DocumentRefID: docRefID, ElementRefID: common.ElementID(eltRefID)}, nil + return common.DocElementID{DocumentRefID: common.DocumentID(docRefID), ElementRefID: common.ElementID(eltRefID)}, nil } // used to extract SPDXRef values only from an SPDX Identifier which can point diff --git a/spdx/v2/v2_2/rdf/reader/utils_test.go b/spdx/v2/v2_2/rdf/reader/utils_test.go index 9d37a154..2a88b924 100644 --- a/spdx/v2/v2_2/rdf/reader/utils_test.go +++ b/spdx/v2/v2_2/rdf/reader/utils_test.go @@ -217,7 +217,7 @@ func helperForExtractDocElementID(t *testing.T, tst string, wantErr bool, wantDo if err == nil && wantErr == true { t.Errorf("testing %v: expected non-nil error, got nil", tst) } - if deID.DocumentRefID != wantDoc { + if deID.DocumentRefID != common.DocumentID(wantDoc) { if wantDoc == "" { t.Errorf("testing %v: want empty string for DocumentRefID, got %v", tst, deID.DocumentRefID) } else { diff --git a/spdx/v2/v2_2/tagvalue/reader/parse_creation_info.go b/spdx/v2/v2_2/tagvalue/reader/parse_creation_info.go index e6919a42..8820b494 100644 --- a/spdx/v2/v2_2/tagvalue/reader/parse_creation_info.go +++ b/spdx/v2/v2_2/tagvalue/reader/parse_creation_info.go @@ -106,7 +106,7 @@ func (parser *tvParser) parsePairFromCreationInfo(tag string, value string) erro // ===== Helper functions ===== -func extractExternalDocumentReference(value string) (string, string, string, string, error) { +func extractExternalDocumentReference(value string) (common.DocumentID, string, string, string, error) { sp := strings.Split(value, " ") // remove any that are just whitespace keepSp := []string{} @@ -117,42 +117,43 @@ func extractExternalDocumentReference(value string) (string, string, string, str } } - var documentRefID, uri, alg, checksum string + var documentRefID common.DocumentID + var strDocRefID, uri, alg, checksum string // now, should have 4 items (or 3, if Alg and Checksum were joined) // and should be able to map them if len(keepSp) == 4 { - documentRefID = keepSp[0] + strDocRefID = keepSp[0] uri = keepSp[1] alg = keepSp[2] // check that colon is present for alg, and remove it if !strings.HasSuffix(alg, ":") { - return "", "", "", "", fmt.Errorf("algorithm does not end with colon") + return common.DocumentID(""), "", "", "", fmt.Errorf("algorithm does not end with colon") } alg = strings.TrimSuffix(alg, ":") checksum = keepSp[3] } else if len(keepSp) == 3 { - documentRefID = keepSp[0] + strDocRefID = keepSp[0] uri = keepSp[1] // split on colon into alg and checksum parts := strings.SplitN(keepSp[2], ":", 2) if len(parts) != 2 { - return "", "", "", "", fmt.Errorf("missing colon separator between algorithm and checksum") + return common.DocumentID(""), "", "", "", fmt.Errorf("missing colon separator between algorithm and checksum") } alg = parts[0] checksum = parts[1] } else { - return "", "", "", "", fmt.Errorf("expected 4 elements, got %d", len(keepSp)) + return common.DocumentID(""), "", "", "", fmt.Errorf("expected 4 elements, got %d", len(keepSp)) } // additionally, we should be able to parse the first element as a // DocumentRef- ID string, and we should remove that prefix - if !strings.HasPrefix(documentRefID, "DocumentRef-") { - return "", "", "", "", fmt.Errorf("expected first element to have DocumentRef- prefix") + if !strings.HasPrefix(strDocRefID, "DocumentRef-") { + return common.DocumentID(""), "", "", "", fmt.Errorf("expected first element to have DocumentRef- prefix") } - documentRefID = strings.TrimPrefix(documentRefID, "DocumentRef-") + documentRefID = common.DocumentID(strings.TrimPrefix(strDocRefID, "DocumentRef-")) if documentRefID == "" { - return "", "", "", "", fmt.Errorf("document identifier has nothing after prefix") + return common.DocumentID(""), "", "", "", fmt.Errorf("document identifier has nothing after prefix") } return documentRefID, uri, alg, checksum, nil diff --git a/spdx/v2/v2_2/tagvalue/reader/parse_creation_info_test.go b/spdx/v2/v2_2/tagvalue/reader/parse_creation_info_test.go index 6f8db7da..4f38aeab 100644 --- a/spdx/v2/v2_2/tagvalue/reader/parse_creation_info_test.go +++ b/spdx/v2/v2_2/tagvalue/reader/parse_creation_info_test.go @@ -5,6 +5,7 @@ import ( "testing" spdx "github.com/spdx/tools-golang/spdx/v2/v2_2" + "github.com/spdx/tools-golang/spdx/v2/common" ) // ===== Parser creation info state change tests ===== @@ -372,7 +373,7 @@ func TestParserCICreatesAnnotation(t *testing.T) { func TestCanExtractExternalDocumentReference(t *testing.T) { refstring := "DocumentRef-spdx-tool-1.2 http://spdx.org/spdxdocs/spdx-tools-v1.2-3F2504E0-4F89-41D3-9A0C-0305E82C3301 SHA1:d6a770ba38583ed4bb4525bd96e50461655d2759" - wantDocumentRefID := "spdx-tool-1.2" + wantDocumentRefID := common.DocumentID("spdx-tool-1.2") wantURI := "http://spdx.org/spdxdocs/spdx-tools-v1.2-3F2504E0-4F89-41D3-9A0C-0305E82C3301" wantAlg := "SHA1" wantChecksum := "d6a770ba38583ed4bb4525bd96e50461655d2759" @@ -397,7 +398,7 @@ func TestCanExtractExternalDocumentReference(t *testing.T) { func TestCanExtractExternalDocumentReferenceWithExtraWhitespace(t *testing.T) { refstring := " DocumentRef-spdx-tool-1.2 \t http://spdx.org/spdxdocs/spdx-tools-v1.2-3F2504E0-4F89-41D3-9A0C-0305E82C3301 \t SHA1: \t d6a770ba38583ed4bb4525bd96e50461655d2759" - wantDocumentRefID := "spdx-tool-1.2" + wantDocumentRefID := common.DocumentID("spdx-tool-1.2") wantURI := "http://spdx.org/spdxdocs/spdx-tools-v1.2-3F2504E0-4F89-41D3-9A0C-0305E82C3301" wantAlg := "SHA1" wantChecksum := "d6a770ba38583ed4bb4525bd96e50461655d2759" diff --git a/spdx/v2/v2_2/tagvalue/reader/util.go b/spdx/v2/v2_2/tagvalue/reader/util.go index 31daafc7..2538693b 100644 --- a/spdx/v2/v2_2/tagvalue/reader/util.go +++ b/spdx/v2/v2_2/tagvalue/reader/util.go @@ -70,7 +70,7 @@ func extractDocElementID(value string) (common.DocElementID, error) { } // we're good - return common.DocElementID{DocumentRefID: docRefID, ElementRefID: common.ElementID(eltRefID)}, nil + return common.DocElementID{DocumentRefID: common.DocumentID(docRefID), ElementRefID: common.ElementID(eltRefID)}, nil } // used to extract SPDXRef values from an SPDX Identifier, OR "special" strings diff --git a/spdx/v2/v2_2/tagvalue/reader/util_test.go b/spdx/v2/v2_2/tagvalue/reader/util_test.go index 83895075..452b24b4 100644 --- a/spdx/v2/v2_2/tagvalue/reader/util_test.go +++ b/spdx/v2/v2_2/tagvalue/reader/util_test.go @@ -58,7 +58,7 @@ func helperForExtractDocElementID(t *testing.T, tst string, wantErr bool, wantDo if err == nil && wantErr == true { t.Errorf("testing %v: expected non-nil error, got nil", tst) } - if deID.DocumentRefID != wantDoc { + if deID.DocumentRefID != common.DocumentID(wantDoc) { if wantDoc == "" { t.Errorf("testing %v: want empty string for DocumentRefID, got %v", tst, deID.DocumentRefID) } else { @@ -96,7 +96,7 @@ func helperForExtractDocElementSpecial(t *testing.T, permittedSpecial []string, if err == nil && wantErr == true { t.Errorf("testing %v: expected non-nil error, got nil", tst) } - if deID.DocumentRefID != wantDoc { + if deID.DocumentRefID != common.DocumentID(wantDoc) { if wantDoc == "" { t.Errorf("testing %v: want empty string for DocumentRefID, got %v", tst, deID.DocumentRefID) } else { diff --git a/spdx/v2/v2_3/document.go b/spdx/v2/v2_3/document.go index 62b7619b..a3e8ab8b 100644 --- a/spdx/v2/v2_3/document.go +++ b/spdx/v2/v2_3/document.go @@ -20,7 +20,7 @@ type ExternalDocumentRef struct { // DocumentRefID is the ID string defined in the start of the // reference. It should _not_ contain the "DocumentRef-" part // of the mandatory ID string. - DocumentRefID string `json:"externalDocumentId"` + DocumentRefID common.DocumentID `json:"externalDocumentId"` // URI is the URI defined for the external document URI string `json:"spdxDocument"` diff --git a/spdx/v2/v2_3/example/example.go b/spdx/v2/v2_3/example/example.go index e426d876..ab9f766c 100644 --- a/spdx/v2/v2_3/example/example.go +++ b/spdx/v2/v2_3/example/example.go @@ -41,7 +41,7 @@ var example = spdx.Document{ DocumentComment: "This document was created using SPDX 2.0 using licenses from the web site.", ExternalDocumentReferences: []spdx.ExternalDocumentRef{ { - DocumentRefID: "DocumentRef-spdx-tool-1.2", + DocumentRefID: "spdx-tool-1.2", URI: "http://spdx.org/spdxdocs/spdx-tools-v1.2-3F2504E0-4F89-41D3-9A0C-0305E82C3301", Checksum: common.Checksum{ Algorithm: common.SHA1, diff --git a/spdx/v2/v2_3/rdf/reader/parse_spdx_document.go b/spdx/v2/v2_3/rdf/reader/parse_spdx_document.go index 4b3cf1aa..7601c4d0 100644 --- a/spdx/v2/v2_3/rdf/reader/parse_spdx_document.go +++ b/spdx/v2/v2_3/rdf/reader/parse_spdx_document.go @@ -96,7 +96,7 @@ func (parser *rdfParser2_3) getExternalDocumentRefFromNode(node *gordfParser.Nod switch triple.Predicate.ID { case SPDX_EXTERNAL_DOCUMENT_ID: // cardinality: exactly 1 - edr.DocumentRefID = triple.Object.ID + edr.DocumentRefID = common.DocumentID(triple.Object.ID) case SPDX_SPDX_DOCUMENT: // cardinality: exactly 1 // assumption: "spdxDocument" property of an external document diff --git a/spdx/v2/v2_3/rdf/reader/utils.go b/spdx/v2/v2_3/rdf/reader/utils.go index 793f6fc5..a2425898 100644 --- a/spdx/v2/v2_3/rdf/reader/utils.go +++ b/spdx/v2/v2_3/rdf/reader/utils.go @@ -112,7 +112,7 @@ func ExtractDocElementID(value string) (common.DocElementID, error) { } // we're good - return common.DocElementID{DocumentRefID: docRefID, ElementRefID: common.ElementID(eltRefID)}, nil + return common.DocElementID{DocumentRefID: common.DocumentID(docRefID), ElementRefID: common.ElementID(eltRefID)}, nil } // used to extract SPDXRef values only from an SPDX Identifier which can point diff --git a/spdx/v2/v2_3/rdf/reader/utils_test.go b/spdx/v2/v2_3/rdf/reader/utils_test.go index bbc85e81..3f49a7a8 100644 --- a/spdx/v2/v2_3/rdf/reader/utils_test.go +++ b/spdx/v2/v2_3/rdf/reader/utils_test.go @@ -217,7 +217,7 @@ func helperForExtractDocElementID(t *testing.T, tst string, wantErr bool, wantDo if err == nil && wantErr == true { t.Errorf("testing %v: expected non-nil error, got nil", tst) } - if deID.DocumentRefID != wantDoc { + if deID.DocumentRefID != common.DocumentID(wantDoc) { if wantDoc == "" { t.Errorf("testing %v: want empty string for DocumentRefID, got %v", tst, deID.DocumentRefID) } else { diff --git a/spdx/v2/v2_3/tagvalue/reader/parse_creation_info.go b/spdx/v2/v2_3/tagvalue/reader/parse_creation_info.go index 58dce8c7..4736d0fc 100644 --- a/spdx/v2/v2_3/tagvalue/reader/parse_creation_info.go +++ b/spdx/v2/v2_3/tagvalue/reader/parse_creation_info.go @@ -106,7 +106,7 @@ func (parser *tvParser) parsePairFromCreationInfo(tag string, value string) erro // ===== Helper functions ===== -func extractExternalDocumentReference(value string) (string, string, string, string, error) { +func extractExternalDocumentReference(value string) (common.DocumentID, string, string, string, error) { sp := strings.Split(value, " ") // remove any that are just whitespace keepSp := []string{} @@ -117,42 +117,43 @@ func extractExternalDocumentReference(value string) (string, string, string, str } } - var documentRefID, uri, alg, checksum string + var documentRefID common.DocumentID + var strDocRefID, uri, alg, checksum string // now, should have 4 items (or 3, if Alg and Checksum were joined) // and should be able to map them if len(keepSp) == 4 { - documentRefID = keepSp[0] + strDocRefID = keepSp[0] uri = keepSp[1] alg = keepSp[2] // check that colon is present for alg, and remove it if !strings.HasSuffix(alg, ":") { - return "", "", "", "", fmt.Errorf("algorithm does not end with colon") + return common.DocumentID(""), "", "", "", fmt.Errorf("algorithm does not end with colon") } alg = strings.TrimSuffix(alg, ":") checksum = keepSp[3] } else if len(keepSp) == 3 { - documentRefID = keepSp[0] + strDocRefID = keepSp[0] uri = keepSp[1] // split on colon into alg and checksum parts := strings.SplitN(keepSp[2], ":", 2) if len(parts) != 2 { - return "", "", "", "", fmt.Errorf("missing colon separator between algorithm and checksum") + return common.DocumentID(""), "", "", "", fmt.Errorf("missing colon separator between algorithm and checksum") } alg = parts[0] checksum = parts[1] } else { - return "", "", "", "", fmt.Errorf("expected 4 elements, got %d", len(keepSp)) + return common.DocumentID(""), "", "", "", fmt.Errorf("expected 4 elements, got %d", len(keepSp)) } // additionally, we should be able to parse the first element as a // DocumentRef- ID string, and we should remove that prefix - if !strings.HasPrefix(documentRefID, "DocumentRef-") { - return "", "", "", "", fmt.Errorf("expected first element to have DocumentRef- prefix") + if !strings.HasPrefix(strDocRefID, "DocumentRef-") { + return common.DocumentID(""), "", "", "", fmt.Errorf("expected first element to have DocumentRef- prefix") } - documentRefID = strings.TrimPrefix(documentRefID, "DocumentRef-") + documentRefID = common.DocumentID(strings.TrimPrefix(strDocRefID, "DocumentRef-")) if documentRefID == "" { - return "", "", "", "", fmt.Errorf("document identifier has nothing after prefix") + return common.DocumentID(""), "", "", "", fmt.Errorf("document identifier has nothing after prefix") } return documentRefID, uri, alg, checksum, nil diff --git a/spdx/v2/v2_3/tagvalue/reader/parse_creation_info_test.go b/spdx/v2/v2_3/tagvalue/reader/parse_creation_info_test.go index 1ce43a56..404c4045 100644 --- a/spdx/v2/v2_3/tagvalue/reader/parse_creation_info_test.go +++ b/spdx/v2/v2_3/tagvalue/reader/parse_creation_info_test.go @@ -5,6 +5,7 @@ import ( "testing" spdx "github.com/spdx/tools-golang/spdx/v2/v2_3" + "github.com/spdx/tools-golang/spdx/v2/common" ) // ===== Parser creation info state change tests ===== @@ -360,7 +361,7 @@ func TestParserCICreatesAnnotation(t *testing.T) { func TestCanExtractExternalDocumentReference(t *testing.T) { refstring := "DocumentRef-spdx-tool-1.2 http://spdx.org/spdxdocs/spdx-tools-v1.2-3F2504E0-4F89-41D3-9A0C-0305E82C3301 SHA1:d6a770ba38583ed4bb4525bd96e50461655d2759" - wantDocumentRefID := "spdx-tool-1.2" + wantDocumentRefID := common.DocumentID("spdx-tool-1.2") wantURI := "http://spdx.org/spdxdocs/spdx-tools-v1.2-3F2504E0-4F89-41D3-9A0C-0305E82C3301" wantAlg := "SHA1" wantChecksum := "d6a770ba38583ed4bb4525bd96e50461655d2759" @@ -385,7 +386,7 @@ func TestCanExtractExternalDocumentReference(t *testing.T) { func TestCanExtractExternalDocumentReferenceWithExtraWhitespace(t *testing.T) { refstring := " DocumentRef-spdx-tool-1.2 \t http://spdx.org/spdxdocs/spdx-tools-v1.2-3F2504E0-4F89-41D3-9A0C-0305E82C3301 \t SHA1: \t d6a770ba38583ed4bb4525bd96e50461655d2759" - wantDocumentRefID := "spdx-tool-1.2" + wantDocumentRefID := common.DocumentID("spdx-tool-1.2") wantURI := "http://spdx.org/spdxdocs/spdx-tools-v1.2-3F2504E0-4F89-41D3-9A0C-0305E82C3301" wantAlg := "SHA1" wantChecksum := "d6a770ba38583ed4bb4525bd96e50461655d2759" diff --git a/spdx/v2/v2_3/tagvalue/reader/util.go b/spdx/v2/v2_3/tagvalue/reader/util.go index 027ff4b0..c7cfee6e 100644 --- a/spdx/v2/v2_3/tagvalue/reader/util.go +++ b/spdx/v2/v2_3/tagvalue/reader/util.go @@ -70,7 +70,7 @@ func extractDocElementID(value string) (common.DocElementID, error) { } // we're good - return common.DocElementID{DocumentRefID: docRefID, ElementRefID: common.ElementID(eltRefID)}, nil + return common.DocElementID{DocumentRefID: common.DocumentID(docRefID), ElementRefID: common.ElementID(eltRefID)}, nil } // used to extract SPDXRef values from an SPDX Identifier, OR "special" strings diff --git a/spdx/v2/v2_3/tagvalue/reader/util_test.go b/spdx/v2/v2_3/tagvalue/reader/util_test.go index 83895075..452b24b4 100644 --- a/spdx/v2/v2_3/tagvalue/reader/util_test.go +++ b/spdx/v2/v2_3/tagvalue/reader/util_test.go @@ -58,7 +58,7 @@ func helperForExtractDocElementID(t *testing.T, tst string, wantErr bool, wantDo if err == nil && wantErr == true { t.Errorf("testing %v: expected non-nil error, got nil", tst) } - if deID.DocumentRefID != wantDoc { + if deID.DocumentRefID != common.DocumentID(wantDoc) { if wantDoc == "" { t.Errorf("testing %v: want empty string for DocumentRefID, got %v", tst, deID.DocumentRefID) } else { @@ -96,7 +96,7 @@ func helperForExtractDocElementSpecial(t *testing.T, permittedSpecial []string, if err == nil && wantErr == true { t.Errorf("testing %v: expected non-nil error, got nil", tst) } - if deID.DocumentRefID != wantDoc { + if deID.DocumentRefID != common.DocumentID(wantDoc) { if wantDoc == "" { t.Errorf("testing %v: want empty string for DocumentRefID, got %v", tst, deID.DocumentRefID) } else { From d31a3d6e5cd1ee6d347b774a608a58c4ca8cfdd4 Mon Sep 17 00:00:00 2001 From: Souta Kawahara Date: Sun, 2 Nov 2025 14:56:51 +0900 Subject: [PATCH 3/5] fix: Replace strings.SplitN() with strings.TrimPrefix() in trimDocumentIdPrefix() and trimElementIdPrefix() Signed-off-by: Souta Kawahara --- spdx/v2/common/identifier.go | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/spdx/v2/common/identifier.go b/spdx/v2/common/identifier.go index 5e979e98..db35044c 100644 --- a/spdx/v2/common/identifier.go +++ b/spdx/v2/common/identifier.go @@ -51,12 +51,10 @@ func prefixDocumentId(id DocumentID) string { // does not start with DocumentRef- func trimDocumentIdPrefix(id string) (DocumentID, error) { // handle DocumentRef- - idFields := strings.SplitN(id, documentRefPrefix, 2) - if len(idFields) != 2 { + if !strings.HasPrefix(id, documentRefPrefix) { return "", fmt.Errorf("failed to parse SPDX identifier '%s'", id) } - - e := DocumentID(idFields[1]) + e := DocumentID(strings.TrimPrefix(id, documentRefPrefix)) return e, nil } @@ -98,12 +96,10 @@ func prefixElementId(id ElementID) string { // does not start with SPDXRef- func trimElementIdPrefix(id string) (ElementID, error) { // handle SPDXRef- - idFields := strings.SplitN(id, spdxRefPrefix, 2) - if len(idFields) != 2 { + if !strings.HasPrefix(id, spdxRefPrefix) { return "", fmt.Errorf("failed to parse SPDX identifier '%s'", id) } - - e := ElementID(idFields[1]) + e := ElementID(strings.TrimPrefix(id, spdxRefPrefix)) return e, nil } From 6cdc30a1bb9a1e7d734e55aad1cbbbd6446af107 Mon Sep 17 00:00:00 2001 From: Souta Kawahara Date: Sun, 2 Nov 2025 15:34:32 +0900 Subject: [PATCH 4/5] fix: Keep return type as string for extractExternalDocumentReference() Signed-off-by: Souta Kawahara --- .../tagvalue/reader/parse_creation_info.go | 13 +++++------ .../reader/parse_creation_info_test.go | 5 ++-- spdx/v2/v2_1/tagvalue/reader/parser.go | 2 +- .../tagvalue/reader/parse_creation_info.go | 23 +++++++++---------- .../reader/parse_creation_info_test.go | 5 ++-- spdx/v2/v2_2/tagvalue/reader/parser.go | 2 +- .../tagvalue/reader/parse_creation_info.go | 23 +++++++++---------- .../reader/parse_creation_info_test.go | 5 ++-- spdx/v2/v2_3/tagvalue/reader/parser.go | 2 +- 9 files changed, 37 insertions(+), 43 deletions(-) diff --git a/spdx/v2/v2_1/tagvalue/reader/parse_creation_info.go b/spdx/v2/v2_1/tagvalue/reader/parse_creation_info.go index 9e3f8e9e..14c315c0 100644 --- a/spdx/v2/v2_1/tagvalue/reader/parse_creation_info.go +++ b/spdx/v2/v2_1/tagvalue/reader/parse_creation_info.go @@ -103,7 +103,7 @@ func (parser *tvParser) parsePairFromCreationInfo(tag string, value string) erro // ===== Helper functions ===== -func extractExternalDocumentReference(value string) (common.DocumentID, string, string, string, error) { +func extractExternalDocumentReference(value string) (string, string, string, string, error) { sp := strings.Split(value, " ") // remove any that are just whitespace keepSp := []string{} @@ -114,13 +114,12 @@ func extractExternalDocumentReference(value string) (common.DocumentID, string, } } - var documentRefID common.DocumentID - var strDocRefID, uri, alg, checksum string + var documentRefID, uri, alg, checksum string // now, should have 4 items (or 3, if Alg and Checksum were joined) // and should be able to map them if len(keepSp) == 4 { - strDocRefID = keepSp[0] + documentRefID = keepSp[0] uri = keepSp[1] alg = keepSp[2] // check that colon is present for alg, and remove it @@ -130,7 +129,7 @@ func extractExternalDocumentReference(value string) (common.DocumentID, string, alg = strings.TrimSuffix(alg, ":") checksum = keepSp[3] } else if len(keepSp) == 3 { - strDocRefID = keepSp[0] + documentRefID = keepSp[0] uri = keepSp[1] // split on colon into alg and checksum parts := strings.SplitN(keepSp[2], ":", 2) @@ -145,10 +144,10 @@ func extractExternalDocumentReference(value string) (common.DocumentID, string, // additionally, we should be able to parse the first element as a // DocumentRef- ID string, and we should remove that prefix - if !strings.HasPrefix(strDocRefID, "DocumentRef-") { + if !strings.HasPrefix(documentRefID, "DocumentRef-") { return "", "", "", "", fmt.Errorf("expected first element to have DocumentRef- prefix") } - documentRefID = common.DocumentID(strings.TrimPrefix(strDocRefID, "DocumentRef-")) + documentRefID = strings.TrimPrefix(documentRefID, "DocumentRef-") if documentRefID == "" { return "", "", "", "", fmt.Errorf("document identifier has nothing after prefix") } diff --git a/spdx/v2/v2_1/tagvalue/reader/parse_creation_info_test.go b/spdx/v2/v2_1/tagvalue/reader/parse_creation_info_test.go index 816a9881..94ab57f3 100644 --- a/spdx/v2/v2_1/tagvalue/reader/parse_creation_info_test.go +++ b/spdx/v2/v2_1/tagvalue/reader/parse_creation_info_test.go @@ -5,7 +5,6 @@ import ( "testing" spdx "github.com/spdx/tools-golang/spdx/v2/v2_1" - "github.com/spdx/tools-golang/spdx/v2/common" ) // ===== Parser creation info state change tests ===== @@ -361,7 +360,7 @@ func TestParserCICreatesAnnotation(t *testing.T) { func TestCanExtractExternalDocumentReference(t *testing.T) { refstring := "DocumentRef-spdx-tool-1.2 http://spdx.org/spdxdocs/spdx-tools-v1.2-3F2504E0-4F89-41D3-9A0C-0305E82C3301 SHA1:d6a770ba38583ed4bb4525bd96e50461655d2759" - wantDocumentRefID := common.DocumentID("spdx-tool-1.2") + wantDocumentRefID := "spdx-tool-1.2" wantURI := "http://spdx.org/spdxdocs/spdx-tools-v1.2-3F2504E0-4F89-41D3-9A0C-0305E82C3301" wantAlg := "SHA1" wantChecksum := "d6a770ba38583ed4bb4525bd96e50461655d2759" @@ -386,7 +385,7 @@ func TestCanExtractExternalDocumentReference(t *testing.T) { func TestCanExtractExternalDocumentReferenceWithExtraWhitespace(t *testing.T) { refstring := " DocumentRef-spdx-tool-1.2 \t http://spdx.org/spdxdocs/spdx-tools-v1.2-3F2504E0-4F89-41D3-9A0C-0305E82C3301 \t SHA1: \t d6a770ba38583ed4bb4525bd96e50461655d2759" - wantDocumentRefID := common.DocumentID("spdx-tool-1.2") + wantDocumentRefID := "spdx-tool-1.2" wantURI := "http://spdx.org/spdxdocs/spdx-tools-v1.2-3F2504E0-4F89-41D3-9A0C-0305E82C3301" wantAlg := "SHA1" wantChecksum := "d6a770ba38583ed4bb4525bd96e50461655d2759" diff --git a/spdx/v2/v2_1/tagvalue/reader/parser.go b/spdx/v2/v2_1/tagvalue/reader/parser.go index 744fb16d..d8073bd6 100644 --- a/spdx/v2/v2_1/tagvalue/reader/parser.go +++ b/spdx/v2/v2_1/tagvalue/reader/parser.go @@ -86,7 +86,7 @@ func (parser *tvParser) parsePairFromStart(tag string, value string) error { return err } edr := spdx.ExternalDocumentRef{ - DocumentRefID: documentRefID, + DocumentRefID: common.DocumentID(documentRefID), URI: uri, Checksum: common.Checksum{Algorithm: common.ChecksumAlgorithm(alg), Value: checksum}, } diff --git a/spdx/v2/v2_2/tagvalue/reader/parse_creation_info.go b/spdx/v2/v2_2/tagvalue/reader/parse_creation_info.go index 8820b494..e6919a42 100644 --- a/spdx/v2/v2_2/tagvalue/reader/parse_creation_info.go +++ b/spdx/v2/v2_2/tagvalue/reader/parse_creation_info.go @@ -106,7 +106,7 @@ func (parser *tvParser) parsePairFromCreationInfo(tag string, value string) erro // ===== Helper functions ===== -func extractExternalDocumentReference(value string) (common.DocumentID, string, string, string, error) { +func extractExternalDocumentReference(value string) (string, string, string, string, error) { sp := strings.Split(value, " ") // remove any that are just whitespace keepSp := []string{} @@ -117,43 +117,42 @@ func extractExternalDocumentReference(value string) (common.DocumentID, string, } } - var documentRefID common.DocumentID - var strDocRefID, uri, alg, checksum string + var documentRefID, uri, alg, checksum string // now, should have 4 items (or 3, if Alg and Checksum were joined) // and should be able to map them if len(keepSp) == 4 { - strDocRefID = keepSp[0] + documentRefID = keepSp[0] uri = keepSp[1] alg = keepSp[2] // check that colon is present for alg, and remove it if !strings.HasSuffix(alg, ":") { - return common.DocumentID(""), "", "", "", fmt.Errorf("algorithm does not end with colon") + return "", "", "", "", fmt.Errorf("algorithm does not end with colon") } alg = strings.TrimSuffix(alg, ":") checksum = keepSp[3] } else if len(keepSp) == 3 { - strDocRefID = keepSp[0] + documentRefID = keepSp[0] uri = keepSp[1] // split on colon into alg and checksum parts := strings.SplitN(keepSp[2], ":", 2) if len(parts) != 2 { - return common.DocumentID(""), "", "", "", fmt.Errorf("missing colon separator between algorithm and checksum") + return "", "", "", "", fmt.Errorf("missing colon separator between algorithm and checksum") } alg = parts[0] checksum = parts[1] } else { - return common.DocumentID(""), "", "", "", fmt.Errorf("expected 4 elements, got %d", len(keepSp)) + return "", "", "", "", fmt.Errorf("expected 4 elements, got %d", len(keepSp)) } // additionally, we should be able to parse the first element as a // DocumentRef- ID string, and we should remove that prefix - if !strings.HasPrefix(strDocRefID, "DocumentRef-") { - return common.DocumentID(""), "", "", "", fmt.Errorf("expected first element to have DocumentRef- prefix") + if !strings.HasPrefix(documentRefID, "DocumentRef-") { + return "", "", "", "", fmt.Errorf("expected first element to have DocumentRef- prefix") } - documentRefID = common.DocumentID(strings.TrimPrefix(strDocRefID, "DocumentRef-")) + documentRefID = strings.TrimPrefix(documentRefID, "DocumentRef-") if documentRefID == "" { - return common.DocumentID(""), "", "", "", fmt.Errorf("document identifier has nothing after prefix") + return "", "", "", "", fmt.Errorf("document identifier has nothing after prefix") } return documentRefID, uri, alg, checksum, nil diff --git a/spdx/v2/v2_2/tagvalue/reader/parse_creation_info_test.go b/spdx/v2/v2_2/tagvalue/reader/parse_creation_info_test.go index 4f38aeab..6f8db7da 100644 --- a/spdx/v2/v2_2/tagvalue/reader/parse_creation_info_test.go +++ b/spdx/v2/v2_2/tagvalue/reader/parse_creation_info_test.go @@ -5,7 +5,6 @@ import ( "testing" spdx "github.com/spdx/tools-golang/spdx/v2/v2_2" - "github.com/spdx/tools-golang/spdx/v2/common" ) // ===== Parser creation info state change tests ===== @@ -373,7 +372,7 @@ func TestParserCICreatesAnnotation(t *testing.T) { func TestCanExtractExternalDocumentReference(t *testing.T) { refstring := "DocumentRef-spdx-tool-1.2 http://spdx.org/spdxdocs/spdx-tools-v1.2-3F2504E0-4F89-41D3-9A0C-0305E82C3301 SHA1:d6a770ba38583ed4bb4525bd96e50461655d2759" - wantDocumentRefID := common.DocumentID("spdx-tool-1.2") + wantDocumentRefID := "spdx-tool-1.2" wantURI := "http://spdx.org/spdxdocs/spdx-tools-v1.2-3F2504E0-4F89-41D3-9A0C-0305E82C3301" wantAlg := "SHA1" wantChecksum := "d6a770ba38583ed4bb4525bd96e50461655d2759" @@ -398,7 +397,7 @@ func TestCanExtractExternalDocumentReference(t *testing.T) { func TestCanExtractExternalDocumentReferenceWithExtraWhitespace(t *testing.T) { refstring := " DocumentRef-spdx-tool-1.2 \t http://spdx.org/spdxdocs/spdx-tools-v1.2-3F2504E0-4F89-41D3-9A0C-0305E82C3301 \t SHA1: \t d6a770ba38583ed4bb4525bd96e50461655d2759" - wantDocumentRefID := common.DocumentID("spdx-tool-1.2") + wantDocumentRefID := "spdx-tool-1.2" wantURI := "http://spdx.org/spdxdocs/spdx-tools-v1.2-3F2504E0-4F89-41D3-9A0C-0305E82C3301" wantAlg := "SHA1" wantChecksum := "d6a770ba38583ed4bb4525bd96e50461655d2759" diff --git a/spdx/v2/v2_2/tagvalue/reader/parser.go b/spdx/v2/v2_2/tagvalue/reader/parser.go index c936258e..5caca6c1 100644 --- a/spdx/v2/v2_2/tagvalue/reader/parser.go +++ b/spdx/v2/v2_2/tagvalue/reader/parser.go @@ -85,7 +85,7 @@ func (parser *tvParser) parsePairFromStart(tag string, value string) error { return err } edr := spdx.ExternalDocumentRef{ - DocumentRefID: documentRefID, + DocumentRefID: common.DocumentID(documentRefID), URI: uri, Checksum: common.Checksum{Algorithm: common.ChecksumAlgorithm(alg), Value: checksum}, } diff --git a/spdx/v2/v2_3/tagvalue/reader/parse_creation_info.go b/spdx/v2/v2_3/tagvalue/reader/parse_creation_info.go index 4736d0fc..58dce8c7 100644 --- a/spdx/v2/v2_3/tagvalue/reader/parse_creation_info.go +++ b/spdx/v2/v2_3/tagvalue/reader/parse_creation_info.go @@ -106,7 +106,7 @@ func (parser *tvParser) parsePairFromCreationInfo(tag string, value string) erro // ===== Helper functions ===== -func extractExternalDocumentReference(value string) (common.DocumentID, string, string, string, error) { +func extractExternalDocumentReference(value string) (string, string, string, string, error) { sp := strings.Split(value, " ") // remove any that are just whitespace keepSp := []string{} @@ -117,43 +117,42 @@ func extractExternalDocumentReference(value string) (common.DocumentID, string, } } - var documentRefID common.DocumentID - var strDocRefID, uri, alg, checksum string + var documentRefID, uri, alg, checksum string // now, should have 4 items (or 3, if Alg and Checksum were joined) // and should be able to map them if len(keepSp) == 4 { - strDocRefID = keepSp[0] + documentRefID = keepSp[0] uri = keepSp[1] alg = keepSp[2] // check that colon is present for alg, and remove it if !strings.HasSuffix(alg, ":") { - return common.DocumentID(""), "", "", "", fmt.Errorf("algorithm does not end with colon") + return "", "", "", "", fmt.Errorf("algorithm does not end with colon") } alg = strings.TrimSuffix(alg, ":") checksum = keepSp[3] } else if len(keepSp) == 3 { - strDocRefID = keepSp[0] + documentRefID = keepSp[0] uri = keepSp[1] // split on colon into alg and checksum parts := strings.SplitN(keepSp[2], ":", 2) if len(parts) != 2 { - return common.DocumentID(""), "", "", "", fmt.Errorf("missing colon separator between algorithm and checksum") + return "", "", "", "", fmt.Errorf("missing colon separator between algorithm and checksum") } alg = parts[0] checksum = parts[1] } else { - return common.DocumentID(""), "", "", "", fmt.Errorf("expected 4 elements, got %d", len(keepSp)) + return "", "", "", "", fmt.Errorf("expected 4 elements, got %d", len(keepSp)) } // additionally, we should be able to parse the first element as a // DocumentRef- ID string, and we should remove that prefix - if !strings.HasPrefix(strDocRefID, "DocumentRef-") { - return common.DocumentID(""), "", "", "", fmt.Errorf("expected first element to have DocumentRef- prefix") + if !strings.HasPrefix(documentRefID, "DocumentRef-") { + return "", "", "", "", fmt.Errorf("expected first element to have DocumentRef- prefix") } - documentRefID = common.DocumentID(strings.TrimPrefix(strDocRefID, "DocumentRef-")) + documentRefID = strings.TrimPrefix(documentRefID, "DocumentRef-") if documentRefID == "" { - return common.DocumentID(""), "", "", "", fmt.Errorf("document identifier has nothing after prefix") + return "", "", "", "", fmt.Errorf("document identifier has nothing after prefix") } return documentRefID, uri, alg, checksum, nil diff --git a/spdx/v2/v2_3/tagvalue/reader/parse_creation_info_test.go b/spdx/v2/v2_3/tagvalue/reader/parse_creation_info_test.go index 404c4045..1ce43a56 100644 --- a/spdx/v2/v2_3/tagvalue/reader/parse_creation_info_test.go +++ b/spdx/v2/v2_3/tagvalue/reader/parse_creation_info_test.go @@ -5,7 +5,6 @@ import ( "testing" spdx "github.com/spdx/tools-golang/spdx/v2/v2_3" - "github.com/spdx/tools-golang/spdx/v2/common" ) // ===== Parser creation info state change tests ===== @@ -361,7 +360,7 @@ func TestParserCICreatesAnnotation(t *testing.T) { func TestCanExtractExternalDocumentReference(t *testing.T) { refstring := "DocumentRef-spdx-tool-1.2 http://spdx.org/spdxdocs/spdx-tools-v1.2-3F2504E0-4F89-41D3-9A0C-0305E82C3301 SHA1:d6a770ba38583ed4bb4525bd96e50461655d2759" - wantDocumentRefID := common.DocumentID("spdx-tool-1.2") + wantDocumentRefID := "spdx-tool-1.2" wantURI := "http://spdx.org/spdxdocs/spdx-tools-v1.2-3F2504E0-4F89-41D3-9A0C-0305E82C3301" wantAlg := "SHA1" wantChecksum := "d6a770ba38583ed4bb4525bd96e50461655d2759" @@ -386,7 +385,7 @@ func TestCanExtractExternalDocumentReference(t *testing.T) { func TestCanExtractExternalDocumentReferenceWithExtraWhitespace(t *testing.T) { refstring := " DocumentRef-spdx-tool-1.2 \t http://spdx.org/spdxdocs/spdx-tools-v1.2-3F2504E0-4F89-41D3-9A0C-0305E82C3301 \t SHA1: \t d6a770ba38583ed4bb4525bd96e50461655d2759" - wantDocumentRefID := common.DocumentID("spdx-tool-1.2") + wantDocumentRefID := "spdx-tool-1.2" wantURI := "http://spdx.org/spdxdocs/spdx-tools-v1.2-3F2504E0-4F89-41D3-9A0C-0305E82C3301" wantAlg := "SHA1" wantChecksum := "d6a770ba38583ed4bb4525bd96e50461655d2759" diff --git a/spdx/v2/v2_3/tagvalue/reader/parser.go b/spdx/v2/v2_3/tagvalue/reader/parser.go index b6da8d94..a616fdef 100644 --- a/spdx/v2/v2_3/tagvalue/reader/parser.go +++ b/spdx/v2/v2_3/tagvalue/reader/parser.go @@ -84,7 +84,7 @@ func (parser *tvParser) parsePairFromStart(tag string, value string) error { return err } edr := spdx.ExternalDocumentRef{ - DocumentRefID: documentRefID, + DocumentRefID: common.DocumentID(documentRefID), URI: uri, Checksum: common.Checksum{Algorithm: common.ChecksumAlgorithm(alg), Value: checksum}, } From 9c2c75d29141e288bc85272b16cb676061d8f9da Mon Sep 17 00:00:00 2001 From: Souta Kawahara Date: Tue, 4 Nov 2025 11:46:01 +0900 Subject: [PATCH 5/5] fix: Update trimDocumentIdPrefix() and trimElementIdPrefix() to handle IDs without prefix Signed-off-by: Souta Kawahara --- spdx/v2/common/identifier.go | 48 ++++++++----------------------- spdx/v2/common/identifier_test.go | 25 ++++++++++------ 2 files changed, 28 insertions(+), 45 deletions(-) diff --git a/spdx/v2/common/identifier.go b/spdx/v2/common/identifier.go index db35044c..73fcbb4e 100644 --- a/spdx/v2/common/identifier.go +++ b/spdx/v2/common/identifier.go @@ -30,11 +30,7 @@ func (d *DocumentID) UnmarshalJSON(data []byte) error { idStr := string(data) idStr = strings.Trim(idStr, "\"") - e, err := trimDocumentIdPrefix(idStr) - if err != nil { - return err - } - *d = e + *d = trimDocumentIdPrefix(idStr) return nil } @@ -47,15 +43,9 @@ func prefixDocumentId(id DocumentID) string { return val } -// trimDocumentIdPrefix removes the DocumentRef- prefix from an document ID string or returns an error if it -// does not start with DocumentRef- -func trimDocumentIdPrefix(id string) (DocumentID, error) { - // handle DocumentRef- - if !strings.HasPrefix(id, documentRefPrefix) { - return "", fmt.Errorf("failed to parse SPDX identifier '%s'", id) - } - e := DocumentID(strings.TrimPrefix(id, documentRefPrefix)) - return e, nil +// trimDocumentIdPrefix removes the DocumentRef- prefix from an document ID string +func trimDocumentIdPrefix(id string) DocumentID { + return DocumentID(strings.TrimPrefix(id, documentRefPrefix)) } // ElementID represents the identifier string portion of an SPDX element @@ -75,11 +65,7 @@ func (d *ElementID) UnmarshalJSON(data []byte) error { idStr := string(data) idStr = strings.Trim(idStr, "\"") - e, err := trimElementIdPrefix(idStr) - if err != nil { - return err - } - *d = e + *d = trimElementIdPrefix(idStr) return nil } @@ -92,15 +78,9 @@ func prefixElementId(id ElementID) string { return val } -// trimElementIdPrefix removes the SPDXRef- prefix from an element ID string or returns an error if it -// does not start with SPDXRef- -func trimElementIdPrefix(id string) (ElementID, error) { - // handle SPDXRef- - if !strings.HasPrefix(id, spdxRefPrefix) { - return "", fmt.Errorf("failed to parse SPDX identifier '%s'", id) - } - e := ElementID(strings.TrimPrefix(id, spdxRefPrefix)) - return e, nil +// trimElementIdPrefix removes the SPDXRef- prefix from an element ID string +func trimElementIdPrefix(id string) ElementID { + return ElementID(strings.TrimPrefix(id, spdxRefPrefix)) } // DocElementID represents an SPDX element identifier that could be defined @@ -141,7 +121,7 @@ func (d DocElementID) MarshalJSON() ([]byte, error) { // UnmarshalJSON takes a SPDX Identifier string parses it into a DocElementID struct. // This function is also used when unmarshalling YAML -func (d *DocElementID) UnmarshalJSON(data []byte) (err error) { +func (d *DocElementID) UnmarshalJSON(data []byte) error { // SPDX identifier will simply be a string idStr := string(data) idStr = strings.Trim(idStr, "\"") @@ -158,11 +138,7 @@ func (d *DocElementID) UnmarshalJSON(data []byte) (err error) { // an SPDXRef can appear after a DocumentRef, separated by a colon idFields = strings.SplitN(idStr, ":", 2) - d.DocumentRefID, err = trimDocumentIdPrefix(idFields[0]) - if err != nil { - return err - } - + d.DocumentRefID = trimDocumentIdPrefix(idFields[0]) if len(idFields) == 2 { idStr = idFields[1] } else { @@ -170,8 +146,8 @@ func (d *DocElementID) UnmarshalJSON(data []byte) (err error) { } } - d.ElementRefID, err = trimElementIdPrefix(idStr) - return err + d.ElementRefID = trimElementIdPrefix(idStr) + return nil } // TODO: add equivalents for LicenseRef- identifiers diff --git a/spdx/v2/common/identifier_test.go b/spdx/v2/common/identifier_test.go index 5adfd436..4c92ba58 100644 --- a/spdx/v2/common/identifier_test.go +++ b/spdx/v2/common/identifier_test.go @@ -103,14 +103,19 @@ func Test_DocElementIDDecoding(t *testing.T) { }, }, { - name: "DocumentRefID invalid ElementRefID", - value: "DocumentRef-a-doc:invalid", - err: true, + name: "DocumentRefID:ElementRefID without spdxref prefix", + value: "DocumentRef-a-doc:some-id", + expected: DocElementID{ + DocumentRefID: "a-doc", + ElementRefID: "some-id", + }, }, { - name: "invalid format", + name: "without spdxref prefix", value: "some-id-without-spdxref", - err: true, + expected: DocElementID{ + ElementRefID: "some-id-without-spdxref", + }, }, { name: "SpecialID NONE", @@ -203,9 +208,9 @@ func Test_ElementIDDecoding(t *testing.T) { expected: ElementID("some-id"), }, { - name: "invalid format", + name: "without prefix", value: "some-id-without-spdxref", - err: true, + expected: ElementID("some-id-without-spdxref"), }, } @@ -292,9 +297,11 @@ func Test_ElementIDStructDecoding(t *testing.T) { value: `{"id":"SPDXRef-some-id"}`, }, { - name: "invalid format", + name: "without prefix", + expected: typ{ + Id: ElementID("some-id"), + }, value: `{"id":"some-id"}`, - err: true, }, }