Skip to content

Commit e845d91

Browse files
nscuronscuro
committed
Make component identity matching strict
To address DependencyTrack/dependency-track#2519 (comment). Also add regression test for this specific issue. Signed-off-by: nscuro <[email protected]>
1 parent f0b1303 commit e845d91

File tree

6 files changed

+63432
-38
lines changed

6 files changed

+63432
-38
lines changed

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

+8-3
Original file line numberDiff line numberDiff line change
@@ -424,19 +424,24 @@ public Component matchSingleIdentity(final Project project, final ComponentIdent
424424
final var params = new HashMap<String, Object>();
425425

426426
if (cid.getPurl() != null) {
427-
filterParts.add("((purl != null && purl == :purl) || (purlCoordinates != null && purlCoordinates == :purlCoordinates))");
427+
filterParts.add("(purl != null && purl == :purl)");
428428
params.put("purl", cid.getPurl().canonicalize());
429-
params.put("purlCoordinates", cid.getPurlCoordinates().canonicalize());
429+
} else {
430+
filterParts.add("purl == null");
430431
}
431432

432433
if (cid.getCpe() != null) {
433434
filterParts.add("(cpe != null && cpe == :cpe)");
434435
params.put("cpe", cid.getCpe());
436+
} else {
437+
filterParts.add("cpe == null");
435438
}
436439

437440
if (cid.getSwidTagId() != null) {
438441
filterParts.add("(swidTagId != null && swidTagId == :swidTagId)");
439442
params.put("swidTagId", cid.getSwidTagId());
443+
} else {
444+
filterParts.add("swidTagId == null");
440445
}
441446

442447
var coordinatesFilter = "(";
@@ -457,7 +462,7 @@ public Component matchSingleIdentity(final Project project, final ComponentIdent
457462
coordinatesFilter += ")";
458463
filterParts.add(coordinatesFilter);
459464

460-
final var filter = "project == :project && (" + String.join(" || ", filterParts) + ")";
465+
final var filter = "project == :project && (" + String.join(" && ", filterParts) + ")";
461466
params.put("project", project);
462467

463468
final Query<Component> query = pm.newQuery(Component.class, filter);

‎src/main/java/org/dependencytrack/tasks/BomUploadProcessingTask.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,7 @@ private static Project processMetadataComponent(final Context ctx, final Persist
331331
changed |= applyIfChanged(project, metadataComponent, Project::getPublisher, project::setPublisher);
332332
changed |= applyIfChanged(project, metadataComponent, Project::getClassifier, project::setClassifier);
333333
// TODO: Currently these properties are "decoupled" from the BOM and managed directly by DT users.
334-
// Perhaps there could be a flag for BOM uploads saying "use BOM properties" or something?
334+
// Perhaps there could be a flag for BOM uploads saying "use BOM properties" or something?
335335
// changed |= applyIfChanged(project, metadataComponent, Project::getGroup, project::setGroup);
336336
// changed |= applyIfChanged(project, metadataComponent, Project::getName, project::setName);
337337
// changed |= applyIfChanged(project, metadataComponent, Project::getVersion, project::setVersion);

‎src/test/java/org/dependencytrack/resources/v1/BomResourceTest.java

+6-6
Original file line numberDiff line numberDiff line change
@@ -668,7 +668,7 @@ public void exportComponentAsCycloneDxInvalid() {
668668
public void uploadBomTest() throws Exception {
669669
initializeWithPermissions(Permissions.BOM_UPLOAD);
670670
Project project = qm.createProject("Acme Example", null, "1.0", null, null, null, true, false);
671-
File file = new File(IOUtils.resourceToURL("/bom-1.xml").toURI());
671+
File file = new File(IOUtils.resourceToURL("/unit/bom-1.xml").toURI());
672672
String bomString = Base64.getEncoder().encodeToString(FileUtils.readFileToByteArray(file));
673673
BomSubmitRequest request = new BomSubmitRequest(project.getUuid().toString(), null, null, false, bomString);
674674
Response response = target(V1_BOM).request()
@@ -684,7 +684,7 @@ public void uploadBomTest() throws Exception {
684684
@Test
685685
public void uploadBomInvalidProjectTest() throws Exception {
686686
initializeWithPermissions(Permissions.BOM_UPLOAD);
687-
File file = new File(IOUtils.resourceToURL("/bom-1.xml").toURI());
687+
File file = new File(IOUtils.resourceToURL("/unit/bom-1.xml").toURI());
688688
String bomString = Base64.getEncoder().encodeToString(FileUtils.readFileToByteArray(file));
689689
BomSubmitRequest request = new BomSubmitRequest(UUID.randomUUID().toString(), null, null, false, bomString);
690690
Response response = target(V1_BOM).request()
@@ -699,7 +699,7 @@ public void uploadBomInvalidProjectTest() throws Exception {
699699
@Test
700700
public void uploadBomAutoCreateTest() throws Exception {
701701
initializeWithPermissions(Permissions.BOM_UPLOAD, Permissions.PROJECT_CREATION_UPLOAD);
702-
File file = new File(IOUtils.resourceToURL("/bom-1.xml").toURI());
702+
File file = new File(IOUtils.resourceToURL("/unit/bom-1.xml").toURI());
703703
String bomString = Base64.getEncoder().encodeToString(FileUtils.readFileToByteArray(file));
704704
BomSubmitRequest request = new BomSubmitRequest(null, "Acme Example", "1.0", true, bomString);
705705
Response response = target(V1_BOM).request()
@@ -716,7 +716,7 @@ public void uploadBomAutoCreateTest() throws Exception {
716716

717717
@Test
718718
public void uploadBomUnauthorizedTest() throws Exception {
719-
File file = new File(IOUtils.resourceToURL("/bom-1.xml").toURI());
719+
File file = new File(IOUtils.resourceToURL("/unit/bom-1.xml").toURI());
720720
String bomString = Base64.getEncoder().encodeToString(FileUtils.readFileToByteArray(file));
721721
BomSubmitRequest request = new BomSubmitRequest(null, "Acme Example", "1.0", true, bomString);
722722
Response response = target(V1_BOM).request()
@@ -730,7 +730,7 @@ public void uploadBomUnauthorizedTest() throws Exception {
730730
@Test
731731
public void uploadBomAutoCreateTestWithParentTest() throws Exception {
732732
initializeWithPermissions(Permissions.BOM_UPLOAD, Permissions.PROJECT_CREATION_UPLOAD);
733-
File file = new File(IOUtils.resourceToURL("/bom-1.xml").toURI());
733+
File file = new File(IOUtils.resourceToURL("/unit/bom-1.xml").toURI());
734734
String bomString = Base64.getEncoder().encodeToString(FileUtils.readFileToByteArray(file));
735735
// Upload parent project
736736
BomSubmitRequest request = new BomSubmitRequest(null, "Acme Parent", "1.0", true, bomString);
@@ -794,7 +794,7 @@ public void uploadBomAutoCreateTestWithParentTest() throws Exception {
794794
@Test
795795
public void uploadBomInvalidParentTest() throws Exception {
796796
initializeWithPermissions(Permissions.BOM_UPLOAD, Permissions.PROJECT_CREATION_UPLOAD);
797-
File file = new File(IOUtils.resourceToURL("/bom-1.xml").toURI());
797+
File file = new File(IOUtils.resourceToURL("/unit/bom-1.xml").toURI());
798798
String bomString = Base64.getEncoder().encodeToString(FileUtils.readFileToByteArray(file));
799799
BomSubmitRequest request = new BomSubmitRequest(null, "Acme Example", "1.0", true, UUID.randomUUID().toString(), null, null, bomString);
800800
Response response = target(V1_BOM).request()

‎src/test/java/org/dependencytrack/tasks/BomUploadProcessingTaskTest.java

+48-28
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import org.junit.Before;
3535
import org.junit.Test;
3636

37+
import java.io.File;
3738
import java.nio.file.Files;
3839
import java.nio.file.Path;
3940
import java.nio.file.Paths;
@@ -66,13 +67,7 @@ public void setUp() {
6667
public void informTest() throws Exception {
6768
Project project = qm.createProject("Acme Example", null, "1.0", null, null, null, true, false);
6869

69-
// The task will delete the input file after processing it,
70-
// so create a temporary copy to not impact other tests.
71-
final Path bomFilePath = Files.createTempFile(null, null);
72-
Files.copy(Paths.get(resourceToURL("/bom-1.xml").toURI()), bomFilePath, StandardCopyOption.REPLACE_EXISTING);
73-
final var bomFile = bomFilePath.toFile();
74-
75-
final var bomUploadEvent = new BomUploadEvent(qm.detach(Project.class, project.getId()), bomFile);
70+
final var bomUploadEvent = new BomUploadEvent(qm.detach(Project.class, project.getId()), createTempBomFile("bom-1.xml"));
7671
new BomUploadProcessingTask().inform(bomUploadEvent);
7772
assertConditionWithTimeout(() -> kafkaMockProducer.history().size() >= 5, Duration.ofSeconds(5));
7873
assertThat(kafkaMockProducer.history()).satisfiesExactly(
@@ -111,13 +106,7 @@ public void informTest() throws Exception {
111106
public void informWithEmptyBomTest() throws Exception {
112107
Project project = qm.createProject("Acme Example", null, "1.0", null, null, null, true, false);
113108

114-
// The task will delete the input file after processing it,
115-
// so create a temporary copy to not impact other tests.
116-
final Path bomFilePath = Files.createTempFile(null, null);
117-
Files.copy(Paths.get(resourceToURL("/unit/bom-empty.json").toURI()), bomFilePath, StandardCopyOption.REPLACE_EXISTING);
118-
final var bomFile = bomFilePath.toFile();
119-
120-
final var bomUploadEvent = new BomUploadEvent(qm.detach(Project.class, project.getId()), bomFile);
109+
final var bomUploadEvent = new BomUploadEvent(qm.detach(Project.class, project.getId()), createTempBomFile("bom-empty.json"));
121110
new BomUploadProcessingTask().inform(bomUploadEvent);
122111
assertConditionWithTimeout(() -> kafkaMockProducer.history().size() >= 3, Duration.ofSeconds(5));
123112
assertThat(kafkaMockProducer.history()).satisfiesExactly(
@@ -141,13 +130,7 @@ public void informWithEmptyBomTest() throws Exception {
141130
public void informWithInvalidBomTest() throws Exception {
142131
Project project = qm.createProject("Acme Example", null, "1.0", null, null, null, true, false);
143132

144-
// The task will delete the input file after processing it,
145-
// so create a temporary copy to not impact other tests.
146-
final Path bomFilePath = Files.createTempFile(null, null);
147-
Files.copy(Paths.get(resourceToURL("/unit/bom-invalid.json").toURI()), bomFilePath, StandardCopyOption.REPLACE_EXISTING);
148-
final var bomFile = bomFilePath.toFile();
149-
150-
new BomUploadProcessingTask().inform(new BomUploadEvent(qm.detach(Project.class, project.getId()), bomFile));
133+
new BomUploadProcessingTask().inform(new BomUploadEvent(qm.detach(Project.class, project.getId()), createTempBomFile("bom-invalid.json")));
151134
assertConditionWithTimeout(() -> kafkaMockProducer.history().size() >= 2, Duration.ofSeconds(5));
152135
assertThat(kafkaMockProducer.history()).satisfiesExactly(
153136
event -> assertThat(event.topic()).isEqualTo(KafkaTopics.NOTIFICATION_PROJECT_CREATED.name()),
@@ -184,13 +167,7 @@ public void informWithInvalidBomTest() throws Exception {
184167
public void informWithBloatedBomTest() throws Exception {
185168
final var project = qm.createProject("Acme Example", null, "1.0", null, null, null, true, false);
186169

187-
// The task will delete the input file after processing it,
188-
// so create a temporary copy to not impact other tests.
189-
final Path bomFilePath = Files.createTempFile(null, null);
190-
Files.copy(Paths.get(resourceToURL("/unit/bloated.bom.json").toURI()), bomFilePath, StandardCopyOption.REPLACE_EXISTING);
191-
final var bomFile = bomFilePath.toFile();
192-
193-
final var bomUploadEvent = new BomUploadEvent(qm.detach(Project.class, project.getId()), bomFile);
170+
final var bomUploadEvent = new BomUploadEvent(qm.detach(Project.class, project.getId()), createTempBomFile("bom-bloated.json"));
194171
new BomUploadProcessingTask().inform(bomUploadEvent);
195172

196173
assertThat(kafkaMockProducer.history())
@@ -276,4 +253,47 @@ public void informWithBloatedBomTest() throws Exception {
276253
assertThat(repoMetaAnalysisCommandsSent).isEqualTo(9056);
277254
}
278255

256+
@Test // https://github.com/DependencyTrack/dependency-track/issues/2519
257+
public void informIssue2519Test() throws Exception {
258+
final var project = qm.createProject("Acme Example", null, "1.0", null, null, null, true, false);
259+
260+
var bomUploadEvent = new BomUploadEvent(qm.detach(Project.class, project.getId()), createTempBomFile("bom-issue2519.xml"));
261+
new BomUploadProcessingTask().inform(bomUploadEvent);
262+
263+
// Make sure processing did not fail.
264+
assertThat(kafkaMockProducer.history())
265+
.noneSatisfy(record -> {
266+
assertThat(record.topic()).isEqualTo(KafkaTopics.NOTIFICATION_BOM.name());
267+
final Notification notification = deserializeValue(KafkaTopics.NOTIFICATION_BOM, record);
268+
assertThat(notification.getGroup()).isEqualTo(GROUP_BOM_PROCESSING_FAILED);
269+
});
270+
271+
// Ensure the expected amount of components is present.
272+
assertThat(qm.getAllComponents(project)).hasSize(1756);
273+
274+
// Upload the same BOM again a few times.
275+
// Ensure processing does not fail, and the number of components ingested doesn't change.
276+
for (int i = 0; i < 3; i++) {
277+
bomUploadEvent = new BomUploadEvent(qm.detach(Project.class, project.getId()), createTempBomFile("bom-issue2519.xml"));
278+
new BomUploadProcessingTask().inform(bomUploadEvent);
279+
280+
assertThat(kafkaMockProducer.history())
281+
.noneSatisfy(record -> {
282+
assertThat(record.topic()).isEqualTo(KafkaTopics.NOTIFICATION_BOM.name());
283+
final Notification notification = deserializeValue(KafkaTopics.NOTIFICATION_BOM, record);
284+
assertThat(notification.getGroup()).isEqualTo(GROUP_BOM_PROCESSING_FAILED);
285+
});
286+
287+
assertThat(qm.getAllComponents(project)).hasSize(1756);
288+
}
289+
}
290+
291+
private static File createTempBomFile(final String testFileName) throws Exception {
292+
// The task will delete the input file after processing it,
293+
// so create a temporary copy to not impact other tests.
294+
final Path bomFilePath = Files.createTempFile(null, null);
295+
Files.copy(Paths.get(resourceToURL("/unit/" + testFileName).toURI()), bomFilePath, StandardCopyOption.REPLACE_EXISTING);
296+
return bomFilePath.toFile();
297+
}
298+
279299
}

‎src/test/resources/bom-1.xml ‎src/test/resources/unit/bom-1.xml

File renamed without changes.

0 commit comments

Comments
 (0)