Skip to content

Commit e66a28c

Browse files
nscuronscuro
authored
Merge pull request #2965 from nscuro/spdx-expression-improvements
SPDX expression support improvements
2 parents f57db7a + 06168eb commit e66a28c

15 files changed

+387
-53
lines changed

‎src/main/java/org/dependencytrack/model/Component.java

+2
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import com.github.packageurl.MalformedPackageURLException;
2929
import com.github.packageurl.PackageURL;
3030
import org.apache.commons.lang3.StringUtils;
31+
import org.dependencytrack.model.validation.ValidSpdxExpression;
3132
import org.dependencytrack.resources.v1.serializers.CustomPackageURLSerializer;
3233

3334
import javax.jdo.annotations.Column;
@@ -286,6 +287,7 @@ public enum FetchGroup {
286287
@Persistent
287288
@Column(name = "LICENSE_EXPRESSION", jdbcType = "CLOB", allowsNull = "true")
288289
@Pattern(regexp = RegexSequence.Definition.PRINTABLE_CHARS, message = "The license expression may only contain printable characters")
290+
@ValidSpdxExpression
289291
private String licenseExpression;
290292

291293
@Persistent

‎src/main/java/org/dependencytrack/model/validation/SpdxExpressionValidator.java

+40
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
/*
2+
* This file is part of Dependency-Track.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*
16+
* SPDX-License-Identifier: Apache-2.0
17+
* Copyright (c) Steve Springett. All Rights Reserved.
18+
*/
19+
package org.dependencytrack.model.validation;
20+
21+
import org.dependencytrack.parser.spdx.expression.SpdxExpressionParser;
22+
import org.dependencytrack.parser.spdx.expression.model.SpdxExpression;
23+
24+
import javax.validation.ConstraintValidator;
25+
import javax.validation.ConstraintValidatorContext;
26+
import java.util.Objects;
27+
28+
public class SpdxExpressionValidator implements ConstraintValidator<ValidSpdxExpression, String> {
29+
30+
@Override
31+
public boolean isValid(final String expressionString, final ConstraintValidatorContext validatorContext) {
32+
if (expressionString == null) {
33+
// null-ness is expected to be validated using @NotNull
34+
return true;
35+
}
36+
37+
return !Objects.equals(new SpdxExpressionParser().parse(expressionString), SpdxExpression.INVALID);
38+
}
39+
40+
}

‎src/main/java/org/dependencytrack/model/validation/ValidSpdxExpression.java

+41
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
/*
2+
* This file is part of Dependency-Track.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*
16+
* SPDX-License-Identifier: Apache-2.0
17+
* Copyright (c) Steve Springett. All Rights Reserved.
18+
*/
19+
package org.dependencytrack.model.validation;
20+
21+
import javax.validation.Constraint;
22+
import javax.validation.Payload;
23+
import java.lang.annotation.Documented;
24+
import java.lang.annotation.ElementType;
25+
import java.lang.annotation.Retention;
26+
import java.lang.annotation.RetentionPolicy;
27+
import java.lang.annotation.Target;
28+
29+
@Documented
30+
@Retention(RetentionPolicy.RUNTIME)
31+
@Target({ElementType.FIELD, ElementType.PARAMETER})
32+
@Constraint(validatedBy = SpdxExpressionValidator.class)
33+
public @interface ValidSpdxExpression {
34+
35+
String message() default "The license expression must be a valid SPDX expression";
36+
37+
Class<?>[] groups() default {};
38+
39+
Class<? extends Payload>[] payload() default {};
40+
41+
}

‎src/main/java/org/dependencytrack/parser/cyclonedx/util/ModelConverter.java

+20-13
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@
6868
import java.util.HashMap;
6969
import java.util.List;
7070
import java.util.Map;
71+
import java.util.Objects;
7172
import java.util.UUID;
7273

7374
public class ModelConverter {
@@ -160,19 +161,25 @@ public static Component convert(final QueryManager qm, final org.cyclonedx.model
160161
if (licenseChoice != null) {
161162
final List<org.cyclonedx.model.License> licenseOptions = new ArrayList<>();
162163
if (licenseChoice.getExpression() != null) {
163-
// store license expression, but don't overwrite manual changes to the field
164-
if (component.getLicenseExpression() == null) {
165-
component.setLicenseExpression(licenseChoice.getExpression());
166-
}
167-
// if the expression just consists of one license id, we can add it as another license option
168-
SpdxExpressionParser parser = new SpdxExpressionParser();
169-
SpdxExpression parsedExpression = parser.parse(licenseChoice.getExpression());
170-
if (parsedExpression.getSpdxLicenseId() != null) {
171-
org.cyclonedx.model.License expressionLicense = null;
172-
expressionLicense = new org.cyclonedx.model.License();
173-
expressionLicense.setId(parsedExpression.getSpdxLicenseId());
174-
expressionLicense.setName(parsedExpression.getSpdxLicenseId());
175-
licenseOptions.add(expressionLicense);
164+
final var expressionParser = new SpdxExpressionParser();
165+
final SpdxExpression parsedExpression = expressionParser.parse(licenseChoice.getExpression());
166+
if (!Objects.equals(parsedExpression, SpdxExpression.INVALID)) {
167+
// store license expression, but don't overwrite manual changes to the field
168+
if (component.getLicenseExpression() == null) {
169+
component.setLicenseExpression(licenseChoice.getExpression());
170+
}
171+
// if the expression just consists of one license id, we can add it as another license option
172+
if (parsedExpression.getSpdxLicenseId() != null) {
173+
org.cyclonedx.model.License expressionLicense = new org.cyclonedx.model.License();
174+
expressionLicense.setId(parsedExpression.getSpdxLicenseId());
175+
licenseOptions.add(expressionLicense);
176+
}
177+
} else {
178+
LOGGER.warn("""
179+
Encountered invalid license expression "%s" for \
180+
Component{group=%s, name=%s, version=%s, bomRef=%s}; Skipping\
181+
""".formatted(licenseChoice.getExpression(), component.getGroup(),
182+
component.getName(), component.getVersion(), component.getBomRef()));
176183
}
177184
}
178185
// add license options from the component's license array. These will have higher priority

‎src/main/java/org/dependencytrack/parser/spdx/expression/SpdxExpressionParser.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
* SpdxExpressions and SpdxExpressionOperations
3232
*
3333
* @author hborchardt
34-
* @since 4.8.0
34+
* @since 4.9.0
3535
*/
3636
public class SpdxExpressionParser {
3737

‎src/main/java/org/dependencytrack/parser/spdx/expression/model/SpdxExpression.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
* inner node, containss an operation.
2626
*
2727
* @author hborchardt
28-
* @since 4.8.0
28+
* @since 4.9.0
2929
*/
3030
public class SpdxExpression {
3131
public static final SpdxExpression INVALID = new SpdxExpression(null);

‎src/main/java/org/dependencytrack/parser/spdx/expression/model/SpdxExpressionOperation.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
* to that operator.
2727
*
2828
* @author hborchardt
29-
* @since 4.8.0
29+
* @since 4.9.0
3030
*/
3131
public class SpdxExpressionOperation {
3232
private SpdxOperator operator;

‎src/main/java/org/dependencytrack/parser/spdx/expression/model/SpdxOperator.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
* One of the SPDX expression operators as defined in the spec, together with their precedence.
2323
*
2424
* @author hborchardt
25-
* @since 4.8.0
25+
* @since 4.9.0
2626
*/
2727
public enum SpdxOperator {
2828
OR(1, "OR"), AND(2, "AND"), WITH(3, "WITH"), PLUS(4, "+");

‎src/main/java/org/dependencytrack/persistence/ComponentQueryManager.java

+4
Original file line numberDiff line numberDiff line change
@@ -371,6 +371,8 @@ public Component cloneComponent(Component sourceComponent, Project destinationPr
371371
component.setDescription(sourceComponent.getDescription());
372372
component.setCopyright(sourceComponent.getCopyright());
373373
component.setLicense(sourceComponent.getLicense());
374+
component.setLicenseExpression(sourceComponent.getLicenseExpression());
375+
component.setLicenseUrl(sourceComponent.getLicenseUrl());
374376
component.setResolvedLicense(sourceComponent.getResolvedLicense());
375377
component.setAuthor(sourceComponent.getAuthor());
376378
// TODO Add support for parent component and children components
@@ -399,6 +401,8 @@ public Component updateComponent(Component transientComponent, boolean commitInd
399401
component.setDescription(transientComponent.getDescription());
400402
component.setCopyright(transientComponent.getCopyright());
401403
component.setLicense(transientComponent.getLicense());
404+
component.setLicenseExpression(transientComponent.getLicenseExpression());
405+
component.setLicenseUrl(transientComponent.getLicenseUrl());
402406
component.setResolvedLicense(transientComponent.getResolvedLicense());
403407
component.setParent(transientComponent.getParent());
404408
component.setCpe(transientComponent.getCpe());

‎src/main/java/org/dependencytrack/resources/v1/ComponentResource.java

+29-5
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,8 @@ public Response createComponent(@PathParam("uuid") String uuid, Component jsonCo
252252
validator.validateProperty(jsonComponent, "group"),
253253
validator.validateProperty(jsonComponent, "description"),
254254
validator.validateProperty(jsonComponent, "license"),
255+
validator.validateProperty(jsonComponent, "licenseExpression"),
256+
validator.validateProperty(jsonComponent, "licenseUrl"),
255257
validator.validateProperty(jsonComponent, "filename"),
256258
validator.validateProperty(jsonComponent, "classifier"),
257259
validator.validateProperty(jsonComponent, "cpe"),
@@ -305,12 +307,20 @@ public Response createComponent(@PathParam("uuid") String uuid, Component jsonCo
305307
component.setSha3_512(StringUtils.trimToNull(jsonComponent.getSha3_512()));
306308
if (resolvedLicense != null) {
307309
component.setLicense(null);
310+
component.setLicenseExpression(null);
311+
component.setLicenseUrl(StringUtils.trimToNull(jsonComponent.getLicenseUrl()));
308312
component.setResolvedLicense(resolvedLicense);
309-
} else {
310-
component.setLicense(StringUtils.trimToNull(jsonComponent.getLicense()));
313+
} else if (StringUtils.trimToNull(jsonComponent.getLicense()) != null) {
314+
component.setLicense(StringUtils.trim(jsonComponent.getLicense()));
315+
component.setLicenseExpression(null);
316+
component.setLicenseUrl(StringUtils.trimToNull(jsonComponent.getLicenseUrl()));
317+
component.setResolvedLicense(null);
318+
} else if (StringUtils.trimToNull(jsonComponent.getLicenseExpression()) != null) {
319+
component.setLicense(null);
320+
component.setLicenseExpression(StringUtils.trim(jsonComponent.getLicenseExpression()));
321+
component.setLicenseUrl(null);
311322
component.setResolvedLicense(null);
312323
}
313-
component.setLicenseUrl(StringUtils.trimToNull(jsonComponent.getLicenseUrl()));
314324
component.setParent(parent);
315325
component.setNotes(StringUtils.trimToNull(jsonComponent.getNotes()));
316326

@@ -347,6 +357,7 @@ public Response updateComponent(Component jsonComponent) {
347357
validator.validateProperty(jsonComponent, "group"),
348358
validator.validateProperty(jsonComponent, "description"),
349359
validator.validateProperty(jsonComponent, "license"),
360+
validator.validateProperty(jsonComponent, "licenseExpression"),
350361
validator.validateProperty(jsonComponent, "licenseUrl"),
351362
validator.validateProperty(jsonComponent, "filename"),
352363
validator.validateProperty(jsonComponent, "classifier"),
@@ -395,12 +406,25 @@ public Response updateComponent(Component jsonComponent) {
395406
final License resolvedLicense = qm.getLicense(jsonComponent.getLicense());
396407
if (resolvedLicense != null) {
397408
component.setLicense(null);
409+
component.setLicenseExpression(null);
410+
component.setLicenseUrl(StringUtils.trimToNull(jsonComponent.getLicenseUrl()));
398411
component.setResolvedLicense(resolvedLicense);
412+
} else if (StringUtils.trimToNull(jsonComponent.getLicense()) != null) {
413+
component.setLicense(StringUtils.trim(jsonComponent.getLicense()));
414+
component.setLicenseExpression(null);
415+
component.setLicenseUrl(StringUtils.trimToNull(jsonComponent.getLicenseUrl()));
416+
component.setResolvedLicense(null);
417+
} else if (StringUtils.trimToNull(jsonComponent.getLicenseExpression()) != null) {
418+
component.setLicense(null);
419+
component.setLicenseExpression(StringUtils.trim(jsonComponent.getLicenseExpression()));
420+
component.setLicenseUrl(null);
421+
component.setResolvedLicense(null);
399422
} else {
400-
component.setLicense(StringUtils.trimToNull(jsonComponent.getLicense()));
423+
component.setLicense(null);
424+
component.setLicenseExpression(null);
425+
component.setLicenseUrl(null);
401426
component.setResolvedLicense(null);
402427
}
403-
component.setLicenseUrl(StringUtils.trimToNull(jsonComponent.getLicenseUrl()));
404428
component.setNotes(StringUtils.trimToNull(jsonComponent.getNotes()));
405429

406430
component = qm.updateComponent(component, true);

‎src/main/java/org/dependencytrack/upgrade/v490/v490Updater.java

-14
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import alpine.common.logging.Logger;
2222
import alpine.persistence.AlpineQueryManager;
2323
import alpine.server.upgrade.AbstractUpgradeItem;
24-
import alpine.server.util.DbUtil;
2524

2625
import java.sql.Connection;
2726
import java.sql.PreparedStatement;
@@ -40,7 +39,6 @@ public String getSchemaVersion() {
4039
@Override
4140
public void executeUpgrade(final AlpineQueryManager qm, final Connection connection) throws Exception {
4241
updateDefaultSnykApiVersion(connection);
43-
addLicenseExpressionColumnToComponents(connection);
4442
}
4543

4644
/**
@@ -64,16 +62,4 @@ private static void updateDefaultSnykApiVersion(final Connection connection) thr
6462
}
6563
}
6664

67-
private void addLicenseExpressionColumnToComponents(Connection connection) throws Exception {
68-
// The JDBC type "CLOB" is mapped to the type CLOB for H2, MEDIUMTEXT for MySQL, and TEXT for PostgreSQL and SQL Server.
69-
LOGGER.info("Adding \"LICENSE_EXPRESSION\" to \"COMPONENTS\"");
70-
if (DbUtil.isH2()) {
71-
DbUtil.executeUpdate(connection, "ALTER TABLE \"COMPONENTS\" ADD \"LICENSE_EXPRESSION\" CLOB");
72-
} else if (DbUtil.isMysql()) {
73-
DbUtil.executeUpdate(connection, "ALTER TABLE \"COMPONENTS\" ADD \"LICENSE_EXPRESSION\" MEDIUMTEXT");
74-
} else {
75-
DbUtil.executeUpdate(connection, "ALTER TABLE \"COMPONENTS\" ADD \"LICENSE_EXPRESSION\" TEXT");
76-
}
77-
}
78-
7965
}

‎src/test/java/org/dependencytrack/PersistenceCapableTest.java

+8
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,14 @@ public void before() throws Exception {
4141

4242
@After
4343
public void after() {
44+
// PersistenceManager will refuse to close when there's an active transaction
45+
// that was neither committed nor rolled back. Unfortunately some areas of the
46+
// code base can leave such a broken state behind if they run into unexpected
47+
// errors. See: https://github.com/DependencyTrack/dependency-track/issues/2677
48+
if (qm.getPersistenceManager().currentTransaction().isActive()) {
49+
qm.getPersistenceManager().currentTransaction().rollback();
50+
}
51+
4452
PersistenceManagerFactory.tearDown();
4553
}
4654

‎src/test/java/org/dependencytrack/model/validation/SpdxExpressionValidatorTest.java

+63
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
/*
2+
* This file is part of Dependency-Track.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*
16+
* SPDX-License-Identifier: Apache-2.0
17+
* Copyright (c) Steve Springett. All Rights Reserved.
18+
*/
19+
package org.dependencytrack.model.validation;
20+
21+
import org.junit.Before;
22+
import org.junit.Test;
23+
24+
import javax.validation.ConstraintViolation;
25+
import javax.validation.Validation;
26+
import javax.validation.Validator;
27+
import javax.validation.ValidatorFactory;
28+
import java.util.Set;
29+
30+
import static org.assertj.core.api.Assertions.assertThat;
31+
32+
public class SpdxExpressionValidatorTest {
33+
34+
private Validator validator;
35+
36+
private record TestRecord(@ValidSpdxExpression String expression) {
37+
}
38+
39+
@Before
40+
public void setUp() {
41+
final ValidatorFactory validatorFactory = Validation.buildDefaultValidatorFactory();
42+
validator = validatorFactory.getValidator();
43+
}
44+
45+
@Test
46+
public void testWithValidExpression() {
47+
final Set<ConstraintViolation<TestRecord>> violations = validator.validate(new TestRecord("Apache-2.0 OR MIT"));
48+
assertThat(violations).isEmpty();
49+
}
50+
51+
@Test
52+
public void testWithInvalidExpression() {
53+
final Set<ConstraintViolation<TestRecord>> violations = validator.validate(new TestRecord("(Apache-2.0"));
54+
assertThat(violations).isNotEmpty();
55+
}
56+
57+
@Test
58+
public void testWithNullExpression() {
59+
final Set<ConstraintViolation<TestRecord>> violations = validator.validate(new TestRecord(null));
60+
assertThat(violations).isEmpty();
61+
}
62+
63+
}

0 commit comments

Comments
 (0)