Merge pull request #4147 from nscuro/issue-4027

Handle existing duplicate component properties
DependencyTrack · Sep 13, 2024 · b4edb30 · b4edb30
2 parents 2b0bfe1 + abbeedc
commit b4edb30
Show file tree

Hide file tree

Showing 2 changed files with 89 additions and 0 deletions.
diff --git a/src/main/java/org/dependencytrack/persistence/ComponentQueryManager.java b/src/main/java/org/dependencytrack/persistence/ComponentQueryManager.java
@@ -863,11 +863,32 @@ public void synchronizeComponentProperties(final Component component, final List
 
         // Group properties by group, name, and value. Because CycloneDX supports duplicate
         // property names, uniqueness can only be determined by also considering the value.
+        final var existingPropertyIdentitiesSeen = new HashSet<ComponentProperty.Identity>();
+        final var existingDuplicateProperties = new HashSet<ComponentProperty>();
         final var existingPropertiesByIdentity = component.getProperties().stream()
+                // The legacy BOM processing in <= 4.11.x allowed duplicates to be persisted.
+                // Collectors#toMap fails upon encounter of duplicate keys.
+                // Prevent existing duplicates from breaking this.
+                // https://github.com/DependencyTrack/dependency-track/issues/4027
+                .filter(property -> {
+                    final var identity = new ComponentProperty.Identity(property);
+                    final boolean isUnique = existingPropertyIdentitiesSeen.add(identity);
+                    if (!isUnique) {
+                        existingDuplicateProperties.add(property);
+                    }
+
+                    return isUnique;
+                })
                 .collect(Collectors.toMap(ComponentProperty.Identity::new, Function.identity()));
+        final var incomingPropertyIdentitiesSeen = new HashSet<ComponentProperty.Identity>();
         final var incomingPropertiesByIdentity = properties.stream()
+                .filter(property -> incomingPropertyIdentitiesSeen.add(new ComponentProperty.Identity(property)))
                 .collect(Collectors.toMap(ComponentProperty.Identity::new, Function.identity()));
 
+        if (!existingDuplicateProperties.isEmpty()) {
+            pm.deletePersistentAll(existingDuplicateProperties);
+        }
+
         final var propertyIdentities = new HashSet<ComponentProperty.Identity>();
         propertyIdentities.addAll(existingPropertiesByIdentity.keySet());
         propertyIdentities.addAll(incomingPropertiesByIdentity.keySet());

diff --git a/src/test/java/org/dependencytrack/tasks/BomUploadProcessingTaskTest.java b/src/test/java/org/dependencytrack/tasks/BomUploadProcessingTaskTest.java
@@ -53,6 +53,7 @@
 import org.junit.Before;
 import org.junit.Test;
 
+import javax.jdo.JDOObjectNotFoundException;
 import java.nio.charset.StandardCharsets;
 import java.time.Duration;
 import java.util.ArrayList;
@@ -65,6 +66,7 @@
 
 import static org.apache.commons.io.IOUtils.resourceToByteArray;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 import static org.assertj.core.api.Assertions.assertThatNoException;
 import static org.assertj.core.api.Assertions.fail;
 import static org.awaitility.Awaitility.await;
@@ -979,6 +981,72 @@ public void informWithExistingComponentPropertiesAndBomWithComponentProperties()
         });
     }
 
+    @Test
+    public void informWithExistingDuplicateComponentPropertiesAndBomWithDuplicateComponentProperties() {
+        final var project = new Project();
+        project.setName("acme-app");
+        qm.persist(project);
+
+        final var component = new Component();
+        component.setProject(project);
+        component.setName("acme-lib");
+        component.setClassifier(Classifier.LIBRARY);
+        qm.persist(component);
+
+        final var componentPropertyA = new ComponentProperty();
+        componentPropertyA.setComponent(component);
+        componentPropertyA.setPropertyName("foo");
+        componentPropertyA.setPropertyValue("bar");
+        componentPropertyA.setPropertyType(PropertyType.STRING);
+        qm.persist(componentPropertyA);
+
+        final var componentPropertyB = new ComponentProperty();
+        componentPropertyB.setComponent(component);
+        componentPropertyB.setPropertyName("foo");
+        componentPropertyB.setPropertyValue("bar");
+        componentPropertyB.setPropertyType(PropertyType.STRING);
+        qm.persist(componentPropertyB);
+
+        final var bomUploadEvent = new BomUploadEvent(qm.detach(Project.class, project.getId()), """
+                {
+                  "bomFormat": "CycloneDX",
+                  "specVersion": "1.4",
+                  "serialNumber": "urn:uuid:3e671687-395b-41f5-a30f-a58921a69b79",
+                  "version": 1,
+                  "components": [
+                    {
+                      "type": "library",
+                      "name": "acme-lib",
+                      "properties": [
+                        {
+                          "name": "foo",
+                          "value": "bar"
+                        },
+                        {
+                          "name": "foo",
+                          "value": "bar"
+                        }
+                      ]
+                    }
+                  ]
+                }
+                """.getBytes());
+        new BomUploadProcessingTask().inform(bomUploadEvent);
+        awaitBomProcessedNotification(bomUploadEvent);
+
+        qm.getPersistenceManager().evictAll();
+        assertThatNoException()
+                .isThrownBy(() -> qm.getPersistenceManager().refresh(componentPropertyA));
+        assertThatExceptionOfType(JDOObjectNotFoundException.class)
+                .isThrownBy(() -> qm.getPersistenceManager().refresh(componentPropertyB));
+        assertThat(component.getProperties()).satisfiesExactly(property -> {
+            assertThat(property.getGroupName()).isNull();
+            assertThat(property.getPropertyName()).isEqualTo("foo");
+            assertThat(property.getPropertyValue()).isEqualTo("bar");
+            assertThat(property.getUuid()).isEqualTo(componentPropertyA.getUuid());
+        });
+    }
+
     @Test
     public void informWithLicenseResolutionByNameTest() {
         final var license = new License();