From 2f3dd372ecacd16a7dbc74940bdfde089a910f8a Mon Sep 17 00:00:00 2001 From: Keith Zantow Date: Mon, 7 Nov 2022 15:24:59 -0500 Subject: fix: JSON decoding not properly handling SPDXRef- prefixes Signed-off-by: Keith Zantow --- spdx/common/identifier.go | 98 +++++++++---- spdx/common/identifier_test.go | 314 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 383 insertions(+), 29 deletions(-) create mode 100644 spdx/common/identifier_test.go (limited to 'spdx/common') diff --git a/spdx/common/identifier.go b/spdx/common/identifier.go index d656867..806a815 100644 --- a/spdx/common/identifier.go +++ b/spdx/common/identifier.go @@ -8,12 +8,58 @@ import ( "strings" ) +const ( + spdxRefPrefix = "SPDXRef-" + documentRefPrefix = "DocumentRef-" +) + // 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. // ElementIDs should NOT contain the mandatory 'SPDXRef-' portion. type ElementID string +// MarshalJSON returns an SPDXRef- prefixed JSON string +func (d ElementID) MarshalJSON() ([]byte, error) { + return json.Marshal(prefixElementId(d)) +} + +// UnmarshalJSON validates SPDXRef- prefixes and removes them when processing ElementIDs +func (d *ElementID) UnmarshalJSON(data []byte) error { + // SPDX identifier will simply be a string + idStr := string(data) + idStr = strings.Trim(idStr, "\"") + + e, err := trimElementIdPrefix(idStr) + if err != nil { + return err + } + *d = e + return nil +} + +// prefixElementId adds the SPDXRef- prefix to an element ID if it does not have one +func prefixElementId(id ElementID) string { + val := string(id) + if !strings.HasPrefix(val, spdxRefPrefix) { + return spdxRefPrefix + val + } + 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- + idFields := strings.SplitN(id, spdxRefPrefix, 2) + if len(idFields) != 2 { + return "", fmt.Errorf("failed to parse SPDX identifier '%s'", id) + } + + e := ElementID(idFields[1]) + return e, nil +} + // DocElementID represents an SPDX element identifier that could be defined // in a different SPDX document, and therefore could have a "DocumentRef-" // portion, such as Relationships and Annotations. @@ -34,9 +80,24 @@ type DocElementID struct { SpecialID string } +// MarshalJSON converts the receiver into a slice of bytes representing a DocElementID in string form. +// 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 json.Marshal(fmt.Sprintf("%s%s:%s", documentRefPrefix, d.DocumentRefID, idStr)) + } else if d.ElementRefID != "" { + return json.Marshal(prefixElementId(d.ElementRefID)) + } else if d.SpecialID != "" { + return json.Marshal(d.SpecialID) + } + + return []byte{}, fmt.Errorf("failed to marshal empty DocElementID") +} + // 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) error { +func (d *DocElementID) UnmarshalJSON(data []byte) (err error) { // SPDX identifier will simply be a string idStr := string(data) idStr = strings.Trim(idStr, "\"") @@ -49,9 +110,9 @@ func (d *DocElementID) UnmarshalJSON(data []byte) error { var idFields []string // handle DocumentRef- if present - if strings.HasPrefix(idStr, "DocumentRef-") { + if strings.HasPrefix(idStr, documentRefPrefix) { // strip out the "DocumentRef-" so we can get the value - idFields = strings.SplitN(idStr, "DocumentRef-", 2) + idFields = strings.SplitN(idStr, documentRefPrefix, 2) idStr = idFields[1] // an SPDXRef can appear after a DocumentRef, separated by a colon @@ -65,29 +126,8 @@ func (d *DocElementID) UnmarshalJSON(data []byte) error { } } - // handle SPDXRef- - idFields = strings.SplitN(idStr, "SPDXRef-", 2) - if len(idFields) != 2 { - return fmt.Errorf("failed to parse SPDX Identifier '%s'", idStr) - } - - d.ElementRefID = ElementID(idFields[1]) - - return nil -} - -// MarshalJSON converts the receiver into a slice of bytes representing a DocElementID in string form. -// This function is also used when marshalling to YAML -func (d DocElementID) MarshalJSON() ([]byte, error) { - if d.DocumentRefID != "" && d.ElementRefID != "" { - return json.Marshal(fmt.Sprintf("DocumentRef-%s:SPDXRef-%s", d.DocumentRefID, d.ElementRefID)) - } else if d.ElementRefID != "" { - return json.Marshal(fmt.Sprintf("SPDXRef-%s", d.ElementRefID)) - } else if d.SpecialID != "" { - return json.Marshal(d.SpecialID) - } - - return []byte{}, fmt.Errorf("failed to marshal empty DocElementID") + d.ElementRefID, err = trimElementIdPrefix(idStr) + return err } // TODO: add equivalents for LicenseRef- identifiers @@ -114,7 +154,7 @@ func MakeDocElementSpecial(specialID string) DocElementID { // RenderElementID takes an ElementID and returns the string equivalent, // with the SPDXRef- prefix reinserted. func RenderElementID(eID ElementID) string { - return "SPDXRef-" + string(eID) + return spdxRefPrefix + string(eID) } // RenderDocElementID takes a DocElementID and returns the string equivalent, @@ -127,7 +167,7 @@ func RenderDocElementID(deID DocElementID) string { } prefix := "" if deID.DocumentRefID != "" { - prefix = "DocumentRef-" + deID.DocumentRefID + ":" + prefix = documentRefPrefix + deID.DocumentRefID + ":" } - return prefix + "SPDXRef-" + string(deID.ElementRefID) + return prefix + spdxRefPrefix + string(deID.ElementRefID) } diff --git a/spdx/common/identifier_test.go b/spdx/common/identifier_test.go new file mode 100644 index 0000000..8df2ea3 --- /dev/null +++ b/spdx/common/identifier_test.go @@ -0,0 +1,314 @@ +package common + +import ( + "encoding/json" + "fmt" + "reflect" + "strings" + "testing" +) + +func Test_DocElementIDEncoding(t *testing.T) { + tests := []struct { + name string + value DocElementID + expected string + err bool + }{ + { + name: "ElementRefID", + value: DocElementID{ + ElementRefID: "some-id", + }, + expected: "SPDXRef-some-id", + }, + { + name: "DocumentRefID:ElementRefID", + value: DocElementID{ + DocumentRefID: "a-doc", + ElementRefID: "some-id", + }, + expected: "DocumentRef-a-doc:SPDXRef-some-id", + }, + { + name: "DocumentRefID no ElementRefID", + value: DocElementID{ + DocumentRefID: "a-doc", + }, + err: true, + }, + { + name: "SpecialID", + value: DocElementID{ + SpecialID: "special-id", + }, + expected: "special-id", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + result, err := json.Marshal(test.value) + switch { + case !test.err && err != nil: + t.Fatalf("unexpected error: %v", err) + case test.err && err == nil: + t.Fatalf("expected error but got none") + case test.err: + return + } + s := string(result) + if !strings.HasPrefix(s, `"`) || !strings.HasSuffix(s, `"`) { + t.Fatalf("string was not returned: %s", s) + } + s = strings.Trim(s, `"`) + if test.expected != s { + t.Fatalf("%s != %s", test.expected, s) + } + }) + } +} + +func Test_DocElementIDDecoding(t *testing.T) { + tests := []struct { + name string + value string + expected DocElementID + err bool + }{ + { + name: "ElementRefID", + value: "SPDXRef-some-id", + expected: DocElementID{ + ElementRefID: "some-id", + }, + }, + { + name: "DocumentRefID:ElementRefID", + value: "DocumentRef-a-doc:SPDXRef-some-id", + expected: DocElementID{ + DocumentRefID: "a-doc", + ElementRefID: "some-id", + }, + }, + { + name: "DocumentRefID no ElementRefID", + value: "DocumentRef-a-doc", + expected: DocElementID{ + DocumentRefID: "a-doc", + }, + }, + { + name: "DocumentRefID invalid ElementRefID", + value: "DocumentRef-a-doc:invalid", + err: true, + }, + { + name: "invalid format", + value: "some-id-without-spdxref", + err: true, + }, + { + name: "SpecialID NONE", + value: "NONE", + expected: DocElementID{ + SpecialID: "NONE", + }, + }, + { + name: "SpecialID NOASSERTION", + value: "NOASSERTION", + expected: DocElementID{ + SpecialID: "NOASSERTION", + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + out := DocElementID{} + s := fmt.Sprintf(`"%s"`, test.value) + err := json.Unmarshal([]byte(s), &out) + switch { + case !test.err && err != nil: + t.Fatalf("unexpected error: %v", err) + case test.err && err == nil: + t.Fatalf("expected error but got none") + case test.err: + return + } + if !reflect.DeepEqual(test.expected, out) { + t.Fatalf("unexpected value: %v != %v", test.expected, out) + } + }) + } +} + +func Test_ElementIDEncoding(t *testing.T) { + tests := []struct { + name string + value ElementID + expected string + err bool + }{ + { + name: "appends spdxref", + value: ElementID("some-id"), + expected: "SPDXRef-some-id", + }, + { + name: "appends spdxref", + value: ElementID("SPDXRef-some-id"), + expected: "SPDXRef-some-id", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + result, err := json.Marshal(test.value) + switch { + case !test.err && err != nil: + t.Fatalf("unexpected error: %v", err) + case test.err && err == nil: + t.Fatalf("expected error but got none") + case test.err: + return + } + s := string(result) + if !strings.HasPrefix(s, `"`) || !strings.HasSuffix(s, `"`) { + t.Fatalf("string was not returned: %s", s) + } + s = strings.Trim(s, `"`) + if test.expected != s { + t.Fatalf("%s != %s", test.expected, s) + } + }) + } +} + +func Test_ElementIDDecoding(t *testing.T) { + tests := []struct { + name string + value string + expected ElementID + err bool + }{ + { + name: "valid id", + value: "SPDXRef-some-id", + expected: ElementID("some-id"), + }, + { + name: "invalid format", + value: "some-id-without-spdxref", + err: true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + var out ElementID + s := fmt.Sprintf(`"%s"`, test.value) + err := json.Unmarshal([]byte(s), &out) + switch { + case !test.err && err != nil: + t.Fatalf("unexpected error: %v", err) + case test.err && err == nil: + t.Fatalf("expected error but got none") + case test.err: + return + } + if !reflect.DeepEqual(test.expected, out) { + t.Fatalf("unexpected value: %v != %v", test.expected, out) + } + }) + } +} + +func Test_ElementIDStructEncoding(t *testing.T) { + type typ struct { + Id ElementID `json:"id"` + } + tests := []struct { + name string + value typ + expected string + err bool + }{ + { + name: "appends spdxref", + value: typ{ + Id: ElementID("some-id"), + }, + expected: `{"id":"SPDXRef-some-id"}`, + }, + { + name: "appends spdxref", + value: typ{ + Id: ElementID("SPDXRef-some-id"), + }, + expected: `{"id":"SPDXRef-some-id"}`, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + result, err := json.Marshal(test.value) + switch { + case !test.err && err != nil: + t.Fatalf("unexpected error: %v", err) + case test.err && err == nil: + t.Fatalf("expected error but got none") + case test.err: + return + } + s := string(result) + if test.expected != s { + t.Fatalf("%s != %s", test.expected, s) + } + }) + } +} + +func Test_ElementIDStructDecoding(t *testing.T) { + type typ struct { + Id ElementID `json:"id"` + } + tests := []struct { + name string + value string + expected typ + err bool + }{ + { + name: "valid id", + expected: typ{ + Id: ElementID("some-id"), + }, + value: `{"id":"SPDXRef-some-id"}`, + }, + { + name: "invalid format", + value: `{"id":"some-id"}`, + err: true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + out := typ{} + err := json.Unmarshal([]byte(test.value), &out) + switch { + case !test.err && err != nil: + t.Fatalf("unexpected error: %v", err) + case test.err && err == nil: + t.Fatalf("expected error but got none") + case test.err: + return + } + if !reflect.DeepEqual(test.expected, out) { + t.Fatalf("unexpected value: %v != %v", test.expected, out) + } + }) + } +} -- cgit v1.2.3