Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: properly output and read the filesAnalyzed field in JSON/YAML #210

Merged
merged 1 commit into from
May 23, 2023
Merged

fix: properly output and read the filesAnalyzed field in JSON/YAML #210

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion examples/sample-docs/json/SPDXJSONExample-v2.2.spdx.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,6 @@
"referenceLocator" : "pkg:maven/org.apache.jena/apache-jena@3.12.0",
"referenceType" : "purl"
} ],
"filesAnalyzed" : false,
"homepage" : "http://www.openjena.org/",
"licenseConcluded" : "NOASSERTION",
"licenseDeclared" : "NOASSERTION",
Expand Down
2 changes: 2 additions & 0 deletions examples/sample-docs/json/SPDXJSONExample-v2.3.spdx.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@
"name": "Apache Commons Lang",
"SPDXID": "SPDXRef-fromDoap-1",
"downloadLocation": "NOASSERTION",
"filesAnalyzed": false,
"homepage": "http://commons.apache.org/proper/commons-lang/",
"licenseConcluded": "NOASSERTION",
"licenseDeclared": "NOASSERTION",
Expand Down Expand Up @@ -125,6 +126,7 @@
"versionInfo": "8.8",
"packageFileName": "saxonB-8.8.zip",
"downloadLocation": "https://sourceforge.net/projects/saxon/files/Saxon-B/8.8.0.7/saxonb8-8-0-7j.zip/download",
"filesAnalyzed": false,
"checksums": [
{
"algorithm": "SHA1",
Expand Down
4 changes: 2 additions & 2 deletions examples/sample-docs/tv/SPDXTagExample-v2.3.spdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ PrimaryPackagePurpose: CONTAINER
ReleaseDate: 2021-10-15T02:38:00Z
BuiltDate: 2021-09-15T02:38:00Z
ValidUntilDate: 2022-10-15T02:38:00Z
FilesAnalyzed: false
FilesAnalyzed: true
PackageHomePage: https://www.centos.org/
PackageCopyrightText: NOASSERTION
PackageDescription: The CentOS container used to run the application.
Expand Down Expand Up @@ -148,7 +148,7 @@ PackageName: Jena
SPDXID: SPDXRef-fromDoap-0
PackageVersion: 3.12.0
PackageDownloadLocation: https://search.maven.org/remotecontent?filepath=org/apache/jena/apache-jena/3.12.0/apache-jena-3.12.0.tar.gz
FilesAnalyzed: false
FilesAnalyzed: true
PackageHomePage: http://www.openjena.org/
PackageLicenseConcluded: NOASSERTION
PackageLicenseDeclared: NOASSERTION
Expand Down
1 change: 0 additions & 1 deletion examples/sample-docs/yaml/SPDXYAMLExample-2.2.spdx.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,6 @@ packages:
- referenceCategory: "PACKAGE_MANAGER"
referenceLocator: "pkg:maven/org.apache.jena/apache-jena@3.12.0"
referenceType: "purl"
filesAnalyzed: false
homepage: "http://www.openjena.org/"
licenseConcluded: "NOASSERTION"
licenseDeclared: "NOASSERTION"
Expand Down
2 changes: 2 additions & 0 deletions examples/sample-docs/yaml/SPDXYAMLExample-2.3.spdx.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,7 @@ packages:
- SPDXID: SPDXRef-fromDoap-1
copyrightText: NOASSERTION
downloadLocation: NOASSERTION
filesAnalyzed: false
homepage: http://commons.apache.org/proper/commons-lang/
licenseConcluded: NOASSERTION
licenseDeclared: NOASSERTION
Expand All @@ -331,6 +332,7 @@ packages:
checksumValue: 85ed0817af83a24ad8da68c2b5094de69833983c
copyrightText: Copyright Saxonica Ltd
description: The Saxon package is a collection of tools for processing XML documents.
filesAnalyzed: false
downloadLocation: https://sourceforge.net/projects/saxon/files/Saxon-B/8.8.0.7/saxonb8-8-0-7j.zip/download
homepage: http://saxon.sourceforge.net/
licenseComments: Other versions available for a commercial license
Expand Down
56 changes: 30 additions & 26 deletions spdx/v2/v2_2/example/example.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,9 @@ var example = spdx.Document{
Originator: "ExampleCodeInspect (contact@example.com)",
OriginatorType: "Organization",
},
PackageDownloadLocation: "http://ftp.gnu.org/gnu/glibc/glibc-ports-2.15.tar.gz",
FilesAnalyzed: true,
PackageDownloadLocation: "http://ftp.gnu.org/gnu/glibc/glibc-ports-2.15.tar.gz",
FilesAnalyzed: true,
IsFilesAnalyzedTagPresent: true,
PackageVerificationCode: common.PackageVerificationCode{
Value: "d6a770ba38583ed4bb4525bd96e50461655d2758",
ExcludedFiles: []string{"./package.spdx"},
Expand Down Expand Up @@ -187,14 +188,15 @@ var example = spdx.Document{
},
},
{
PackageSPDXIdentifier: "fromDoap-1",
PackageCopyrightText: "NOASSERTION",
PackageDownloadLocation: "NOASSERTION",
FilesAnalyzed: false,
PackageHomePage: "http://commons.apache.org/proper/commons-lang/",
PackageLicenseConcluded: "NOASSERTION",
PackageLicenseDeclared: "NOASSERTION",
PackageName: "Apache Commons Lang",
PackageSPDXIdentifier: "fromDoap-1",
PackageCopyrightText: "NOASSERTION",
PackageDownloadLocation: "NOASSERTION",
FilesAnalyzed: false,
IsFilesAnalyzedTagPresent: true,
PackageHomePage: "http://commons.apache.org/proper/commons-lang/",
PackageLicenseConcluded: "NOASSERTION",
PackageLicenseDeclared: "NOASSERTION",
PackageName: "Apache Commons Lang",
},
{
PackageName: "Jena",
Expand All @@ -208,11 +210,12 @@ var example = spdx.Document{
Locator: "pkg:maven/org.apache.jena/apache-jena@3.12.0",
},
},
FilesAnalyzed: false,
PackageHomePage: "http://www.openjena.org/",
PackageLicenseConcluded: "NOASSERTION",
PackageLicenseDeclared: "NOASSERTION",
PackageVersion: "3.12.0",
FilesAnalyzed: true,
IsFilesAnalyzedTagPresent: false,
PackageHomePage: "http://www.openjena.org/",
PackageLicenseConcluded: "NOASSERTION",
PackageLicenseDeclared: "NOASSERTION",
PackageVersion: "3.12.0",
},
{
PackageSPDXIdentifier: "Saxon",
Expand All @@ -222,17 +225,18 @@ var example = spdx.Document{
Value: "85ed0817af83a24ad8da68c2b5094de69833983c",
},
},
PackageCopyrightText: "Copyright Saxonica Ltd",
PackageDescription: "The Saxon package is a collection of tools for processing XML documents.",
PackageDownloadLocation: "https://sourceforge.net/projects/saxon/files/Saxon-B/8.8.0.7/saxonb8-8-0-7j.zip/download",
FilesAnalyzed: false,
PackageHomePage: "http://saxon.sourceforge.net/",
PackageLicenseComments: "Other versions available for a commercial license",
PackageLicenseConcluded: "MPL-1.0",
PackageLicenseDeclared: "MPL-1.0",
PackageName: "Saxon",
PackageFileName: "saxonB-8.8.zip",
PackageVersion: "8.8",
PackageCopyrightText: "Copyright Saxonica Ltd",
PackageDescription: "The Saxon package is a collection of tools for processing XML documents.",
PackageDownloadLocation: "https://sourceforge.net/projects/saxon/files/Saxon-B/8.8.0.7/saxonb8-8-0-7j.zip/download",
FilesAnalyzed: false,
IsFilesAnalyzedTagPresent: true,
PackageHomePage: "http://saxon.sourceforge.net/",
PackageLicenseComments: "Other versions available for a commercial license",
PackageLicenseConcluded: "MPL-1.0",
PackageLicenseDeclared: "MPL-1.0",
PackageName: "Saxon",
PackageFileName: "saxonB-8.8.zip",
PackageVersion: "8.8",
},
},
Files: []*spdx.File{
Expand Down
13 changes: 13 additions & 0 deletions spdx/v2/v2_2/json/json_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,11 @@ func TestLoad(t *testing.T) {
func Test_Write(t *testing.T) {
want := example.Copy()

// we always output FilesAnalyzed, even though we handle reading files where it is omitted
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the weird part of filesAnalyzed is that if it is omitted technically the correct value is "true" but really no one does it this way... which is kind of cringe, because the intention is usually filesAnalyzed=false... if we actually did a validation check when reading, likely >90% of SPDX documents will fail since they assume omission is default false based on the programming language defaults, and thus they don't include VerificationCode which technically is required if filesAnalyzed=true.

I see trade offs both ways, but since we do require user to signify their intention and the use of the library is not asking users to provide a mandatory field, a golang user will think that it defaults to false. My thought is to perhaps maybe flip the IsFilesAnalyzedTagPresent to IsFilesAnalyzedTagOmitted, or make the default value to true... i don't quite want to break the interface. This may create a experience a bit more aligned with golang

This however, doesn't fix the issue of all the input SBOMs having their filesAnalyzed intent misinterpreted, which technically isn't an issue for the library... but i think it will break enough of the ecosystem that we have to consider it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i thought about it a bit more... maybe what we can do is that during reading, is that if we see no VerificationCode we can set filesAnalyzed=false.. wdyt @kzantow ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the more i look into this, the worse it gets... since filesAnalyzed=false is also an indication that a package doesn't contain any files, this messes things up..

I think my new thoughts around this is to just output it IsFilesAnalyzedTagPresent=true, and otherwise omit it and not assert anything about its intended value but to parse it as is.. We think we will need to support this bad behavior as accounting for a deficiency of the spec.

for _, p := range want.Packages {
p.IsFilesAnalyzedTagPresent = true
}

w := &bytes.Buffer{}

if err := json.Write(&want, w); err != nil {
Expand Down Expand Up @@ -153,16 +158,19 @@ func Test_ShorthandFields(t *testing.T) {
{
PackageName: "Container",
PackageSPDXIdentifier: "Container",
FilesAnalyzed: true,
},
{
PackageName: "Package-1",
PackageSPDXIdentifier: "Package-1",
PackageVersion: "1.1.1",
FilesAnalyzed: true,
},
{
PackageName: "Package-2",
PackageSPDXIdentifier: "Package-2",
PackageVersion: "2.2.2",
FilesAnalyzed: true,
},
},
Files: []*spdx.File{
Expand Down Expand Up @@ -272,6 +280,7 @@ func Test_JsonEnums(t *testing.T) {
{
PackageName: "Container",
PackageSPDXIdentifier: "Container",
FilesAnalyzed: true,
},
{
PackageName: "Package-1",
Expand All @@ -284,6 +293,7 @@ func Test_JsonEnums(t *testing.T) {
Locator: "pkg:somepkg/ns/name1",
},
},
FilesAnalyzed: true,
},
{
PackageName: "Package-2",
Expand All @@ -296,6 +306,7 @@ func Test_JsonEnums(t *testing.T) {
Locator: "pkg:somepkg/ns/name2",
},
},
FilesAnalyzed: true,
},
{
PackageName: "Package-3",
Expand All @@ -308,6 +319,7 @@ func Test_JsonEnums(t *testing.T) {
Locator: "gitoid:blob:sha1:261eeb9e9f8b2b4b0d119366dda99c6fd7d35c64",
},
},
FilesAnalyzed: true,
},
{
PackageName: "Package-4",
Expand All @@ -320,6 +332,7 @@ func Test_JsonEnums(t *testing.T) {
Locator: "gitoid:blob:sha1:261eeb9e9f8b2b4b0d119366dda99c6fd7d35c64",
},
},
FilesAnalyzed: true,
},
},
Relationships: []*spdx.Relationship{
Expand Down
11 changes: 9 additions & 2 deletions spdx/v2/v2_2/package.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ type Package struct {

// 7.8: FilesAnalyzed
// Cardinality: optional, one; default value is "true" if omitted
FilesAnalyzed bool `json:"filesAnalyzed,omitempty"`
FilesAnalyzed bool `json:"filesAnalyzed"`
// NOT PART OF SPEC: did FilesAnalyzed tag appear?
IsFilesAnalyzedTagPresent bool `json:"-"`

Expand Down Expand Up @@ -125,7 +125,8 @@ type Package struct {
func (p *Package) UnmarshalJSON(b []byte) error {
type pkg Package
type extras struct {
HasFiles []common.DocElementID `json:"hasFiles"`
HasFiles []common.DocElementID `json:"hasFiles"`
FilesAnalyzed *bool `json:"filesAnalyzed"`
}

var p2 pkg
Expand All @@ -141,6 +142,12 @@ func (p *Package) UnmarshalJSON(b []byte) error {
*p = Package(p2)

p.hasFiles = e.HasFiles
// FilesAnalyzed defaults to true if omitted
if e.FilesAnalyzed == nil {
p.FilesAnalyzed = true
} else {
p.IsFilesAnalyzedTagPresent = true
}

return nil
}
Expand Down
5 changes: 5 additions & 0 deletions spdx/v2/v2_2/yaml/yaml_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,11 @@ func Test_Read(t *testing.T) {
func Test_Write(t *testing.T) {
want := example.Copy()

// we always output FilesAnalyzed, even though we handle reading files where it is omitted
for _, p := range want.Packages {
p.IsFilesAnalyzedTagPresent = true
}

w := &bytes.Buffer{}
if err := yaml.Write(want, w); err != nil {
t.Errorf("Save() error = %v", err.Error())
Expand Down
58 changes: 31 additions & 27 deletions spdx/v2/v2_3/example/example.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,9 @@ var example = spdx.Document{
Originator: "ExampleCodeInspect (contact@example.com)",
OriginatorType: "Organization",
},
PackageDownloadLocation: "http://ftp.gnu.org/gnu/glibc/glibc-ports-2.15.tar.gz",
FilesAnalyzed: true,
PackageDownloadLocation: "http://ftp.gnu.org/gnu/glibc/glibc-ports-2.15.tar.gz",
FilesAnalyzed: true,
IsFilesAnalyzedTagPresent: true,
PackageVerificationCode: &common.PackageVerificationCode{
Value: "d6a770ba38583ed4bb4525bd96e50461655d2758",
ExcludedFiles: []string{"./package.spdx"},
Expand Down Expand Up @@ -187,14 +188,15 @@ var example = spdx.Document{
},
},
{
PackageSPDXIdentifier: "fromDoap-1",
PackageCopyrightText: "NOASSERTION",
PackageDownloadLocation: "NOASSERTION",
FilesAnalyzed: false,
PackageHomePage: "http://commons.apache.org/proper/commons-lang/",
PackageLicenseConcluded: "NOASSERTION",
PackageLicenseDeclared: "NOASSERTION",
PackageName: "Apache Commons Lang",
PackageSPDXIdentifier: "fromDoap-1",
PackageCopyrightText: "NOASSERTION",
PackageDownloadLocation: "NOASSERTION",
FilesAnalyzed: false,
IsFilesAnalyzedTagPresent: true,
PackageHomePage: "http://commons.apache.org/proper/commons-lang/",
PackageLicenseConcluded: "NOASSERTION",
PackageLicenseDeclared: "NOASSERTION",
PackageName: "Apache Commons Lang",
},
{
PackageName: "Jena",
Expand All @@ -208,11 +210,12 @@ var example = spdx.Document{
Locator: "pkg:maven/org.apache.jena/apache-jena@3.12.0",
},
},
FilesAnalyzed: false,
PackageHomePage: "http://www.openjena.org/",
PackageLicenseConcluded: "NOASSERTION",
PackageLicenseDeclared: "NOASSERTION",
PackageVersion: "3.12.0",
FilesAnalyzed: true,
IsFilesAnalyzedTagPresent: false,
PackageHomePage: "http://www.openjena.org/",
PackageLicenseConcluded: "NOASSERTION",
PackageLicenseDeclared: "NOASSERTION",
PackageVersion: "3.12.0",
},
{
PackageSPDXIdentifier: "Saxon",
Expand All @@ -222,25 +225,26 @@ var example = spdx.Document{
Value: "85ed0817af83a24ad8da68c2b5094de69833983c",
},
},
PackageCopyrightText: "Copyright Saxonica Ltd",
PackageDescription: "The Saxon package is a collection of tools for processing XML documents.",
PackageDownloadLocation: "https://sourceforge.net/projects/saxon/files/Saxon-B/8.8.0.7/saxonb8-8-0-7j.zip/download",
FilesAnalyzed: false,
PackageHomePage: "http://saxon.sourceforge.net/",
PackageLicenseComments: "Other versions available for a commercial license",
PackageLicenseConcluded: "MPL-1.0",
PackageLicenseDeclared: "MPL-1.0",
PackageName: "Saxon",
PackageFileName: "saxonB-8.8.zip",
PackageVersion: "8.8",
PackageCopyrightText: "Copyright Saxonica Ltd",
PackageDescription: "The Saxon package is a collection of tools for processing XML documents.",
PackageDownloadLocation: "https://sourceforge.net/projects/saxon/files/Saxon-B/8.8.0.7/saxonb8-8-0-7j.zip/download",
FilesAnalyzed: false,
IsFilesAnalyzedTagPresent: true,
PackageHomePage: "http://saxon.sourceforge.net/",
PackageLicenseComments: "Other versions available for a commercial license",
PackageLicenseConcluded: "MPL-1.0",
PackageLicenseDeclared: "MPL-1.0",
PackageName: "Saxon",
PackageFileName: "saxonB-8.8.zip",
PackageVersion: "8.8",
},
{
PrimaryPackagePurpose: "CONTAINER",
PackageSPDXIdentifier: "CentOS-7",
PackageCopyrightText: "NOASSERTION",
PackageDescription: "The CentOS container used to run the application.",
PackageDownloadLocation: "NOASSERTION",
FilesAnalyzed: false,
FilesAnalyzed: true,
PackageHomePage: "https://www.centos.org/",
PackageName: "centos",
PackageFileName: "saxonB-8.8.zip",
Expand Down
Loading