From 6234aa66fed08ee03a2be31ec177152cefeedfd2 Mon Sep 17 00:00:00 2001 From: Steve Winslow Date: Sat, 20 Mar 2021 17:16:31 -0400 Subject: [PATCH] Fix special IDs for right-side 2.2 Relationships In SPDX 2.2, the right-hand side of Relationships are not limited to SPDX IDs; they can also include the special values NONE and NOASSERTION. To handle these, since Golang doesn't (to my knowledge) have a concept of union types, and since I don't want to use interface{}, this commit instead adds a new SpecialID field to DocElementID. When SpecialID is non-empty, it should be treated as being a "special" ID value, and DocumentRefID / ElementRefID should be ignored. (Unfortunately, we can't just use ElementRefID == "NONE", etc. for this purpose, because in theory an SPDX document could define the identifier SPDXRef-NONE to mean something. Even though they really, really shouldn't do that.) This commit updates tvloader and tvsaver to appropriately handle the possibility of NONE and NOASSERTION for this field. Signed-off-by: Steve Winslow --- spdx/identifier.go | 19 ++++++- tvloader/parser2v2/parse_relationship.go | 4 +- tvloader/parser2v2/parse_relationship_test.go | 31 ++++++++++++ tvloader/parser2v2/util.go | 17 +++++++ tvloader/parser2v2/util_test.go | 47 ++++++++++++++++- tvsaver/saver2v2/save_relationship_test.go | 50 +++++++++++++++++++ 6 files changed, 165 insertions(+), 3 deletions(-) diff --git a/spdx/identifier.go b/spdx/identifier.go index 496aeb3e..baf44c1c 100644 --- a/spdx/identifier.go +++ b/spdx/identifier.go @@ -18,9 +18,14 @@ type ElementID string // present document. // DocElementIDs should NOT contain the mandatory 'DocumentRef-' or // 'SPDXRef-' portions. +// SpecialID is used ONLY if the DocElementID matches a defined set of +// permitted special values for a particular field, e.g. "NONE" or +// "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 ElementRefID ElementID + SpecialID string } // TODO: add equivalents for LicenseRef- identifiers @@ -36,6 +41,14 @@ func MakeDocElementID(docRef string, eltRef string) DocElementID { } } +// MakeDocElementSpecial takes a "special" string (e.g. "NONE" or +// "NOASSERTION" for the right side of a Relationship), nd returns +// a DocElementID with it in the SpecialID field. Other fields will +// be empty. +func MakeDocElementSpecial(specialID string) DocElementID { + return DocElementID{SpecialID: specialID} +} + // RenderElementID takes an ElementID and returns the string equivalent, // with the SPDXRef- prefix reinserted. func RenderElementID(eID ElementID) string { @@ -44,8 +57,12 @@ func RenderElementID(eID ElementID) string { // RenderDocElementID takes a DocElementID and returns the string equivalent, // with the SPDXRef- prefix (and, if applicable, the DocumentRef- prefix) -// reinserted. +// reinserted. If a SpecialID is present, it will be rendered verbatim and +// DocumentRefID and ElementRefID will be ignored. func RenderDocElementID(deID DocElementID) string { + if deID.SpecialID != "" { + return deID.SpecialID + } prefix := "" if deID.DocumentRefID != "" { prefix = "DocumentRef-" + deID.DocumentRefID + ":" diff --git a/tvloader/parser2v2/parse_relationship.go b/tvloader/parser2v2/parse_relationship.go index 28e5b728..092b554c 100644 --- a/tvloader/parser2v2/parse_relationship.go +++ b/tvloader/parser2v2/parse_relationship.go @@ -35,7 +35,9 @@ func (parser *tvParser2_2) parsePairForRelationship2_2(tag string, value string) } parser.rln.RefA = aID parser.rln.Relationship = strings.TrimSpace(rp[1]) - bID, err := extractDocElementID(strings.TrimSpace(rp[2])) + // NONE and NOASSERTION are permitted on right side + permittedSpecial := []string{"NONE", "NOASSERTION"} + bID, err := extractDocElementSpecial(strings.TrimSpace(rp[2]), permittedSpecial) if err != nil { return err } diff --git a/tvloader/parser2v2/parse_relationship_test.go b/tvloader/parser2v2/parse_relationship_test.go index 1312cc55..0e6c013e 100644 --- a/tvloader/parser2v2/parse_relationship_test.go +++ b/tvloader/parser2v2/parse_relationship_test.go @@ -152,6 +152,37 @@ func TestParser2_2InvalidRelationshipTagsInvalidRefIDs(t *testing.T) { } } +func TestParser2_2SpecialValuesValidForRightSideOfRelationship(t *testing.T) { + parser := tvParser2_2{ + doc: &spdx.Document2_2{}, + st: psCreationInfo2_2, + } + + // NONE in right side of relationship should pass + err := parser.parsePair2_2("Relationship", "SPDXRef-a CONTAINS NONE") + if err != nil { + t.Errorf("expected nil error for CONTAINS NONE, got %v", err) + } + + // NOASSERTION in right side of relationship should pass + err = parser.parsePair2_2("Relationship", "SPDXRef-a CONTAINS NOASSERTION") + if err != nil { + t.Errorf("expected nil error for CONTAINS NOASSERTION, got %v", err) + } + + // NONE in left side of relationship should fail + err = parser.parsePair2_2("Relationship", "NONE CONTAINS SPDXRef-a") + if err == nil { + t.Errorf("expected non-nil error for NONE CONTAINS, got nil") + } + + // NOASSERTION in left side of relationship should fail + err = parser.parsePair2_2("Relationship", "NOASSERTION CONTAINS SPDXRef-a") + if err == nil { + t.Errorf("expected non-nil error for NOASSERTION CONTAINS, got nil") + } +} + func TestParser2_2FailsToParseUnknownTagInRelationshipSection(t *testing.T) { parser := tvParser2_2{ doc: &spdx.Document2_2{}, diff --git a/tvloader/parser2v2/util.go b/tvloader/parser2v2/util.go index 691fe4eb..66768469 100644 --- a/tvloader/parser2v2/util.go +++ b/tvloader/parser2v2/util.go @@ -73,6 +73,23 @@ func extractDocElementID(value string) (spdx.DocElementID, error) { return spdx.DocElementID{DocumentRefID: docRefID, ElementRefID: spdx.ElementID(eltRefID)}, nil } +// used to extract SPDXRef values from an SPDX Identifier, OR "special" strings +// from a specified set of permitted values. The primary use case for this is +// the right-hand side of Relationships, where beginning in SPDX 2.2 the values +// "NONE" and "NOASSERTION" are permitted. If the value does not match one of +// the specified permitted values, it will fall back to the ordinary +// DocElementID extractor. +func extractDocElementSpecial(value string, permittedSpecial []string) (spdx.DocElementID, error) { + // check value against special set first + for _, sp := range permittedSpecial { + if sp == value { + return spdx.DocElementID{SpecialID: sp}, nil + } + } + // not found, fall back to regular search + return extractDocElementID(value) +} + // used to extract SPDXRef values only from an SPDX Identifier which can point // to this document only. Use extractDocElementID for parsing IDs that can // refer either to this document or a different one. diff --git a/tvloader/parser2v2/util_test.go b/tvloader/parser2v2/util_test.go index 9f63364e..e2f75d7b 100644 --- a/tvloader/parser2v2/util_test.go +++ b/tvloader/parser2v2/util_test.go @@ -67,13 +67,58 @@ func helperForExtractDocElementID(t *testing.T, tst string, wantErr bool, wantDo } if deID.ElementRefID != spdx.ElementID(wantElt) { if wantElt == "" { - t.Errorf("testing %v: want emptyString for ElementRefID, got %v", tst, deID.ElementRefID) + t.Errorf("testing %v: want empty string for ElementRefID, got %v", tst, deID.ElementRefID) } else { t.Errorf("testing %v: want %v for ElementRefID, got %v", tst, wantElt, deID.ElementRefID) } } } +func TestCanExtractSpecialDocumentIDs(t *testing.T) { + permittedSpecial := []string{"NONE", "NOASSERTION"} + // test with valid special values + helperForExtractDocElementSpecial(t, permittedSpecial, "NONE", false, "", "", "NONE") + helperForExtractDocElementSpecial(t, permittedSpecial, "NOASSERTION", false, "", "", "NOASSERTION") + // test with valid regular IDs + helperForExtractDocElementSpecial(t, permittedSpecial, "SPDXRef-file1", false, "", "file1", "") + helperForExtractDocElementSpecial(t, permittedSpecial, "DocumentRef-doc2:SPDXRef-file2", false, "doc2", "file2", "") + helperForExtractDocElementSpecial(t, permittedSpecial, "a:SPDXRef-file1", true, "", "", "") + helperForExtractDocElementSpecial(t, permittedSpecial, "DocumentRef-doc2", true, "", "", "") + // test with invalid other words not on permitted list + helperForExtractDocElementSpecial(t, permittedSpecial, "FOO", true, "", "", "") +} + +func helperForExtractDocElementSpecial(t *testing.T, permittedSpecial []string, tst string, wantErr bool, wantDoc string, wantElt string, wantSpecial string) { + deID, err := extractDocElementSpecial(tst, permittedSpecial) + if err != nil && wantErr == false { + t.Errorf("testing %v: expected nil error, got %v", tst, err) + } + if err == nil && wantErr == true { + t.Errorf("testing %v: expected non-nil error, got nil", tst) + } + if deID.DocumentRefID != wantDoc { + if wantDoc == "" { + t.Errorf("testing %v: want empty string for DocumentRefID, got %v", tst, deID.DocumentRefID) + } else { + t.Errorf("testing %v: want %v for DocumentRefID, got %v", tst, wantDoc, deID.DocumentRefID) + } + } + if deID.ElementRefID != spdx.ElementID(wantElt) { + if wantElt == "" { + t.Errorf("testing %v: want empty string for ElementRefID, got %v", tst, deID.ElementRefID) + } else { + t.Errorf("testing %v: want %v for ElementRefID, got %v", tst, wantElt, deID.ElementRefID) + } + } + if deID.SpecialID != wantSpecial { + if wantSpecial == "" { + t.Errorf("testing %v: want empty string for SpecialID, got %v", tst, deID.SpecialID) + } else { + t.Errorf("testing %v: want %v for SpecialID, got %v", tst, wantSpecial, deID.SpecialID) + } + } +} + func TestCanExtractElementRefsOnlyFromID(t *testing.T) { // test with valid ID in this document helperForExtractElementID(t, "SPDXRef-file1", false, "file1") diff --git a/tvsaver/saver2v2/save_relationship_test.go b/tvsaver/saver2v2/save_relationship_test.go index eecbb6c3..ab981842 100644 --- a/tvsaver/saver2v2/save_relationship_test.go +++ b/tvsaver/saver2v2/save_relationship_test.go @@ -62,3 +62,53 @@ func TestSaver2_2RelationshipOmitsOptionalFieldsIfEmpty(t *testing.T) { t.Errorf("Expected %v, got %v", want.String(), got.String()) } } + +func TestSaver2_2RelationshipCanHaveNONEOnRight(t *testing.T) { + rln := &spdx.Relationship2_2{ + RefA: spdx.MakeDocElementID("", "PackageA"), + RefB: spdx.MakeDocElementSpecial("NONE"), + Relationship: "DEPENDS_ON", + } + + // what we want to get, as a buffer of bytes + // no trailing blank newline + want := bytes.NewBufferString("Relationship: SPDXRef-PackageA DEPENDS_ON NONE\n") + + // render as buffer of bytes + var got bytes.Buffer + err := renderRelationship2_2(rln, &got) + if err != nil { + t.Errorf("Expected nil error, got %v", err) + } + + // check that they match + c := bytes.Compare(want.Bytes(), got.Bytes()) + if c != 0 { + t.Errorf("Expected %v, got %v", want.String(), got.String()) + } +} + +func TestSaver2_2RelationshipCanHaveNOASSERTIONOnRight(t *testing.T) { + rln := &spdx.Relationship2_2{ + RefA: spdx.MakeDocElementID("", "PackageA"), + RefB: spdx.MakeDocElementSpecial("NOASSERTION"), + Relationship: "DEPENDS_ON", + } + + // what we want to get, as a buffer of bytes + // no trailing blank newline + want := bytes.NewBufferString("Relationship: SPDXRef-PackageA DEPENDS_ON NOASSERTION\n") + + // render as buffer of bytes + var got bytes.Buffer + err := renderRelationship2_2(rln, &got) + if err != nil { + t.Errorf("Expected nil error, got %v", err) + } + + // check that they match + c := bytes.Compare(want.Bytes(), got.Bytes()) + if c != 0 { + t.Errorf("Expected %v, got %v", want.String(), got.String()) + } +}