Skip to content

Commit ce8b54d

Browse files
goneallgoneall
committed
Improve license parsing error messages
Fixes #338 Dependes on updates to the SPDX Models versions 2 and 3 which add a new class "InvalidLicenseExpression" which is used to produce the errors. This changes the behavior of the license parser from throwing an exception to returning an InvalidLicenseExpression where the verify routine will return the error message.
1 parent 5407c1d commit ce8b54d

File tree

4 files changed

+65
-12
lines changed

4 files changed

+65
-12
lines changed

‎src/main/java/org/spdx/library/LicenseInfoFactory.java‎

Lines changed: 36 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -30,15 +30,16 @@
3030
import org.spdx.core.IModelCopyManager;
3131
import org.spdx.core.InvalidSPDXAnalysisException;
3232
import org.spdx.library.model.v2.license.InvalidLicenseStringException;
33-
import org.spdx.library.model.v2.license.LicenseParserException;
3433
import org.spdx.library.model.v2.license.SpdxListedLicense;
3534
import org.spdx.library.model.v3_0_1.core.CreationInfo;
3635
import org.spdx.library.model.v3_0_1.core.DictionaryEntry;
3736
import org.spdx.library.model.v3_0_1.expandedlicensing.ListedLicense;
3837
import org.spdx.library.model.v3_0_1.expandedlicensing.ListedLicenseException;
3938
import org.spdx.library.model.v3_0_1.simplelicensing.AnyLicenseInfo;
39+
import org.spdx.library.model.v3_0_1.simplelicensing.InvalidLicenseExpression;
4040
import org.spdx.storage.IModelStore;
4141
import org.spdx.utility.license.LicenseExpressionParser;
42+
import org.spdx.utility.license.LicenseParserException;
4243

4344
/**
4445
* Factory for creating SPDXLicenseInfo objects from a Jena model
@@ -90,8 +91,7 @@ public static SpdxListedLicense getListedLicenseByIdCompatV2(String licenseId)th
9091
* @param documentUri Document URI for the document containing any extractedLicenseInfos - if any extractedLicenseInfos by ID already exist, they will be used. If
9192
* none exist for an ID, they will be added. If null, the default model document URI will be used.
9293
* @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.
93-
* @return an SPDXLicenseInfo created from the string
94-
* @throws InvalidLicenseStringException if the license string is not valid
94+
* @return an SPDXLicenseInfo created from the string. If the license expression is not parseable, a <code>InvalidLicenseExpression</code> is returned.
9595
* @throws DefaultStoreNotInitializedException if the default model store is not initialized
9696
*/
9797
public static org.spdx.library.model.v2.license.AnyLicenseInfo parseSPDXLicenseStringCompatV2(String licenseString, @Nullable IModelStore store,
@@ -109,9 +109,20 @@ public static org.spdx.library.model.v2.license.AnyLicenseInfo parseSPDXLicenseS
109109
return LicenseExpressionParser.parseLicenseExpressionCompatV2(licenseString, store, documentUri,
110110
copyManager);
111111
} catch (LicenseParserException e) {
112-
throw new InvalidLicenseStringException(e.getMessage(),e);
113-
} catch (InvalidSPDXAnalysisException e) {
114-
throw new InvalidLicenseStringException("Unexpected SPDX error parsing license string", e);
112+
try {
113+
return new org.spdx.library.model.v2.license.InvalidLicenseExpression(store, documentUri,
114+
store.getNextId(IModelStore.IdType.Anonymous), copyManager, e.getMessage(), licenseString);
115+
} catch (InvalidSPDXAnalysisException ex) {
116+
throw new RuntimeException(ex);
117+
}
118+
} catch (InvalidSPDXAnalysisException e) {
119+
try {
120+
return new org.spdx.library.model.v2.license.InvalidLicenseExpression(store, documentUri,
121+
store.getNextId(IModelStore.IdType.Anonymous), copyManager,
122+
String.format("Unexpected SPDX error parsing license string: %s", e.getMessage()), licenseString);
123+
} catch (InvalidSPDXAnalysisException ex) {
124+
throw new RuntimeException(ex);
125+
}
115126
}
116127
}
117128

@@ -134,8 +145,7 @@ public static org.spdx.library.model.v2.license.AnyLicenseInfo parseSPDXLicenseS
134145
* for an ID, they will be added. If null, the default model document URI + "#" will be used.
135146
* @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.
136147
* @param customIdToUri Mapping of the id prefixes used in the license expression to the namespace preceding the external ID
137-
* @return an SPDXLicenseInfo created from the string
138-
* @throws InvalidLicenseStringException if the license string is not valid
148+
* @return an SPDXLicenseInfo created from the string. If the license expression is not parseable, a <code>InvalidLicenseExpression</code> is returned.
139149
* @throws DefaultStoreNotInitializedException if the default model store is not initialized
140150
*/
141151
public static AnyLicenseInfo parseSPDXLicenseString(String licenseString, @Nullable IModelStore store,
@@ -151,13 +161,28 @@ public static AnyLicenseInfo parseSPDXLicenseString(String licenseString, @Nulla
151161
copyManager = DefaultModelStore.getDefaultCopyManager();
152162
}
153163
try {
154-
return LicenseExpressionParser.parseLicenseExpression(licenseString, store, customLicensePrefix,
164+
return LicenseExpressionParser.parseLicenseExpression(licenseString, store, customLicensePrefix,
155165
copyManager, customIdToUri);
156166
} catch (LicenseParserException e) {
157-
throw new InvalidLicenseStringException(e.getMessage(),e);
167+
try {
168+
InvalidLicenseExpression retval = new InvalidLicenseExpression(store, store.getNextId(IModelStore.IdType.Anonymous),
169+
copyManager, true, customLicensePrefix);
170+
retval.setMessage(e.getMessage());
171+
return retval;
172+
} catch (InvalidSPDXAnalysisException e1) {
173+
throw new RuntimeException(e1);
174+
}
158175
} catch (InvalidSPDXAnalysisException e) {
159-
throw new InvalidLicenseStringException("Unexpected SPDX error parsing license string", e);
176+
try {
177+
InvalidLicenseExpression retval = new InvalidLicenseExpression(store, store.getNextId(IModelStore.IdType.Anonymous),
178+
copyManager, true, customLicensePrefix);
179+
retval.setMessage(String.format("Unexpected SPDX error parsing license string: %s", e.getMessage()));
180+
return retval;
181+
} catch (InvalidSPDXAnalysisException e1) {
182+
throw new RuntimeException(e1);
183+
}
160184
}
185+
161186
}
162187

163188
/**

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -504,6 +504,10 @@ private static AnyLicenseInfo parseSimpleLicenseToken(String token, IModelStore
504504
return localLicense;
505505
} else if (LicenseInfoFactory.isSpdxListedExceptionId(token)) {
506506
throw new LicenseParserException(String.format("Unexpected listed license exception %s. Must be a listed license or a LicenseRef", token));
507+
} else if (SpdxConstantsCompatV2.NOASSERTION_VALUE.equals(token)) {
508+
throw new LicenseParserException("NOASSERTION is currently not allowed in a complex license expression");
509+
} else if (SpdxConstantsCompatV2.NONE_VALUE.equals(token)) {
510+
throw new LicenseParserException("NONE is currently not allowed in a complex license expression");
507511
} else {
508512
throw new LicenseParserException(String.format("Unknown license %s. Must be a listed license or have the syntax %s", token, SpdxConstantsCompatV2.LICENSE_ID_PATTERN));
509513
}
@@ -590,6 +594,10 @@ private static org.spdx.library.model.v2.license.AnyLicenseInfo parseSimpleLicen
590594
return localLicense;
591595
} else if (LicenseInfoFactory.isSpdxListedExceptionId(token)) {
592596
throw new LicenseParserException(String.format("Unexpected listed license exception %s. Must be a listed license or a LicenseRef", token));
597+
} else if (SpdxConstantsCompatV2.NOASSERTION_VALUE.equals(token)) {
598+
throw new LicenseParserException("NOASSERTION is currently not allowed in a complex license expression");
599+
} else if (SpdxConstantsCompatV2.NONE_VALUE.equals(token)) {
600+
throw new LicenseParserException("NONE is currently not allowed in a complex license expression");
593601
} else {
594602
throw new LicenseParserException(String.format("Unknown license %s. Must be a listed license or have the syntax %s", token, SpdxConstantsCompatV2.LICENSE_ID_PATTERN));
595603
}

‎src/test/java/org/spdx/library/LicenseInfoFactoryTest.java‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import org.spdx.library.model.v3_0_1.expandedlicensing.NoAssertionLicense;
3333
import org.spdx.library.model.v3_0_1.expandedlicensing.NoneLicense;
3434
import org.spdx.library.model.v3_0_1.simplelicensing.AnyLicenseInfo;
35+
import org.spdx.library.model.v3_0_1.simplelicensing.InvalidLicenseExpression;
3536
import org.spdx.storage.IModelStore;
3637
import org.spdx.storage.IModelStore.IdType;
3738
import org.spdx.storage.simple.InMemSpdxStore;
@@ -161,7 +162,7 @@ public void testSpecialLicenses() throws InvalidLicenseStringException, InvalidS
161162
public void testDifferentLicenseOrder() throws InvalidSPDXAnalysisException {
162163
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)");
163164
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)");
164-
assertTrue(order1.equals(order2));
165+
assertEquals(order1, order2);
165166
assertTrue(order1.equivalent(order2));
166167
}
167168

@@ -171,4 +172,14 @@ public void testParseSPDXLicenseStringMixedCase() throws InvalidSPDXAnalysisExce
171172
AnyLicenseInfo result = LicenseInfoFactory.parseSPDXLicenseString(lowerCaseCecil);
172173
assertEquals(COMPLEX_LICENSE, result);
173174
}
175+
176+
public void testInvalid() throws InvalidSPDXAnalysisException {
177+
AnyLicenseInfo result = LicenseInfoFactory.parseSPDXLicenseString("MIT AND NOT Apache-2.0");
178+
assertTrue(result instanceof InvalidLicenseExpression);
179+
List<String> verify = result.verify();
180+
assertEquals(1, verify.size());
181+
assertTrue(verify.get(0).contains("NOT"));
182+
assertTrue(verify.get(0).contains("Unknown license"));
183+
}
184+
174185
}

‎src/test/java/org/spdx/library/LicenseInfoFactoryTestV2.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,4 +159,13 @@ public void testParseSPDXLicenseStringMixedCase() throws InvalidSPDXAnalysisExce
159159
result = LicenseInfoFactory.parseSPDXLicenseStringCompatV2(lowerCaseCecil);
160160
assertEquals(COMPLEX_LICENSE, result);
161161
}
162+
163+
public void testInvalidV2() throws InvalidSPDXAnalysisException {
164+
AnyLicenseInfo result = LicenseInfoFactory.parseSPDXLicenseStringCompatV2("MIT AND NOT Apache-2.0");
165+
assertTrue(result instanceof org.spdx.library.model.v2.license.InvalidLicenseExpression);
166+
List<String> verify = result.verify();
167+
assertEquals(1, verify.size());
168+
assertTrue(verify.get(0).contains("NOT"));
169+
assertTrue(verify.get(0).contains("Unknown license"));
170+
}
162171
}

0 commit comments

Comments
 (0)