Skip to content

Improve license parsing error messages #339

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.

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
47 changes: 36 additions & 11 deletions src/main/java/org/spdx/library/LicenseInfoFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,16 @@
import org.spdx.core.IModelCopyManager;
import org.spdx.core.InvalidSPDXAnalysisException;
import org.spdx.library.model.v2.license.InvalidLicenseStringException;
import org.spdx.library.model.v2.license.LicenseParserException;
import org.spdx.library.model.v2.license.SpdxListedLicense;
import org.spdx.library.model.v3_0_1.core.CreationInfo;
import org.spdx.library.model.v3_0_1.core.DictionaryEntry;
import org.spdx.library.model.v3_0_1.expandedlicensing.ListedLicense;
import org.spdx.library.model.v3_0_1.expandedlicensing.ListedLicenseException;
import org.spdx.library.model.v3_0_1.simplelicensing.AnyLicenseInfo;
import org.spdx.library.model.v3_0_1.simplelicensing.InvalidLicenseExpression;
import org.spdx.storage.IModelStore;
import org.spdx.utility.license.LicenseExpressionParser;
import org.spdx.utility.license.LicenseParserException;

/**
* Factory for creating SPDXLicenseInfo objects from a Jena model
Expand Down Expand Up @@ -90,8 +91,7 @@ public static SpdxListedLicense getListedLicenseByIdCompatV2(String licenseId)th
* @param documentUri Document URI for the document containing any extractedLicenseInfos - if any extractedLicenseInfos by ID already exist, they will be used. If
* none exist for an ID, they will be added. If null, the default model document URI will be used.
* @param copyManager allows for copying of any properties set which use other model stores or document URI's. If null, the default will be used.
* @return an SPDXLicenseInfo created from the string
* @throws InvalidLicenseStringException if the license string is not valid
* @return an SPDXLicenseInfo created from the string. If the license expression is not parseable, a <code>InvalidLicenseExpression</code> is returned.
* @throws DefaultStoreNotInitializedException if the default model store is not initialized
*/
public static org.spdx.library.model.v2.license.AnyLicenseInfo parseSPDXLicenseStringCompatV2(String licenseString, @Nullable IModelStore store,
Expand All @@ -109,9 +109,20 @@ public static org.spdx.library.model.v2.license.AnyLicenseInfo parseSPDXLicenseS
return LicenseExpressionParser.parseLicenseExpressionCompatV2(licenseString, store, documentUri,
copyManager);
} catch (LicenseParserException e) {
throw new InvalidLicenseStringException(e.getMessage(),e);
} catch (InvalidSPDXAnalysisException e) {
throw new InvalidLicenseStringException("Unexpected SPDX error parsing license string", e);
try {
return new org.spdx.library.model.v2.license.InvalidLicenseExpression(store, documentUri,
store.getNextId(IModelStore.IdType.Anonymous), copyManager, e.getMessage(), licenseString);
} catch (InvalidSPDXAnalysisException ex) {
throw new RuntimeException(ex);
}
} catch (InvalidSPDXAnalysisException e) {
try {
return new org.spdx.library.model.v2.license.InvalidLicenseExpression(store, documentUri,
store.getNextId(IModelStore.IdType.Anonymous), copyManager,
String.format("Unexpected SPDX error parsing license string: %s", e.getMessage()), licenseString);
} catch (InvalidSPDXAnalysisException ex) {
throw new RuntimeException(ex);
}
}
}

Expand All @@ -134,8 +145,7 @@ public static org.spdx.library.model.v2.license.AnyLicenseInfo parseSPDXLicenseS
* for an ID, they will be added. If null, the default model document URI + "#" will be used.
* @param copyManager allows for copying of any properties set which use other model stores or document URI's. If null, the default will be used.
* @param customIdToUri Mapping of the id prefixes used in the license expression to the namespace preceding the external ID
* @return an SPDXLicenseInfo created from the string
* @throws InvalidLicenseStringException if the license string is not valid
* @return an SPDXLicenseInfo created from the string. If the license expression is not parseable, a <code>InvalidLicenseExpression</code> is returned.
* @throws DefaultStoreNotInitializedException if the default model store is not initialized
*/
public static AnyLicenseInfo parseSPDXLicenseString(String licenseString, @Nullable IModelStore store,
Expand All @@ -151,13 +161,28 @@ public static AnyLicenseInfo parseSPDXLicenseString(String licenseString, @Nulla
copyManager = DefaultModelStore.getDefaultCopyManager();
}
try {
return LicenseExpressionParser.parseLicenseExpression(licenseString, store, customLicensePrefix,
return LicenseExpressionParser.parseLicenseExpression(licenseString, store, customLicensePrefix,
copyManager, customIdToUri);
} catch (LicenseParserException e) {
throw new InvalidLicenseStringException(e.getMessage(),e);
try {
InvalidLicenseExpression retval = new InvalidLicenseExpression(store, store.getNextId(IModelStore.IdType.Anonymous),
copyManager, true, customLicensePrefix);
retval.setMessage(e.getMessage());
return retval;
} catch (InvalidSPDXAnalysisException e1) {
throw new RuntimeException(e1);
}
} catch (InvalidSPDXAnalysisException e) {
throw new InvalidLicenseStringException("Unexpected SPDX error parsing license string", e);
try {
InvalidLicenseExpression retval = new InvalidLicenseExpression(store, store.getNextId(IModelStore.IdType.Anonymous),
copyManager, true, customLicensePrefix);
retval.setMessage(String.format("Unexpected SPDX error parsing license string: %s", e.getMessage()));
return retval;
} catch (InvalidSPDXAnalysisException e1) {
throw new RuntimeException(e1);
}
}

}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -504,6 +504,10 @@ private static AnyLicenseInfo parseSimpleLicenseToken(String token, IModelStore
return localLicense;
} else if (LicenseInfoFactory.isSpdxListedExceptionId(token)) {
throw new LicenseParserException(String.format("Unexpected listed license exception %s. Must be a listed license or a LicenseRef", token));
} else if (SpdxConstantsCompatV2.NOASSERTION_VALUE.equals(token)) {
throw new LicenseParserException("NOASSERTION is currently not allowed in a complex license expression");
} else if (SpdxConstantsCompatV2.NONE_VALUE.equals(token)) {
throw new LicenseParserException("NONE is currently not allowed in a complex license expression");
} else {
throw new LicenseParserException(String.format("Unknown license %s. Must be a listed license or have the syntax %s", token, SpdxConstantsCompatV2.LICENSE_ID_PATTERN));
}
Expand Down Expand Up @@ -590,6 +594,10 @@ private static org.spdx.library.model.v2.license.AnyLicenseInfo parseSimpleLicen
return localLicense;
} else if (LicenseInfoFactory.isSpdxListedExceptionId(token)) {
throw new LicenseParserException(String.format("Unexpected listed license exception %s. Must be a listed license or a LicenseRef", token));
} else if (SpdxConstantsCompatV2.NOASSERTION_VALUE.equals(token)) {
throw new LicenseParserException("NOASSERTION is currently not allowed in a complex license expression");
} else if (SpdxConstantsCompatV2.NONE_VALUE.equals(token)) {
throw new LicenseParserException("NONE is currently not allowed in a complex license expression");
} else {
throw new LicenseParserException(String.format("Unknown license %s. Must be a listed license or have the syntax %s", token, SpdxConstantsCompatV2.LICENSE_ID_PATTERN));
}
Expand Down
13 changes: 12 additions & 1 deletion src/test/java/org/spdx/library/LicenseInfoFactoryTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import org.spdx.library.model.v3_0_1.expandedlicensing.NoAssertionLicense;
import org.spdx.library.model.v3_0_1.expandedlicensing.NoneLicense;
import org.spdx.library.model.v3_0_1.simplelicensing.AnyLicenseInfo;
import org.spdx.library.model.v3_0_1.simplelicensing.InvalidLicenseExpression;
import org.spdx.storage.IModelStore;
import org.spdx.storage.IModelStore.IdType;
import org.spdx.storage.simple.InMemSpdxStore;
Expand Down Expand Up @@ -161,7 +162,7 @@ public void testSpecialLicenses() throws InvalidLicenseStringException, InvalidS
public void testDifferentLicenseOrder() throws InvalidSPDXAnalysisException {
AnyLicenseInfo order1 = LicenseInfoFactory.parseSPDXLicenseString("(LicenseRef-14 AND LicenseRef-5 AND LicenseRef-6 AND LicenseRef-15 AND LicenseRef-3 AND LicenseRef-12 AND LicenseRef-4 AND LicenseRef-13 AND LicenseRef-10 AND LicenseRef-9 AND LicenseRef-11 AND LicenseRef-7 AND LicenseRef-8 AND LGPL-2.1+ AND LicenseRef-1 AND LicenseRef-2 AND LicenseRef-0 AND GPL-2.0+ AND GPL-2.0 AND LicenseRef-17 AND LicenseRef-16 AND BSD-3-Clause-Clear)");
AnyLicenseInfo order2 = LicenseInfoFactory.parseSPDXLicenseString("(LicenseRef-14 AND LicenseRef-5 AND LicenseRef-6 AND LicenseRef-15 AND LicenseRef-12 AND LicenseRef-3 AND LicenseRef-13 AND LicenseRef-4 AND LicenseRef-10 AND LicenseRef-9 AND LicenseRef-11 AND LicenseRef-7 AND LicenseRef-8 AND LGPL-2.1+ AND LicenseRef-1 AND LicenseRef-2 AND LicenseRef-0 AND GPL-2.0+ AND GPL-2.0 AND LicenseRef-17 AND BSD-3-Clause-Clear AND LicenseRef-16)");
assertTrue(order1.equals(order2));
assertEquals(order1, order2);
assertTrue(order1.equivalent(order2));
}

Expand All @@ -171,4 +172,14 @@ public void testParseSPDXLicenseStringMixedCase() throws InvalidSPDXAnalysisExce
AnyLicenseInfo result = LicenseInfoFactory.parseSPDXLicenseString(lowerCaseCecil);
assertEquals(COMPLEX_LICENSE, result);
}

public void testInvalid() throws InvalidSPDXAnalysisException {
AnyLicenseInfo result = LicenseInfoFactory.parseSPDXLicenseString("MIT AND NOT Apache-2.0");
assertTrue(result instanceof InvalidLicenseExpression);
List<String> verify = result.verify();
assertEquals(1, verify.size());
assertTrue(verify.get(0).contains("NOT"));
assertTrue(verify.get(0).contains("Unknown license"));
}

}
9 changes: 9 additions & 0 deletions src/test/java/org/spdx/library/LicenseInfoFactoryTestV2.java
Original file line number Diff line number Diff line change
Expand Up @@ -159,4 +159,13 @@ public void testParseSPDXLicenseStringMixedCase() throws InvalidSPDXAnalysisExce
result = LicenseInfoFactory.parseSPDXLicenseStringCompatV2(lowerCaseCecil);
assertEquals(COMPLEX_LICENSE, result);
}

public void testInvalidV2() throws InvalidSPDXAnalysisException {
AnyLicenseInfo result = LicenseInfoFactory.parseSPDXLicenseStringCompatV2("MIT AND NOT Apache-2.0");
assertTrue(result instanceof org.spdx.library.model.v2.license.InvalidLicenseExpression);
List<String> verify = result.verify();
assertEquals(1, verify.size());
assertTrue(verify.get(0).contains("NOT"));
assertTrue(verify.get(0).contains("Unknown license"));
}
}
Loading