DependencyTrack · sahibamittal · Oct 26, 2023 · Oct 25, 2023
diff --git a/src/main/java/org/dependencytrack/model/Component.java b/src/main/java/org/dependencytrack/model/Component.java
@@ -28,6 +28,7 @@
 import com.github.packageurl.MalformedPackageURLException;
 import com.github.packageurl.PackageURL;
 import org.apache.commons.lang3.StringUtils;
+import org.dependencytrack.model.validation.ValidSpdxExpression;
 import org.dependencytrack.resources.v1.serializers.CustomPackageURLSerializer;
 
 import javax.jdo.annotations.Column;
@@ -291,6 +292,7 @@ public enum FetchGroup {
     @Persistent
     @Column(name = "LICENSE_EXPRESSION", jdbcType = "CLOB", allowsNull = "true")
     @Pattern(regexp = RegexSequence.Definition.PRINTABLE_CHARS, message = "The license expression may only contain printable characters")
+    @ValidSpdxExpression
     private String licenseExpression;
 
     @Persistent

diff --git a/src/main/java/org/dependencytrack/model/validation/SpdxExpressionValidator.java b/src/main/java/org/dependencytrack/model/validation/SpdxExpressionValidator.java
@@ -0,0 +1,40 @@
+/*
+ * This file is part of Dependency-Track.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ * Copyright (c) Steve Springett. All Rights Reserved.
+ */
+package org.dependencytrack.model.validation;
+
+import org.dependencytrack.parser.spdx.expression.SpdxExpressionParser;
+import org.dependencytrack.parser.spdx.expression.model.SpdxExpression;
+
+import javax.validation.ConstraintValidator;
+import javax.validation.ConstraintValidatorContext;
+import java.util.Objects;
+
+public class SpdxExpressionValidator implements ConstraintValidator<ValidSpdxExpression, String> {
+
+    @Override
+    public boolean isValid(final String expressionString, final ConstraintValidatorContext validatorContext) {
+        if (expressionString == null) {
+            // null-ness is expected to be validated using @NotNull
+            return true;
+        }
+
+        return !Objects.equals(new SpdxExpressionParser().parse(expressionString), SpdxExpression.INVALID);
+    }
+
+}
diff --git a/src/main/java/org/dependencytrack/model/validation/ValidSpdxExpression.java b/src/main/java/org/dependencytrack/model/validation/ValidSpdxExpression.java
@@ -0,0 +1,41 @@
+/*
+ * This file is part of Dependency-Track.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ * Copyright (c) Steve Springett. All Rights Reserved.
+ */
+package org.dependencytrack.model.validation;
+
+import javax.validation.Constraint;
+import javax.validation.Payload;
+import java.lang.annotation.Documented;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+@Documented
+@Retention(RetentionPolicy.RUNTIME)
+@Target({ElementType.FIELD, ElementType.PARAMETER})
+@Constraint(validatedBy = SpdxExpressionValidator.class)
+public @interface ValidSpdxExpression {
+
+    String message() default "The license expression must be a valid SPDX expression";
+
+    Class<?>[] groups() default {};
+
+    Class<? extends Payload>[] payload() default {};
+
+}
diff --git a/src/main/java/org/dependencytrack/parser/cyclonedx/util/ModelConverter.java b/src/main/java/org/dependencytrack/parser/cyclonedx/util/ModelConverter.java
@@ -180,16 +180,24 @@ public static Component convertComponent(final org.cyclonedx.model.Component cdx
             }
 
             if (isNotBlank(cdxComponent.getLicenseChoice().getExpression())) {
-                component.setLicenseExpression(trim(cdxComponent.getLicenseChoice().getExpression()));
-
                 // If the expression consists of just one license ID, add it as another option.
                 final var expressionParser = new SpdxExpressionParser();
-                final SpdxExpression expression = expressionParser.parse(component.getLicenseExpression());
-                if (expression.getSpdxLicenseId() != null) {
-                    final var expressionLicense = new org.cyclonedx.model.License();
-                    expressionLicense.setId(expression.getSpdxLicenseId());
-                    expressionLicense.setName(expression.getSpdxLicenseId());
-                    licenseCandidates.add(expressionLicense);
+                final SpdxExpression expression = expressionParser.parse(cdxComponent.getLicenseChoice().getExpression());
+                if (!SpdxExpression.INVALID.equals(expression)) {
+                    component.setLicenseExpression(trim(cdxComponent.getLicenseChoice().getExpression()));
+
+                    if (expression.getSpdxLicenseId() != null) {
+                        final var expressionLicense = new org.cyclonedx.model.License();
+                        expressionLicense.setId(expression.getSpdxLicenseId());
+                        expressionLicense.setName(expression.getSpdxLicenseId());
+                        licenseCandidates.add(expressionLicense);
+                    }
+                } else {
+                    LOGGER.warn("""
+                            Encountered invalid license expression "%s" for \
+                            Component{group=%s, name=%s, version=%s, bomRef=%s}; Skipping\
+                            """.formatted(cdxComponent.getLicenseChoice().getExpression(), component.getGroup(),
+                            component.getName(), component.getVersion(), component.getBomRef()));
                 }
             }
         }

diff --git a/src/main/java/org/dependencytrack/parser/spdx/expression/SpdxExpressionParser.java b/src/main/java/org/dependencytrack/parser/spdx/expression/SpdxExpressionParser.java
@@ -31,7 +31,7 @@
  * SpdxExpressions and SpdxExpressionOperations
  *
  * @author hborchardt
- * @since 4.8.0
+ * @since 4.9.0
  */
 public class SpdxExpressionParser {
 

diff --git a/src/main/java/org/dependencytrack/parser/spdx/expression/model/SpdxExpression.java b/src/main/java/org/dependencytrack/parser/spdx/expression/model/SpdxExpression.java
@@ -25,7 +25,7 @@
  * inner node, containss an operation.
  *
  * @author hborchardt
- * @since 4.8.0
+ * @since 4.9.0
  */
 public class SpdxExpression {
     public static final SpdxExpression INVALID = new SpdxExpression(null);

diff --git a/src/main/java/org/dependencytrack/parser/spdx/expression/model/SpdxExpressionOperation.java b/src/main/java/org/dependencytrack/parser/spdx/expression/model/SpdxExpressionOperation.java
@@ -26,7 +26,7 @@
  * to that operator.
  *
  * @author hborchardt
- * @since 4.8.0
+ * @since 4.9.0
  */
 public class SpdxExpressionOperation {
     private SpdxOperator operator;

diff --git a/src/main/java/org/dependencytrack/parser/spdx/expression/model/SpdxOperator.java b/src/main/java/org/dependencytrack/parser/spdx/expression/model/SpdxOperator.java
@@ -22,7 +22,7 @@
  * One of the SPDX expression operators as defined in the spec, together with their precedence.
  *
  * @author hborchardt
- * @since 4.8.0
+ * @since 4.9.0
  */
 public enum SpdxOperator {
     OR(1, "OR"), AND(2, "AND"), WITH(3, "WITH"), PLUS(4, "+");

diff --git a/src/main/java/org/dependencytrack/persistence/ComponentQueryManager.java b/src/main/java/org/dependencytrack/persistence/ComponentQueryManager.java
@@ -398,6 +398,8 @@ public Component cloneComponent(Component sourceComponent, Project destinationPr
         component.setDescription(sourceComponent.getDescription());
         component.setCopyright(sourceComponent.getCopyright());
         component.setLicense(sourceComponent.getLicense());
+        component.setLicenseExpression(sourceComponent.getLicenseExpression());
+        component.setLicenseUrl(sourceComponent.getLicenseUrl());
         component.setResolvedLicense(sourceComponent.getResolvedLicense());
         component.setAuthor(sourceComponent.getAuthor());
         // TODO Add support for parent component and children components
@@ -427,6 +429,8 @@ public Component updateComponent(Component transientComponent, boolean commitInd
         component.setDescription(transientComponent.getDescription());
         component.setCopyright(transientComponent.getCopyright());
         component.setLicense(transientComponent.getLicense());
+        component.setLicenseExpression(transientComponent.getLicenseExpression());
+        component.setLicenseUrl(transientComponent.getLicenseUrl());
         component.setResolvedLicense(transientComponent.getResolvedLicense());
         component.setParent(transientComponent.getParent());
         component.setCpe(transientComponent.getCpe());

diff --git a/src/main/java/org/dependencytrack/resources/v1/ComponentResource.java b/src/main/java/org/dependencytrack/resources/v1/ComponentResource.java
@@ -333,6 +333,8 @@ public Response createComponent(@PathParam("uuid") String uuid, Component jsonCo
                 validator.validateProperty(jsonComponent, "group"),
                 validator.validateProperty(jsonComponent, "description"),
                 validator.validateProperty(jsonComponent, "license"),
+                validator.validateProperty(jsonComponent, "licenseExpression"),
+                validator.validateProperty(jsonComponent, "licenseUrl"),
                 validator.validateProperty(jsonComponent, "filename"),
                 validator.validateProperty(jsonComponent, "classifier"),
                 validator.validateProperty(jsonComponent, "cpe"),
@@ -387,12 +389,20 @@ public Response createComponent(@PathParam("uuid") String uuid, Component jsonCo
             component.setSha3_512(StringUtils.trimToNull(jsonComponent.getSha3_512()));
             if (resolvedLicense != null) {
                 component.setLicense(null);
+                component.setLicenseExpression(null);
+                component.setLicenseUrl(StringUtils.trimToNull(jsonComponent.getLicenseUrl()));
                 component.setResolvedLicense(resolvedLicense);
-            } else {
-                component.setLicense(StringUtils.trimToNull(jsonComponent.getLicense()));
+            } else if (StringUtils.isNotBlank(jsonComponent.getLicense())) {
+                component.setLicense(StringUtils.trim(jsonComponent.getLicense()));
+                component.setLicenseExpression(null);
+                component.setLicenseUrl(StringUtils.trimToNull(jsonComponent.getLicenseUrl()));
+                component.setResolvedLicense(null);
+            } else if (StringUtils.isNotBlank(jsonComponent.getLicenseExpression())) {
+                component.setLicense(null);
+                component.setLicenseExpression(StringUtils.trim(jsonComponent.getLicenseExpression()));
+                component.setLicenseUrl(null);
                 component.setResolvedLicense(null);
             }
-            component.setLicenseUrl(StringUtils.trimToNull(jsonComponent.getLicenseUrl()));
             component.setParent(parent);
             component.setNotes(StringUtils.trimToNull(jsonComponent.getNotes()));
 
@@ -434,6 +444,7 @@ public Response updateComponent(Component jsonComponent) {
                 validator.validateProperty(jsonComponent, "group"),
                 validator.validateProperty(jsonComponent, "description"),
                 validator.validateProperty(jsonComponent, "license"),
+                validator.validateProperty(jsonComponent, "licenseExpression"),
                 validator.validateProperty(jsonComponent, "licenseUrl"),
                 validator.validateProperty(jsonComponent, "filename"),
                 validator.validateProperty(jsonComponent, "classifier"),
@@ -483,12 +494,25 @@ public Response updateComponent(Component jsonComponent) {
                 final License resolvedLicense = qm.getLicense(jsonComponent.getLicense());
                 if (resolvedLicense != null) {
                     component.setLicense(null);
+                    component.setLicenseExpression(null);
+                    component.setLicenseUrl(StringUtils.trimToNull(jsonComponent.getLicenseUrl()));
                     component.setResolvedLicense(resolvedLicense);
+                } else if (StringUtils.trimToNull(jsonComponent.getLicense()) != null) {
+                    component.setLicense(StringUtils.trim(jsonComponent.getLicense()));
+                    component.setLicenseExpression(null);
+                    component.setLicenseUrl(StringUtils.trimToNull(jsonComponent.getLicenseUrl()));
+                    component.setResolvedLicense(null);
+                } else if (StringUtils.trimToNull(jsonComponent.getLicenseExpression()) != null) {
+                    component.setLicense(null);
+                    component.setLicenseExpression(StringUtils.trim(jsonComponent.getLicenseExpression()));
+                    component.setLicenseUrl(null);
+                    component.setResolvedLicense(null);
                 } else {
-                    component.setLicense(StringUtils.trimToNull(jsonComponent.getLicense()));
+                    component.setLicense(null);
+                    component.setLicenseExpression(null);
+                    component.setLicenseUrl(null);
                     component.setResolvedLicense(null);
                 }
-                component.setLicenseUrl(StringUtils.trimToNull(jsonComponent.getLicenseUrl()));
                 component.setNotes(StringUtils.trimToNull(jsonComponent.getNotes()));
 
                 component = qm.updateComponent(component, true);

diff --git a/src/main/java/org/dependencytrack/tasks/BomUploadProcessingTask.java b/src/main/java/org/dependencytrack/tasks/BomUploadProcessingTask.java
@@ -541,7 +541,6 @@ private static Map<ComponentIdentity, Component> processComponents(final QueryMa
                         final License resolvedLicense = resolveLicense(pm, licenseCache, licenseCandidate.getId());
                         if (resolvedLicense != null) {
                             component.setResolvedLicense(resolvedLicense);
-                            component.setLicense(trimToNull(licenseCandidate.getName()));
                             component.setLicenseUrl(trimToNull(licenseCandidate.getUrl()));
                             break;
                         }
@@ -551,7 +550,6 @@ private static Map<ComponentIdentity, Component> processComponents(final QueryMa
                         final License resolvedCustomLicense = resolveCustomLicense(pm, customLicenseCache, licenseCandidate.getName());
                         if (resolvedCustomLicense != null) {
                             component.setResolvedLicense(resolvedCustomLicense);
-                            component.setLicense(trimToNull(licenseCandidate.getName()));
                             component.setLicenseUrl(trimToNull(licenseCandidate.getUrl()));
                             break;
                         }

diff --git a/src/test/java/org/dependencytrack/AbstractPostgresEnabledTest.java b/src/test/java/org/dependencytrack/AbstractPostgresEnabledTest.java
@@ -64,6 +64,15 @@ public void setUp() throws Exception {
 
     @After
     public void tearDown() {
+        // PersistenceManager will refuse to close when there's an active transaction
+        // that was neither committed nor rolled back. Unfortunately some areas of the
+        // code base can leave such a broken state behind if they run into unexpected
+        // errors. See: https://github.com/DependencyTrack/dependency-track/issues/2677
+        if (!qm.getPersistenceManager().isClosed()
+                && qm.getPersistenceManager().currentTransaction().isActive()) {
+            qm.getPersistenceManager().currentTransaction().rollback();
+        }
+
         PersistenceManagerFactory.tearDown();
         if (postgresContainer != null) {
             postgresContainer.stop();

diff --git a/src/test/java/org/dependencytrack/PersistenceCapableTest.java b/src/test/java/org/dependencytrack/PersistenceCapableTest.java
@@ -45,6 +45,14 @@ public void before() throws Exception {
 
     @After
     public void after() {
+        // PersistenceManager will refuse to close when there's an active transaction
+        // that was neither committed nor rolled back. Unfortunately some areas of the
+        // code base can leave such a broken state behind if they run into unexpected
+        // errors. See: https://github.com/DependencyTrack/dependency-track/issues/2677
+        if (!qm.getPersistenceManager().isClosed()
+                && qm.getPersistenceManager().currentTransaction().isActive()) {
+            qm.getPersistenceManager().currentTransaction().rollback();
+        }
         PersistenceManagerFactory.tearDown();
         KafkaProducerInitializer.tearDown();
     }

diff --git a/src/test/java/org/dependencytrack/model/validation/SpdxExpressionValidatorTest.java b/src/test/java/org/dependencytrack/model/validation/SpdxExpressionValidatorTest.java
@@ -0,0 +1,63 @@
+/*
+ * This file is part of Dependency-Track.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ * Copyright (c) Steve Springett. All Rights Reserved.
+ */
+package org.dependencytrack.model.validation;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import javax.validation.ConstraintViolation;
+import javax.validation.Validation;
+import javax.validation.Validator;
+import javax.validation.ValidatorFactory;
+import java.util.Set;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+public class SpdxExpressionValidatorTest {
+
+    private Validator validator;
+
+    private record TestRecord(@ValidSpdxExpression String expression) {
+    }
+
+    @Before
+    public void setUp() {
+        final ValidatorFactory validatorFactory = Validation.buildDefaultValidatorFactory();
+        validator = validatorFactory.getValidator();
+    }
+
+    @Test
+    public void testWithValidExpression() {
+        final Set<ConstraintViolation<TestRecord>> violations = validator.validate(new TestRecord("Apache-2.0 OR MIT"));
+        assertThat(violations).isEmpty();
+    }
+
+    @Test
+    public void testWithInvalidExpression() {
+        final Set<ConstraintViolation<TestRecord>> violations = validator.validate(new TestRecord("(Apache-2.0"));
+        assertThat(violations).isNotEmpty();
+    }
+
+    @Test
+    public void testWithNullExpression() {
+        final Set<ConstraintViolation<TestRecord>> violations = validator.validate(new TestRecord(null));
+        assertThat(violations).isEmpty();
+    }
+
+}