From 7d1d84e843faf7572c48705116a129ebbafa2857 Mon Sep 17 00:00:00 2001
From: Janelle Law <jalaw@redhat.com>
Date: Tue, 27 Jul 2021 08:50:58 -0400
Subject: [PATCH] fix(certificates): Empty cert uploads throw 400 instead of
 500 (#605)

* Add no cert upload test

* Throw 400 on empty cert upload
---
 .../http/api/v2/CertificatePostHandler.java   |  5 ++-
 src/test/java/itest/UploadCertificateIT.java  | 42 +++++++++++++------
 2 files changed, 33 insertions(+), 14 deletions(-)

diff --git a/src/main/java/io/cryostat/net/web/http/api/v2/CertificatePostHandler.java b/src/main/java/io/cryostat/net/web/http/api/v2/CertificatePostHandler.java
index 9e70dfefe5..b251fee83c 100644
--- a/src/main/java/io/cryostat/net/web/http/api/v2/CertificatePostHandler.java
+++ b/src/main/java/io/cryostat/net/web/http/api/v2/CertificatePostHandler.java
@@ -38,6 +38,7 @@
 package io.cryostat.net.web.http.api.v2;
 
 import java.io.File;
+import java.io.FileNotFoundException;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
@@ -154,7 +155,7 @@ public IntermediateResponse<Path> handle(RequestParameters params) throws ApiExc
             Collection<? extends Certificate> certificates = certValidator.parseCertificates(fis);
 
             if (certificates.isEmpty()) {
-                throw new ApiException(500, "No certificates found");
+                throw new FileNotFoundException("No certificates found");
             }
 
             try (FileOutputStream out = outputStreamFunction.apply(filePath.toFile())) {
@@ -165,6 +166,8 @@ public IntermediateResponse<Path> handle(RequestParameters params) throws ApiExc
                 }
             }
 
+        } catch (FileNotFoundException e) {
+            throw new ApiException(400, e.getMessage(), e);
         } catch (IOException ioe) {
             throw new ApiException(500, ioe.getMessage(), ioe);
         } catch (CertificateEncodingException cee) {
diff --git a/src/test/java/itest/UploadCertificateIT.java b/src/test/java/itest/UploadCertificateIT.java
index fac0ce294a..a046174c8b 100644
--- a/src/test/java/itest/UploadCertificateIT.java
+++ b/src/test/java/itest/UploadCertificateIT.java
@@ -39,14 +39,15 @@
 
 import java.io.File;
 import java.util.concurrent.CompletableFuture;
-import java.util.concurrent.TimeUnit;
+import java.util.concurrent.ExecutionException;
 
-import io.vertx.core.buffer.Buffer;
-import io.vertx.ext.web.client.HttpResponse;
+import io.vertx.core.json.JsonObject;
+import io.vertx.ext.web.handler.impl.HttpStatusException;
 import io.vertx.ext.web.multipart.MultipartForm;
 import itest.bases.StandardSelfTest;
 import org.hamcrest.MatcherAssert;
 import org.hamcrest.Matchers;
+import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 
 public class UploadCertificateIT extends StandardSelfTest {
@@ -55,11 +56,12 @@ public class UploadCertificateIT extends StandardSelfTest {
     static final String FILE_NAME = "empty.cer";
     static final String TRUSTSTORE_CERT = "truststore/" + FILE_NAME;
     static final String MEDIA_TYPE = "application/pkix-cert";
+    static final String REQ_URL = String.format("/api/v2/certificates");
 
     @Test
     public void shouldNotAddEmptyCertToTrustStore() throws Exception {
 
-        CompletableFuture<Integer> uploadRespFuture = new CompletableFuture<>();
+        CompletableFuture<Integer> response = new CompletableFuture<>();
         ClassLoader classLoader = getClass().getClassLoader();
         File emptyCert = new File(classLoader.getResource(FILE_NAME).getFile());
         String path = emptyCert.getAbsolutePath();
@@ -70,20 +72,34 @@ public void shouldNotAddEmptyCertToTrustStore() throws Exception {
                         .binaryFileUpload(CERT_NAME, FILE_NAME, path, MEDIA_TYPE);
 
         webClient
-                .post(String.format("/api/v2/certificates"))
+                .post(REQ_URL)
                 .sendMultipartForm(
                         form,
                         ar -> {
-                            if (ar.failed()) {
-                                uploadRespFuture.completeExceptionally(ar.cause());
-                                return;
-                            }
-                            HttpResponse<Buffer> result = ar.result();
-                            uploadRespFuture.complete(result.statusCode());
+                            assertRequestStatus(ar, response);
                         });
+        ExecutionException ex =
+                Assertions.assertThrows(ExecutionException.class, () -> response.get());
+        MatcherAssert.assertThat(
+                ((HttpStatusException) ex.getCause()).getStatusCode(), Matchers.equalTo(400));
+        MatcherAssert.assertThat(ex.getCause().getMessage(), Matchers.equalTo("Bad Request"));
+    }
+
+    @Test
+    public void shouldThrowWhenPostingWithoutCert() throws Exception {
 
-        int statusCode = uploadRespFuture.get(REQUEST_TIMEOUT_SECONDS, TimeUnit.SECONDS);
+        CompletableFuture<JsonObject> response = new CompletableFuture<>();
 
-        MatcherAssert.assertThat(statusCode, Matchers.equalTo(500));
+        webClient
+                .post(REQ_URL)
+                .send(
+                        ar -> {
+                            assertRequestStatus(ar, response);
+                        });
+        ExecutionException ex =
+                Assertions.assertThrows(ExecutionException.class, () -> response.get());
+        MatcherAssert.assertThat(
+                ((HttpStatusException) ex.getCause()).getStatusCode(), Matchers.equalTo(400));
+        MatcherAssert.assertThat(ex.getCause().getMessage(), Matchers.equalTo("Bad Request"));
     }
 }