Merge pull request #3198 from nscuro/issue-3197

Fix NPE for `BOM_PROCESSING_FAILED` notifications when parsing of the BOM failed

pom.xml

@@ -91,6 +91,7 @@
         <lib.cvss-calculator.version>1.4.1</lib.cvss-calculator.version>
         <lib.owasp-rr-calculator.version>1.0.1</lib.owasp-rr-calculator.version>
         <lib.cyclonedx-java.version>8.0.3</lib.cyclonedx-java.version>
+        <lib.greenmail.version>1.6.14</lib.greenmail.version>
         <lib.jaxb.runtime.version>2.3.8</lib.jaxb.runtime.version>
         <lib.json-unit.version>3.2.2</lib.json-unit.version>
         <!--
@@ -436,6 +437,12 @@
             <version>${lib.awaitility.version}</version>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>com.icegreen</groupId>
+            <artifactId>greenmail-junit4</artifactId>
+            <version>${lib.greenmail.version}</version>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 
     <build>

src/main/java/org/dependencytrack/util/NotificationUtil.java

@@ -23,6 +23,7 @@
 import alpine.notification.NotificationLevel;
 import org.apache.commons.io.FileUtils;
 import org.dependencytrack.model.Analysis;
+import org.dependencytrack.model.Bom;
 import org.dependencytrack.model.Component;
 import org.dependencytrack.model.ComponentIdentity;
 import org.dependencytrack.model.ConfigPropertyConstants;
@@ -66,6 +67,7 @@
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
 
@@ -450,9 +452,9 @@ public static JsonObject toJson(final BomProcessingFailed vo) {
         }
         if (vo.getBom() != null) {
             builder.add("bom", Json.createObjectBuilder()
-                    .add("content", vo.getBom())
-                    .add("format", vo.getFormat().getFormatShortName())
-                    .add("specVersion", vo.getSpecVersion()).build()
+                    .add("content", Optional.ofNullable(vo.getBom()).orElse("Unknown"))
+                    .add("format", Optional.ofNullable(vo.getFormat()).map(Bom.Format::getFormatShortName).orElse("Unknown"))
+                    .add("specVersion", Optional.ofNullable(vo.getSpecVersion()).orElse("Unknown")).build()
             );
         }
         if (vo.getCause() != null) {

src/main/resources/templates/notification/publisher/webhook.peb

@@ -6,6 +6,6 @@
     "timestamp": "{{ notification.timestamp }}",
     "title": "{{ notification.title | escape(strategy="json") }}",
     "content": "{{ notification.content | escape(strategy="json") }}",
-    "subject": {{ subjectJson | raw }}
+    "subject": {% if subjectJson != null %}{{ subjectJson | raw }}{% else %}null{% endif %}
   }
 }

src/test/java/org/dependencytrack/notification/publisher/AbstractPublisherTest.java

@@ -35,13 +35,16 @@
 import org.dependencytrack.notification.NotificationScope;
 import org.dependencytrack.notification.vo.AnalysisDecisionChange;
 import org.dependencytrack.notification.vo.BomConsumedOrProcessed;
+import org.dependencytrack.notification.vo.BomProcessingFailed;
 import org.dependencytrack.notification.vo.NewVulnerabilityIdentified;
 import org.junit.Test;
 
 import javax.json.Json;
 import javax.json.JsonObject;
 import javax.json.JsonObjectBuilder;
 import java.math.BigDecimal;
+import java.time.LocalDateTime;
+import java.time.ZoneOffset;
 import java.util.List;
 import java.util.Set;
 import java.util.UUID;
@@ -69,6 +72,41 @@ public void testInformWithBomConsumedNotification() {
                 .title(NotificationConstants.Title.BOM_CONSUMED)
                 .content("A CycloneDX BOM was consumed and will be processed")
                 .level(NotificationLevel.INFORMATIONAL)
+                .timestamp(LocalDateTime.ofEpochSecond(66666, 666, ZoneOffset.UTC))
+                .subject(subject);
+
+        assertThatNoException()
+                .isThrownBy(() -> publisherInstance.inform(notification, createConfig()));
+    }
+
+    @Test
+    public void testInformWithBomProcessingFailedNotification() {
+        final var subject = new BomProcessingFailed(createProject(), "bomContent", "cause", Bom.Format.CYCLONEDX, "1.5");
+
+        final var notification = new Notification()
+                .scope(NotificationScope.PORTFOLIO)
+                .group(NotificationGroup.BOM_PROCESSING_FAILED)
+                .title(NotificationConstants.Title.BOM_PROCESSING_FAILED)
+                .content("An error occurred while processing a BOM")
+                .level(NotificationLevel.ERROR)
+                .timestamp(LocalDateTime.ofEpochSecond(66666, 666, ZoneOffset.UTC))
+                .subject(subject);
+
+        assertThatNoException()
+                .isThrownBy(() -> publisherInstance.inform(notification, createConfig()));
+    }
+
+    @Test // https://github.com/DependencyTrack/dependency-track/issues/3197
+    public void testInformWithBomProcessingFailedNotificationAndNoSpecVersionInSubject() {
+        final var subject = new BomProcessingFailed(createProject(), "bomContent", "cause", Bom.Format.CYCLONEDX, null);
+
+        final var notification = new Notification()
+                .scope(NotificationScope.PORTFOLIO)
+                .group(NotificationGroup.BOM_PROCESSING_FAILED)
+                .title(NotificationConstants.Title.BOM_PROCESSING_FAILED)
+                .content("An error occurred while processing a BOM")
+                .level(NotificationLevel.ERROR)
+                .timestamp(LocalDateTime.ofEpochSecond(66666, 666, ZoneOffset.UTC))
                 .subject(subject);
 
         assertThatNoException()
@@ -82,7 +120,8 @@ public void testInformWithDataSourceMirroringNotification() {
                 .group(NotificationGroup.DATASOURCE_MIRRORING)
                 .title(NotificationConstants.Title.GITHUB_ADVISORY_MIRROR)
                 .content("An error occurred mirroring the contents of GitHub Advisories. Check log for details.")
-                .level(NotificationLevel.ERROR);
+                .level(NotificationLevel.ERROR)
+                .timestamp(LocalDateTime.ofEpochSecond(66666, 666, ZoneOffset.UTC));
 
         assertThatNoException()
                 .isThrownBy(() -> publisherInstance.inform(notification, createConfig()));
@@ -103,6 +142,7 @@ public void testInformWithNewVulnerabilityNotification() {
                 .level(NotificationLevel.INFORMATIONAL)
                 .title(NotificationConstants.Title.NEW_VULNERABILITY)
                 .content("")
+                .timestamp(LocalDateTime.ofEpochSecond(66666, 666, ZoneOffset.UTC))
                 .subject(subject);
 
         assertThatNoException()
@@ -124,6 +164,7 @@ public void testInformWithProjectAuditChangeNotification() {
                 .level(NotificationLevel.INFORMATIONAL)
                 .title(NotificationConstants.Title.ANALYSIS_DECISION_SUPPRESSED)
                 .content("")
+                .timestamp(LocalDateTime.ofEpochSecond(66666, 666, ZoneOffset.UTC))
                 .subject(subject);
 
         assertThatNoException()

src/test/java/org/dependencytrack/notification/publisher/AbstractWebhookPublisherTest.java

@@ -25,6 +25,7 @@
 import javax.json.JsonObjectBuilder;
 
 import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
+import static com.github.tomakehurst.wiremock.client.WireMock.anyUrl;
 import static com.github.tomakehurst.wiremock.client.WireMock.post;
 import static com.github.tomakehurst.wiremock.client.WireMock.stubFor;
 import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.options;
@@ -40,7 +41,7 @@ public abstract class AbstractWebhookPublisherTest<T extends AbstractWebhookPubl
     }
 
     @Before
-    public void setUp() {
+    public void setUp() throws Exception {
         qm.createConfigProperty(
                 GENERAL_BASE_URL.getGroupName(),
                 GENERAL_BASE_URL.getPropertyName(),
@@ -49,7 +50,7 @@ public void setUp() {
                 GENERAL_BASE_URL.getDescription()
         );
 
-        stubFor(post("/")
+        stubFor(post(anyUrl())
                 .willReturn(aResponse()
                         .withStatus(200)));
     }