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

Follow CycloneDX 1.4 spec for SPDX license expressions for npm. #690

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Follow CycloneDX 1.4 spec for SPDX license expressions for npm. #690

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4016517
Follow CycloneDX 1.4 spec for SPDX license expressions for npm.
ansonallard Nov 2, 2023
a359172
Added unit test for SPDX expression
ansonallard Nov 2, 2023
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
6 changes: 5 additions & 1 deletion utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,8 @@ export function getLicenses(pkg, format = "xml") {
licenseContent.name = "CUSTOM";
}
licenseContent.url = l;
} else if (l.includes(" ") || l.includes("(")) {
licenseContent.expression = l;
} else {
licenseContent.name = l;
}
Expand All @@ -234,7 +236,9 @@ export function getLicenses(pkg, format = "xml") {
}
return licenseContent;
})
.map((l) => ({ license: l }));
.map((l) =>
l.expression ? { expression: l.expression } : { license: l }
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think there can be only one expression, so possible we are returning multiple.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The licenses list supports multiple schemas in the array. I'm not certain if the spec requires all data to be the same schema in the list. Spec

Copy link
Contributor Author

@ansonallard ansonallard Nov 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

That appears to be the difference between CycloneDX 1.4 and 1.5. It seems 1.4 allows for a list of SPDX expressions (which seems wrong anyways), whereas, like you said, 1.5 requires a list of licenses or a single expression.

1.4 Spec

1.5 Spec

Does this match your understanding of the specs?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh yeah. Looks like that is new 1.5 restriction. I remember the rationale for enforcing a single expression. We may have to try to fix this bug in index.js or elsewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are a few situations that I can think of:

  1. A component has multiple licenses and a SPDX expression
  2. A component has multiple SPDX expressions and multiple licenses
  3. A component has multiple licenses, no SPDX expressions
  4. A component has one SPDX expression

For 1, it may make sense to ignore the license list and serve the single SPDX expression (the expression takes precedence).

For 2, one would have to choose which expression to persist. It could be simple, but arbitrary, like choosing the first entry, or more complex, like choosing the most permissive or open.

For 3 and 4, those behaviors are known.

Copy link
Collaborator

Choose a reason for hiding this comment

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

  1. If there are legitimately multiple expressions (with a space or bracket), then we can use the first one and show a warning.

);
}
return undefined;
}
Expand Down
10 changes: 10 additions & 0 deletions utils.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1591,6 +1591,16 @@ test("get licenses", () => {
}
}
]);

let inputLicense = "(MIT or Apache-2.0)";
licenses = getLicenses({
license: inputLicense
});
expect(licenses).toEqual([
{
expression: inputLicense
}
]);
});

test("parsePkgLock v1", async () => {
Expand Down
Loading