Error on missing package and file ids in v2.2

- Shows error if a Package has a missing SPDXID tag
- Show error if a File has a missing SPDXID tag

Signed-off-by: specter25 <ujjwalcoding012@gmail.com>

7 files changed, 125 insertions, 1 deletions

diff --git a/tvloader/parser2v2/parse_creation_info.go b/tvloader/parser2v2/parse_creation_info.go
index a3c7fbf..48efc58 100644
--- a/tvloader/parser2v2/parse_creation_info.go
+++ b/tvloader/parser2v2/parse_creation_info.go
@@ -76,6 +76,10 @@ func (parser *tvParser2_2) parsePairFromCreationInfo2_2(tag string, value string
 
 	// tag for going on to package section
 	case "PackageName":
+		//before starting the parsing of packages check if the last unpackaged file did contain a SPDX file Identifier
+		if parser.file != nil && parser.file.FileSPDXIdentifier == spdx.ElementID("") {
+			return fmt.Errorf("Invalid file without a package SPDX identifier")
+		}
 		parser.st = psPackage2_2
 		parser.pkg = &spdx.Package2_2{
 			FilesAnalyzed:             true,
diff --git a/tvloader/parser2v2/parse_file.go b/tvloader/parser2v2/parse_file.go
index 86886d3..f6447fa 100644
--- a/tvloader/parser2v2/parse_file.go
+++ b/tvloader/parser2v2/parse_file.go
@@ -18,11 +18,19 @@ func (parser *tvParser2_2) parsePairFromFile2_2(tag string, value string) error
 	switch tag {
 	// tag for creating new file section
 	case "FileName":
+		//check if the previous file contained a spdxId or not
+		if parser.file != nil && parser.file.FileSPDXIdentifier == spdx.ElementID("") {
+			return fmt.Errorf("Invalid file without a file SPDX identifier")
+		}
 		parser.file = &spdx.File2_2{}
 		parser.file.FileName = value
 	// tag for creating new package section and going back to parsing Package
 	case "PackageName":
 		parser.st = psPackage2_2
+		//check if the previous file containes a spdxId or not
+		if parser.file != nil && parser.file.FileSPDXIdentifier == spdx.ElementID("") {
+			return fmt.Errorf("Invalid file without a file SPDX identifier")
+		}
 		parser.file = nil
 		return parser.parsePairFromPackage2_2(tag, value)
 	// tag for going on to snippet section
diff --git a/tvloader/parser2v2/parse_file_test.go b/tvloader/parser2v2/parse_file_test.go
index 411593f..3af06a1 100644
--- a/tvloader/parser2v2/parse_file_test.go
+++ b/tvloader/parser2v2/parse_file_test.go
@@ -5,6 +5,7 @@ import (
 	"testing"
 
 	"github.com/spdx/tools-golang/spdx"
+	"github.com/spdx/tools-golang/tvloader/reader"
 )
 
 // ===== Parser file section state change tests =====
@@ -917,3 +918,61 @@ func TestParser2_2FailsIfArtifactURIBeforeArtifactName(t *testing.T) {
 		t.Errorf("expected non-nil error, got nil")
 	}
 }
+
+func TestParser2_2FilesWithoutSpdxIdThrowError(t *testing.T) {
+	//case 1
+	// Last unpackaged file no packages in doc
+	// Last file of last package in the doc
+	var tvPairs []reader.TagValuePair
+	tvPair1 := reader.TagValuePair{Tag: "SPDXVersion", Value: "SPDX-2.2"}
+	tvPairs = append(tvPairs, tvPair1)
+	tvPair2 := reader.TagValuePair{Tag: "DataLicense", Value: "CC0-1.0"}
+	tvPairs = append(tvPairs, tvPair2)
+	tvPair3 := reader.TagValuePair{Tag: "SPDXID", Value: "SPDXRef-DOCUMENT"}
+	tvPairs = append(tvPairs, tvPair3)
+	tvPair4 := reader.TagValuePair{Tag: "FileName", Value: "f1"}
+	tvPairs = append(tvPairs, tvPair4)
+	_, err := ParseTagValues(tvPairs)
+	if err == nil {
+		t.Errorf("files withoutSpdx Identifiers getting accepted")
+	}
+
+	//case 2 : The previous file (packaged or unpackaged does not contain spdxID)
+	tvPair5 := reader.TagValuePair{Tag: "FileName", Value: "f2"}
+	tvPairs = append(tvPairs, tvPair5)
+	_, err = ParseTagValues(tvPairs)
+	if err == nil {
+		t.Errorf("%s", err)
+	}
+
+	//case 3 : Invalid file with snippet
+	//Last unpackaged file before the packges start
+	//Last file of a package and New package starts
+	sid1 := spdx.ElementID("s1")
+	parser := tvParser2_2{
+		doc: &spdx.Document2_2{},
+		st:  psCreationInfo2_2,
+	}
+	fileName := "f2.txt"
+	err = parser.parsePair2_2("FileName", fileName)
+	err = parser.parsePair2_2("SnippetSPDXID", string(sid1))
+	err = parser.parsePair2_2("PackageName", "p2")
+	if err == nil {
+		t.Errorf("files withoutSpdx Identifiers getting accepted")
+	}
+
+	//case 4 : Invalid File without snippets
+	//Last unpackaged file before the packges start
+	//Last file of a package and New package starts
+	parser3 := tvParser2_2{
+		doc: &spdx.Document2_2{},
+		st:  psCreationInfo2_2,
+	}
+	fileName = "f3.txt"
+	err = parser3.parsePair2_2("FileName", fileName)
+	err = parser3.parsePair2_2("PackageName", "p2")
+	if err == nil {
+		t.Errorf("files withoutSpdx Identifiers getting accepted")
+	}
+	err = parser3.parsePair2_2("PackageName", "p2")
+}
diff --git a/tvloader/parser2v2/parse_package.go b/tvloader/parser2v2/parse_package.go
index 3f5939b..729db18 100644
--- a/tvloader/parser2v2/parse_package.go
+++ b/tvloader/parser2v2/parse_package.go
@@ -20,6 +20,10 @@ func (parser *tvParser2_2) parsePairFromPackage2_2(tag string, value string) err
 	case "PackageName":
 		// if package already has a name, create and go on to a new package
 		if parser.pkg == nil || parser.pkg.PackageName != "" {
+			//check if the previous package contained a spdxId or not
+			if parser.pkg != nil && parser.pkg.PackageSPDXIdentifier == spdx.ElementID("") {
+				return fmt.Errorf("Invalid package without a package SPDX identifier")
+			}
 			parser.pkg = &spdx.Package2_2{
 				FilesAnalyzed:             true,
 				IsFilesAnalyzedTagPresent: false,
diff --git a/tvloader/parser2v2/parse_package_test.go b/tvloader/parser2v2/parse_package_test.go
index 33b4784..1983baf 100644
--- a/tvloader/parser2v2/parse_package_test.go
+++ b/tvloader/parser2v2/parse_package_test.go
@@ -5,6 +5,7 @@ import (
 	"testing"
 
 	"github.com/spdx/tools-golang/spdx"
+	"github.com/spdx/tools-golang/tvloader/reader"
 )
 
 // ===== Parser package section state change tests =====
@@ -1102,3 +1103,42 @@ func TestFailsPackageExternalRefWithInvalidFormat(t *testing.T) {
 		t.Errorf("expected non-nil error, got nil")
 	}
 }
+
+func TestParser2_2PackageWithoutSpdxIdentifierThrowsError(t *testing.T) {
+	// More than one package , the previous package doesn't contain the SPDXID
+	pkgOldName := "p1"
+	parser := tvParser2_2{
+		doc: &spdx.Document2_2{Packages: map[spdx.ElementID]*spdx.Package2_2{}},
+		st:  psPackage2_2,
+		pkg: &spdx.Package2_2{PackageName: pkgOldName},
+	}
+	pkgOld := parser.pkg
+	parser.doc.Packages["p1"] = pkgOld
+	// the Document's Packages should have this one only
+	if parser.doc.Packages["p1"] != pkgOld {
+		t.Errorf("expected package %v, got %v", pkgOld, parser.doc.Packages["p1"])
+	}
+	if len(parser.doc.Packages) != 1 {
+		t.Errorf("expected 1 package, got %d", len(parser.doc.Packages))
+	}
+
+	// Case 2: Checks the Last package
+	pkgName := "p2"
+	err := parser.parsePair2_2("PackageName", pkgName)
+	if err == nil {
+		t.Errorf("Packages withoutSpdx Identifiers getting accepted")
+	}
+	var tvPairs []reader.TagValuePair
+	tvPair1 := reader.TagValuePair{Tag: "SPDXVersion", Value: "SPDX-2.2"}
+	tvPairs = append(tvPairs, tvPair1)
+	tvPair2 := reader.TagValuePair{Tag: "DataLicense", Value: "CC0-1.0"}
+	tvPairs = append(tvPairs, tvPair2)
+	tvPair3 := reader.TagValuePair{Tag: "SPDXID", Value: "SPDXRef-DOCUMENT"}
+	tvPairs = append(tvPairs, tvPair3)
+	tvPair4 := reader.TagValuePair{Tag: "PackageName", Value: "p1"}
+	tvPairs = append(tvPairs, tvPair4)
+	_, err = ParseTagValues(tvPairs)
+	if err == nil {
+		t.Errorf("Packages withoutSpdx Identifiers getting accepted")
+	}
+}
diff --git a/tvloader/parser2v2/parse_snippet.go b/tvloader/parser2v2/parse_snippet.go
index 0d9dc3f..0de54e2 100644
--- a/tvloader/parser2v2/parse_snippet.go
+++ b/tvloader/parser2v2/parse_snippet.go
@@ -34,6 +34,10 @@ func (parser *tvParser2_2) parsePairFromSnippet2_2(tag string, value string) err
 	// tag for creating new package section and going back to parsing Package
 	case "PackageName":
 		parser.st = psPackage2_2
+		//check here whether the last file of the previous package contained the FileSpdxIdentifier
+		if parser.file != nil && parser.file.FileSPDXIdentifier == spdx.ElementID("") {
+			return fmt.Errorf("Invalid file without a file SPDX identifier")
+		}
 		parser.file = nil
 		parser.snippet = nil
 		return parser.parsePairFromPackage2_2(tag, value)
diff --git a/tvloader/parser2v2/parser.go b/tvloader/parser2v2/parser.go
index e89bab1..8216fbb 100644
--- a/tvloader/parser2v2/parser.go
+++ b/tvloader/parser2v2/parser.go
@@ -20,7 +20,12 @@ func ParseTagValues(tvs []reader.TagValuePair) (*spdx.Document2_2, error) {
 			return nil, err
 		}
 	}
-
+	if parser.file != nil && parser.file.FileSPDXIdentifier == spdx.ElementID("") {
+		return nil, fmt.Errorf("Invalid file without a file SPDX identifier")
+	}
+	if parser.pkg != nil && parser.pkg.PackageSPDXIdentifier == spdx.ElementID("") {
+		return nil, fmt.Errorf("Invalid package without a package SPDX identifier")
+	}
 	return parser.doc, nil
 }