Fail parsing if the required prefix isn't present for IDs

.github/workflows/build.yml

@@ -9,7 +9,7 @@ jobs:
       - uses: actions/checkout@v6
       - uses: actions/setup-go@v6
         with:
-          go-version: '1.18'
+          go-version: '1.22'
       - name: Run tests
         run: make test
       - name: Send coverage report to coveralls

spdx/v2/common/identifier.go

@@ -30,7 +30,11 @@ func (d *DocumentID) UnmarshalJSON(data []byte) error {
 	idStr := string(data)
 	idStr = strings.Trim(idStr, "\"")
 
-	*d = trimDocumentIdPrefix(idStr)
+	documentID, err := trimDocumentIdPrefix(idStr)
+	if err != nil {
+		return err
+	}
+	*d = documentID
 	return nil
 }
 
@@ -43,9 +47,14 @@ func prefixDocumentId(id DocumentID) string {
 	return val
 }
 
-// trimDocumentIdPrefix removes the DocumentRef- prefix from an document ID string
-func trimDocumentIdPrefix(id string) DocumentID {
-	return DocumentID(strings.TrimPrefix(id, documentRefPrefix))
+// trimDocumentIdPrefix removes the DocumentRef- prefix from an document ID string and
+// returns an error if the prefix isn't present or if the resulting ID is empty.
+func trimDocumentIdPrefix(id string) (DocumentID, error) {
+	idWithoutPrefix, found := strings.CutPrefix(id, documentRefPrefix)
+	if !found || idWithoutPrefix == "" {
+		return DocumentID(""), fmt.Errorf("failed to parse DocumentID: %s", id)
+	}
+	return DocumentID(idWithoutPrefix), nil
 }
 
 // ElementID represents the identifier string portion of an SPDX element
@@ -65,7 +74,11 @@ func (d *ElementID) UnmarshalJSON(data []byte) error {
 	idStr := string(data)
 	idStr = strings.Trim(idStr, "\"")
 
-	*d = trimElementIdPrefix(idStr)
+	elementID, err := trimElementIdPrefix(idStr)
+	if err != nil {
+		return err
+	}
+	*d = elementID
 	return nil
 }
 
@@ -78,9 +91,14 @@ func prefixElementId(id ElementID) string {
 	return val
 }
 
-// trimElementIdPrefix removes the SPDXRef- prefix from an element ID string
-func trimElementIdPrefix(id string) ElementID {
-	return ElementID(strings.TrimPrefix(id, spdxRefPrefix))
+// trimElementIdPrefix removes the SPDXRef- prefix from an element ID string and
+// returns an error if the prefix isn't present or if the resulting ID is empty.
+func trimElementIdPrefix(id string) (ElementID, error) {
+	idWithoutPrefix, found := strings.CutPrefix(id, spdxRefPrefix)
+	if !found || idWithoutPrefix == "" {
+		return ElementID(""), fmt.Errorf("failed to parse ElementID: %s", id)
+	}
+	return ElementID(idWithoutPrefix), nil
 }
 
 // DocElementID represents an SPDX element identifier that could be defined
@@ -138,15 +156,23 @@ func (d *DocElementID) UnmarshalJSON(data []byte) error {
 		// an SPDXRef can appear after a DocumentRef, separated by a colon
 		idFields = strings.SplitN(idStr, ":", 2)
 
-		d.DocumentRefID = trimDocumentIdPrefix(idFields[0])
+		documentRefID, err := trimDocumentIdPrefix(idFields[0])
+		if err != nil {
+			return err
+		}
+		d.DocumentRefID = documentRefID
 		if len(idFields) == 2 {
 			idStr = idFields[1]
 		} else {
 			return nil
 		}
 	}
 
-	d.ElementRefID = trimElementIdPrefix(idStr)
+	elementRefID, err := trimElementIdPrefix(idStr)
+	if err != nil {
+		return err
+	}
+	d.ElementRefID = elementRefID
 	return nil
 }
 

spdx/v2/common/identifier_test.go

@@ -102,20 +102,25 @@ func Test_DocElementIDDecoding(t *testing.T) {
 				DocumentRefID: "a-doc",
 			},
 		},
+		{
+			name:  "DocumentRef with empty suffix",
+			value: "DocumentRef-",
+			err:   true,
+		},
 		{
 			name:  "DocumentRefID:ElementRefID without spdxref prefix",
 			value: "DocumentRef-a-doc:some-id",
-			expected: DocElementID{
-				DocumentRefID: "a-doc",
-				ElementRefID:  "some-id",
-			},
+			err:   true,
+		},
+		{
+			name:  "DocumentRefID:ElementRefID with empty ElementRefID suffix",
+			value: "DocumentRef-a-doc:SPDXRef-",
+			err:   true,
 		},
 		{
 			name:  "without spdxref prefix",
 			value: "some-id-without-spdxref",
-			expected: DocElementID{
-				ElementRefID: "some-id-without-spdxref",
-			},
+			err:   true,
 		},
 		{
 			name:  "SpecialID NONE",
@@ -207,10 +212,15 @@ func Test_ElementIDDecoding(t *testing.T) {
 			value:    "SPDXRef-some-id",
 			expected: ElementID("some-id"),
 		},
+		{
+			name:  "empty id suffix",
+			value: "SPDXRef-",
+			err:   true,
+		},
 		{
 			name:  "without prefix",
 			value: "some-id-without-spdxref",
-			expected: ElementID("some-id-without-spdxref"),
+			err:   true,
 		},
 	}
 
@@ -298,10 +308,8 @@ func Test_ElementIDStructDecoding(t *testing.T) {
 		},
 		{
 			name:  "without prefix",
-			expected: typ{
-				Id: ElementID("some-id"),
-			},
 			value: `{"id":"some-id"}`,
+			err:   true,
 		},
 	}
 
@@ -378,10 +386,15 @@ func Test_DocumentIDDecoding(t *testing.T) {
 			value:    "DocumentRef-some-id",
 			expected: DocumentID("some-id"),
 		},
+		{
+			name:  "empty id suffix",
+			value: "DocumentRef-",
+			err:   true,
+		},
 		{
 			name:  "without prefix",
 			value: "some-id-without-documentref",
-			expected: DocumentID("some-id-without-documentref"),
+			err:   true,
 		},
 	}
 
@@ -467,12 +480,15 @@ func Test_DocumentIDStructDecoding(t *testing.T) {
 			},
 			value: `{"id":"DocumentRef-some-id"}`,
 		},
+		{
+			name:  "empty suffix",
+			value: `{"id":"DocumentRef-"}`,
+			err:   true,
+		},
 		{
 			name:  "without prefix",
-			expected: typ{
-				Id: DocumentID("some-id"),
-			},
 			value: `{"id":"some-id"}`,
+			err:   true,
 		},
 	}