New License: libpng-1.6.35

[jlovejoy]

fixes #2591

Signed-off-by: Jilayne Lovejoy

[jlovejoy]

ugh, @xsuchy or @swinslow - help...

[xsuchy]

It seems ok to me. But

java -jar -DLocalFsfFreeJson=true -DlistedLicenseSchema="schema/ListedLicense.xsd" licenseListPublisher-3.1.1.jar LicenseRDFAGenerator 'src/libpng-1.6.35.xml' '/tmp' 1.0 2000-01-01 test/simpleTestForGenerator expected-warnings test/fullTestForGenerator

fails. This is what GH actions runs.

But

./test-one-license libpng-1.6.35

success.

@goneall ideas?

[swinslow]

I'm taking a look at this to try to figure out what's going on with the failing test.

Often, when test-one-license passes but the java . . . LicenseRDFAGenerator call fails, it often has to do with either the generation code for creating the published license text in one of its formats, or something else in the actual "publishing" process.

Looking at the failing test error message, I see:

Unhandled exception generating html: Cannot invoke "String.contains(java.lang.CharSequence)" because "text" is null
make: *** [Makefile:30: validate-canonical-match] Error 1

Digging into the latest LicenseListPublisher code, I see that this appears to be getting triggered from https://github.com/spdx/LicenseListPublisher/blob/14e017edbb5eb5fa1f6eaf43a4725e45addb6dfd/src/org/spdx/licenselistpublisher/LicenseRDFAGenerator.java#L386, based on the "Unhandled exception generating html" line.

There's a lot happening in the try block before that. I see a couple of places where it is calling contains() directly (though not certain whether it's a direct call to contains() that is leading to this exception):

• https://github.com/spdx/LicenseListPublisher/blob/14e017edbb5eb5fa1f6eaf43a4725e45addb6dfd/src/org/spdx/licenselistpublisher/LicenseRDFAGenerator.java#L446C20-L446C28
  • if (licenseIds.contains(nextExceptionContainer.getV2Exception().getLicenseExceptionId())) {
• https://github.com/spdx/LicenseListPublisher/blob/14e017edbb5eb5fa1f6eaf43a4725e45addb6dfd/src/org/spdx/licenselistpublisher/LicenseRDFAGenerator.java#L494
  • if (INVALID_TEXT_CHARS.contains(line.charAt(i))) {

I also see contains() getting called from a number of other locations in the LicenseListPublisher source code:

crossref/Wayback.java:44:			if(url.contains(UrlConstants.WAYBACK_URLS[i])) {
crossref/Valid.java:47:			boolean containsInvalidUrl = false;
crossref/Valid.java:49:				if(url.contains(UrlConstants.INVALID_URL_DOMAINS[i])) {
crossref/Valid.java:50:					containsInvalidUrl = url.contains(UrlConstants.INVALID_URL_DOMAINS[i]);
crossref/Valid.java:54:			return defaultValidator.isValid(url) && !containsInvalidUrl;
crossref/OsiApi.java:143:			if (!WHITE_LIST.contains(redirectUrl.getHost())) {
crossref/OsiApi.java:168:    	Boolean isLiveUrl = Objects.nonNull(spdxIds) && spdxIds.contains(license.getId());
licenselistpublisher/licensegenerator/LicenseHtmlFormatWriter.java:103:			if (INVALID_FILENAME_CHARS.contains(id.charAt(i))) {
licensexml/LicenseXmlHelper.java:198:			} else if (unprocessedTags.contains(tagName)) {
licensexml/LicenseXmlHelper.java:214:			if (parent.getNodeType() == Node.ELEMENT_NODE && ALT_ELEMENTS.contains(((Element)(parent)).getTagName())) {
licensexml/LicenseXmlHelper.java:399:				if (FLOW_CONTROL_ELEMENTS.contains(eChild.getTagName())) {

I've been staring at this for a while and will continue to see if I can sort out anything for why this is failing. Just posting this here to track what I'm seeing so far.

[swinslow]

New theory -- I'm noting the following timeline:

• @jlovejoy created this PR on May 10th
• The LicenseListPublisher was updated to a newer version on May 12th -- see e4e3897
• @jlovejoy made further edits to this PR since then

I see that the new publisher version (3.1.1) is running in the GitHub Actions check. But not sure whether something has gotten out of sync due to the publisher change since then.

I'm going to suggest re-opening a new PR with the same content of these XML and test text files, to see whether it passes then. I'll likely try that myself in a bit just to see if that resolves things.

[goneall]

Note that there were some changes made to the crossref code in the license list publisher to improve performance. From what is posted above, my guess is those changes may be causing the issue.

What is confusing me is I ran a full set of tests against the license list XML source before submitting the changes and the CI for the updated license list publisher should do the same. Not sure why it didn't fail earlier.

[goneall]

I'm able to duplicate the problem on my local machine. If I get some more time today, I'll try to determine the cause.

In any case, there is definitely an issue with the license list publisher - specifically the license matching code in the SPDX Java Library - since an NPE should never be thrown.

[goneall]

I think I found the issue.

The source of the issue is the license XML for the libpng-2.0 license.

The XML has a nested "rule" - an <optional> tag with an embedded <alt> tag (here). For the license list publisher, this generates the old template with nested rules which is explicitly disallowed (reference).

I don't think this was introduced with the last license list publisher - this is likely showed up now since this license is close to the libpng-2.0 license and hit that piece of matching code.

I didn't see any restrictions on the license XML on embedded <optional> and <var> tags. To be honest, I don't recall why we added that restriction - but we should at least look into the history and see if we need to add a similar restriction to the XML.

If we do want to carry forward the restriction, we should update the XML schema to reflect the restriction. This will also catch this earlier with a better error message when submitting new licenses.

There is also an issue in the license list publisher in that it should be able to handle this situation more gracefully.

[swinslow]

Thank you @goneall for taking the time to dig into this!

I note that that optional / alt tag in libpng-2.0 is for a line of hyphens. Now that we've changed the matching guidelines in recent years to make standalone hyphen lines like this ignoreable, we could probably just take that optional / alt tags out entirely.

I'll take a closer look again and then I'll submit a PR to remove those tags. Will also skim the rest of the license to see if any similar issues jump out.

[goneall]

@swinslow - You'll probably need to combine the updated libpng-2.0 XML with this one since the license list publisher checks for duplicates against the published license list unless the license XML is included in the same PR.