Improve license parsing error handling (#309)

* Improve license parsing error handling

Checks simple license tokens to make sure they are either a listed
license or a LicenseRef-

Fixes #227

* Fix typos in description

Co-authored-by: Arthit Suriyawongkul <[email protected]>
Signed-off-by: Gary O'Neall <[email protected]>

* Add check for addition ref syntax

* Fix unit tests for AdditionRefs

* Update src/main/java/org/spdx/utility/license/LicenseExpressionParser.java

Signed-off-by: Arthit Suriyawongkul <[email protected]>

* Update src/main/java/org/spdx/utility/license/LicenseExpressionParser.java

Signed-off-by: Arthit Suriyawongkul <[email protected]>

* Update src/main/java/org/spdx/utility/license/LicenseExpressionParser.java

Signed-off-by: Arthit Suriyawongkul <[email protected]>

* Update src/test/java/org/spdx/utility/license/LicenseExpressionParserTest.java

Signed-off-by: Arthit Suriyawongkul <[email protected]>

---------

Signed-off-by: Gary O'Neall <[email protected]>
Signed-off-by: Arthit Suriyawongkul <[email protected]>
Co-authored-by: Arthit Suriyawongkul <[email protected]>

Author: goneall

src/main/java/org/spdx/utility/license/LicenseExpressionParser.java

@@ -440,7 +440,7 @@ private static LicenseAddition parseSimpleLicenseAdditionToken(String token, IMo
 			}
 			return new ListedLicenseException(store, listedException.getObjectUri(), copyManager,
 					true, customLicenseUriPrefix);
-		} else {
+		} else if (token.toLowerCase().startsWith("additionref-")) {
 			// custom addition
 			String objectUri = customLicenseUriPrefix + token;
 			CustomLicenseAddition localAddition;
@@ -451,12 +451,13 @@ private static LicenseAddition parseSimpleLicenseAdditionToken(String token, IMo
 				localAddition.setAdditionText(UNINITIALIZED_LICENSE_TEXT);
 			}
 			return localAddition;
+		} else {
+			throw new LicenseParserException(String.format("Invalid license addition %s.  Must be either a listed license exception or be prefixed with 'AdditionRef-'", token));
 		}
 	}
 
 	/**
-	 * Converts a string token into its equivalent license
-	 * checking for a listed license
+	 * Converts a string token into its equivalent license - either a listed license or a LicenseRef
 	 * @param token Token to translate to the equivalent license
 	 * @param store Store for the licenses
 	 * @param customLicenseUriPrefix Prefix to use for any created local licenses or additions
@@ -490,7 +491,7 @@ private static AnyLicenseInfo parseSimpleLicenseToken(String token, IModelStore
 				}
 			}
 			return new ListedLicense(store, listedLicense.getObjectUri(), copyManager, true, SpdxConstantsV3.SPDX_LISTED_LICENSE_NAMESPACE);
-		} else {
+		} else if (token.toLowerCase().startsWith("licenseref-")) {
 			// LicenseRef
 			String objectUri = customLicenseUriPrefix + token;
 			CustomLicense localLicense;
@@ -501,6 +502,10 @@ private static AnyLicenseInfo parseSimpleLicenseToken(String token, IModelStore
 				localLicense.setLicenseText(UNINITIALIZED_LICENSE_TEXT);
 			}
 			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 {
+			throw new LicenseParserException(String.format("Unknown license %s.  Must be a listed license or have the syntax %s", token, SpdxConstantsCompatV2.LICENSE_ID_PATTERN));
 		}
 	}
 
@@ -520,26 +525,23 @@ private static String convertToExternalObjectUri(String externalReference, @Null
 			throw new LicenseParserException("Invalid external ID: "+externalReference);
 		}
 		String namespace = null;
-		if (Objects.nonNull(customIdToUri)) {
-			for (DictionaryEntry entry : customIdToUri) {
-				if (refParts[0].equals(entry.getIdPrefix())) {
-					Optional<String> entryValue = entry.getValue();
-					if (!entryValue.isPresent()) {
-						throw new LicenseParserException("No associated namespace for license ID prefix "+entry.getIdPrefix());
-					}
-					namespace = entryValue.get();
-				}
-			}
-		}
-		if (Objects.isNull(namespace)) {
+        for (DictionaryEntry entry : customIdToUri) {
+            if (refParts[0].equals(entry.getIdPrefix())) {
+                Optional<String> entryValue = entry.getValue();
+                if (!entryValue.isPresent()) {
+                    throw new LicenseParserException("No associated namespace for license ID prefix " + entry.getIdPrefix());
+                }
+                namespace = entryValue.get();
+            }
+        }
+        if (Objects.isNull(namespace)) {
 			throw new LicenseParserException("No ID Prefix "+refParts[0]+" found in the customIdToUri map");
 		}
 		return namespace + refParts[1];
 	}
 
 	/**
-	 * Converts a string token into its equivalent license
-	 * checking for a listed license
+	 * Converts a string token into its equivalent license - either a listed license or a LicenseRef
 	 * @param token license ID token
 	 * @param store model store for non-listed licenses
 	 * @param documentUri document URI for non-listed licenses
@@ -568,14 +570,13 @@ private static org.spdx.library.model.v2.license.AnyLicenseInfo parseSimpleLicen
 				SpdxListedLicense listedLicense = LicenseInfoFactory.getListedLicenseByIdCompatV2(licenseId.get());
 				if (Objects.nonNull(copyManager)) {
 					// copy to the local store
-					copyManager.copy(store, listedLicense.getObjectUri(), listedLicense.getModelStore(), 
+					copyManager.copy(store, listedLicense.getObjectUri(), listedLicense.getModelStore(),
 							listedLicense.getObjectUri(), ModelObjectV2.LATEST_SPDX_2_VERSION, listedLicense.getDocumentUri());
 				}
 			}
-			return (org.spdx.library.model.v2.license.AnyLicenseInfo) org.spdx.library.model.v2.SpdxModelFactoryCompatV2.getModelObjectV2(store, SpdxConstantsCompatV2.LISTED_LICENSE_NAMESPACE_PREFIX, 
+			return (org.spdx.library.model.v2.license.AnyLicenseInfo) org.spdx.library.model.v2.SpdxModelFactoryCompatV2.getModelObjectV2(store, SpdxConstantsCompatV2.LISTED_LICENSE_NAMESPACE_PREFIX,
 					licenseId.get(), SpdxConstantsCompatV2.CLASS_SPDX_LISTED_LICENSE, copyManager, true);
-		} else {
-			// LicenseRef
+		} else if (token.toLowerCase().startsWith("licenseref-")) {
 			Optional<String> caseSensitiveId = store.getCaseSensitiveId(documentUri, token);
 			ExtractedLicenseInfo localLicense;
 			if (caseSensitiveId.isPresent()) {
@@ -587,6 +588,10 @@ private static org.spdx.library.model.v2.license.AnyLicenseInfo parseSimpleLicen
 				localLicense.setExtractedText(UNINITIALIZED_LICENSE_TEXT);
 			}
 			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 {
+			throw new LicenseParserException(String.format("Unknown license %s.  Must be a listed license or have the syntax %s", token, SpdxConstantsCompatV2.LICENSE_ID_PATTERN));
 		}
 	}
 

src/test/java/org/spdx/utility/license/LicenseExpressionParserTest.java

@@ -61,7 +61,8 @@ public class LicenseExpressionParserTest extends TestCase {
 	static final String[] STD_TEXTS = new String[] {"Academic Free License (", "CONTRAT DE LICENCE DE LOGICIEL LIBRE CeCILL-B",
 	"European Union Public Licence", "Afmparse License"};
 	static final String[] NONSTD_TEXTS = new String[] {"text1", "text2", "text3", "text4"};
-	static final String[] NONSTD_ADDITION_IDS = new String[] {"exception-1","exception-2", "exception-3", "exception-4"};
+	static final String[] NONSTD_ADDITION_IDS = new String[] {"AdditionRef-exception-1", "AdditionRef-exception-2",
+			"AdditionRef-exception-3", "AdditionRef-exception-4"};
 	static final String[] STD_EXCEPTION_IDS = new String[] {"389-exception", "Autoconf-exception-2.0"};
 	static final String[] NON_STD_ADDITION_NAMES = new String[] {"exName-1", "exName-2", "exName-3", "exName-4"};
 	static final String[] NON_STD_ADDITION_TEXTS = new String[] {"Ex text 1", "Ex text 2", "Ex text 3", "Ex text 4"};