Error on missing package and file ids in v2.1

- 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>

8 files changed, 123 insertions, 1 deletions

diff --git a/tvloader/parser2v1/parse_creation_info.go b/tvloader/parser2v1/parse_creation_info.go
index 05130ee..d6986f0 100644
--- a/tvloader/parser2v1/parse_creation_info.go
+++ b/tvloader/parser2v1/parse_creation_info.go
@@ -76,6 +76,10 @@ func (parser *tvParser2_1) parsePairFromCreationInfo2_1(tag string, value string
 
 	// tag for going on to package section
 	case "PackageName":
+		//Error if last file does not has FileSPDXId
+		if parser.file != nil && parser.file.FileSPDXIdentifier == spdx.ElementID("") {
+			return fmt.Errorf("Invalid file without a package SPDX identifier")
+		}
 		parser.st = psPackage2_1
 		parser.pkg = &spdx.Package2_1{
 			FilesAnalyzed:             true,
diff --git a/tvloader/parser2v1/parse_file.go b/tvloader/parser2v1/parse_file.go
index efe31d9..a43ba64 100644
--- a/tvloader/parser2v1/parse_file.go
+++ b/tvloader/parser2v1/parse_file.go
@@ -18,10 +18,18 @@ func (parser *tvParser2_1) parsePairFromFile2_1(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_1{}
 		parser.file.FileName = value
 	// tag for creating new package section and going back to parsing Package
 	case "PackageName":
+		//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.st = psPackage2_1
 		parser.file = nil
 		return parser.parsePairFromPackage2_1(tag, value)
diff --git a/tvloader/parser2v1/parse_file_test.go b/tvloader/parser2v1/parse_file_test.go
index 68efe2b..05de9ea 100644
--- a/tvloader/parser2v1/parse_file_test.go
+++ b/tvloader/parser2v1/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 =====
@@ -885,4 +886,60 @@ func TestParser2_1FailsIfArtifactURIBeforeArtifactName(t *testing.T) {
 	}
 }
 
+func TestParser2_1FilesWithoutSpdxIdThrowError(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.1"}
+	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_1{
+		doc: &spdx.Document2_1{},
+		st:  psCreationInfo2_1,
+	}
+	fileName := "f2.txt"
+	err = parser.parsePair2_1("FileName", fileName)
+	err = parser.parsePair2_1("SnippetSPDXID", string(sid1))
+	err = parser.parsePair2_1("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_1{
+		doc: &spdx.Document2_1{},
+		st:  psCreationInfo2_1,
+	}
+	fileName = "f3.txt"
+	err = parser3.parsePair2_1("FileName", fileName)
+	err = parser3.parsePair2_1("PackageName", "p2")
+	if err == nil {
+		t.Errorf("files withoutSpdx Identifiers getting accepted")
+	}
+	err = parser3.parsePair2_1("PackageName", "p2")
+}
diff --git a/tvloader/parser2v1/parse_package.go b/tvloader/parser2v1/parse_package.go
index b653d9e..9a03e9a 100644
--- a/tvloader/parser2v1/parse_package.go
+++ b/tvloader/parser2v1/parse_package.go
@@ -20,6 +20,10 @@ func (parser *tvParser2_1) parsePairFromPackage2_1(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_1{
 				FilesAnalyzed:             true,
 				IsFilesAnalyzedTagPresent: false,
diff --git a/tvloader/parser2v1/parse_package_test.go b/tvloader/parser2v1/parse_package_test.go
index e6dc5e4..e9e095d 100644
--- a/tvloader/parser2v1/parse_package_test.go
+++ b/tvloader/parser2v1/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 =====
@@ -1069,3 +1070,41 @@ func TestFailsPackageExternalRefWithInvalidFormat(t *testing.T) {
 	}
 }
 
+func TestParser2_1PackageWithoutSpdxIdentifierThrowsError(t *testing.T) {
+	// More than one package , the previous package doesn't contain the SPDXID
+	pkgOldName := "p1"
+	parser := tvParser2_1{
+		doc: &spdx.Document2_1{Packages: map[spdx.ElementID]*spdx.Package2_1{}},
+		st:  psPackage2_1,
+		pkg: &spdx.Package2_1{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_1("PackageName", pkgName)
+	if err == nil {
+		t.Errorf("Packages withoutSpdx Identifiers getting accepted")
+	}
+	var tvPairs []reader.TagValuePair
+	tvPair1 := reader.TagValuePair{Tag: "SPDXVersion", Value: "SPDX-2.1"}
+	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/parser2v1/parse_snippet.go b/tvloader/parser2v1/parse_snippet.go
index f7085a7..fefa9ad 100644
--- a/tvloader/parser2v1/parse_snippet.go
+++ b/tvloader/parser2v1/parse_snippet.go
@@ -34,6 +34,10 @@ func (parser *tvParser2_1) parsePairFromSnippet2_1(tag string, value string) err
 	// tag for creating new package section and going back to parsing Package
 	case "PackageName":
 		parser.st = psPackage2_1
+		//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_1(tag, value)
diff --git a/tvloader/parser2v1/parser.go b/tvloader/parser2v1/parser.go
index 78f4685..9c30fb3 100644
--- a/tvloader/parser2v1/parser.go
+++ b/tvloader/parser2v1/parser.go
@@ -20,6 +20,12 @@ func ParseTagValues(tvs []reader.TagValuePair) (*spdx.Document2_1, 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
 }
diff --git a/tvloader/parser2v2/parse_creation_info.go b/tvloader/parser2v2/parse_creation_info.go
index 48efc58..b70fe84 100644
--- a/tvloader/parser2v2/parse_creation_info.go
+++ b/tvloader/parser2v2/parse_creation_info.go
@@ -76,7 +76,7 @@ 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
+		//Error if last file does not has FileSPDXId
 		if parser.file != nil && parser.file.FileSPDXIdentifier == spdx.ElementID("") {
 			return fmt.Errorf("Invalid file without a package SPDX identifier")
 		}