Remove empty ssl file upload (#518)

* Delete empty file when handling exception

* Add unit test, apply formatting

* Handle case when file.delete() fails

* Prevent empty file creation

* Close outstream when finished writing

* Refactor file write, verify file not created

* Pass outputStreamFunction to constructor

* Apply spotless formatting

* Add integration test

* Move empty cert to test resources

* Rename test

* Remove unneeded assertion

src/main/java/io/cryostat/net/web/http/api/v2/CertificatePostHandler.java

@@ -45,7 +45,6 @@
 import java.security.cert.Certificate;
 import java.security.cert.CertificateEncodingException;
 import java.util.Collection;
-import java.util.Iterator;
 import java.util.function.Function;
 
 import javax.inject.Inject;
@@ -151,16 +150,21 @@ public IntermediateResponse<Path> handle(RequestParameters params) throws ApiExc
             throw new ApiException(409, filePath.toString() + " Certificate already exists");
         }
 
-        try (InputStream fis = fs.newInputStream(certPath);
-                FileOutputStream out = outputStreamFunction.apply(filePath.toFile())) {
-
+        try (InputStream fis = fs.newInputStream(certPath)) {
             Collection<? extends Certificate> certificates = certValidator.parseCertificates(fis);
-            Iterator<? extends Certificate> it = certificates.iterator();
-            while (it.hasNext()) {
-                Certificate certificate = (Certificate) it.next();
-                byte[] buf = certificate.getEncoded();
-                out.write(buf);
+
+            if (certificates.isEmpty()) {
+                throw new ApiException(500, "No certificates found");
+            }
+
+            try (FileOutputStream out = outputStreamFunction.apply(filePath.toFile())) {
+
+                for (Certificate certificate : certificates) {
+                    byte[] buf = certificate.getEncoded();
+                    out.write(buf);
+                }
             }
+
         } catch (IOException ioe) {
             throw new ApiException(500, ioe.getMessage(), ioe);
         } catch (CertificateEncodingException cee) {

src/test/java/io/cryostat/net/web/http/api/v2/CertificatePostHandlerTest.java

@@ -38,6 +38,7 @@
 package io.cryostat.net.web.http.api.v2;
 
 import java.io.ByteArrayInputStream;
+import java.io.File;
 import java.io.FileOutputStream;
 import java.io.InputStream;
 import java.nio.file.Path;
@@ -48,6 +49,7 @@
 import java.util.Iterator;
 import java.util.Set;
 import java.util.concurrent.CompletableFuture;
+import java.util.function.Function;
 
 import io.cryostat.MainModule;
 import io.cryostat.core.log.Logger;
@@ -92,6 +94,7 @@ class CertificatePostHandlerTest {
 
     @Mock RoutingContext ctx;
     @Mock FileOutputStream outStream;
+    @Mock Function<File, FileOutputStream> outputStreamFunction;
     @Mock FileUpload fu;
     @Mock Path truststorePath;
     @Mock Path fileUploadPath;
@@ -103,7 +106,8 @@ class CertificatePostHandlerTest {
     @BeforeEach
     void setup() {
         this.handler =
-                new CertificatePostHandler(auth, env, fs, gson, (file) -> outStream, certValidator);
+                new CertificatePostHandler(
+                        auth, env, fs, gson, outputStreamFunction, certValidator);
 
         HttpServerRequest req = Mockito.mock(HttpServerRequest.class);
         MultiMap headers = MultiMap.caseInsensitiveMultiMap();
@@ -182,6 +186,8 @@ void shouldThrowExceptionIfCertIsMalformed() throws Exception {
 
         ApiException ex = Assertions.assertThrows(ApiException.class, () -> handler.handle(ctx));
         MatcherAssert.assertThat(ex.getFailureReason(), Matchers.equalTo("parsing error"));
+        Mockito.verify(outputStreamFunction, Mockito.never()).apply(Mockito.any());
+        Mockito.verify(outStream, Mockito.never()).write(Mockito.any());
     }
 
     @Test
@@ -200,6 +206,8 @@ void shouldAddCertToTruststore() throws Exception {
 
         InputStream instream = new ByteArrayInputStream("certificate".getBytes());
         Mockito.when(fs.newInputStream(fileUploadPath)).thenReturn(instream);
+
+        Mockito.when(outputStreamFunction.apply(Mockito.any())).thenReturn(outStream);
         Mockito.when(certValidator.parseCertificates(Mockito.any())).thenReturn(certificates);
         Mockito.when(certificates.iterator()).thenReturn(iterator);
         Mockito.when(iterator.hasNext()).thenReturn(true).thenReturn(false);

src/test/java/itest/UploadCertificateIT.java

@@ -0,0 +1,88 @@
+/*
+ * Copyright The Cryostat Authors
+ *
+ * The Universal Permissive License (UPL), Version 1.0
+ *
+ * Subject to the condition set forth below, permission is hereby granted to any
+ * person obtaining a copy of this software, associated documentation and/or data
+ * (collectively the "Software"), free of charge and under any and all copyright
+ * rights in the Software, and any and all patent rights owned or freely
+ * licensable by each licensor hereunder covering either (i) the unmodified
+ * Software as contributed to or provided by such licensor, or (ii) the Larger
+ * Works (as defined below), to deal in both
+ *
+ * (a) the Software, and
+ * (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if
+ * one is included with the Software (each a "Larger Work" to which the Software
+ * is contributed by such licensors),
+ *
+ * without restriction, including without limitation the rights to copy, create
+ * derivative works of, display, perform, and distribute the Software and make,
+ * use, sell, offer for sale, import, export, have made, and have sold the
+ * Software and the Larger Work(s), and to sublicense the foregoing rights on
+ * either these or other terms.
+ *
+ * This license is subject to the following condition:
+ * The above copyright notice and either this complete permission notice or at
+ * a minimum a reference to the UPL must be included in all copies or
+ * substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+package itest;
+
+import java.io.File;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.TimeUnit;
+
+import io.vertx.core.buffer.Buffer;
+import io.vertx.ext.web.client.HttpResponse;
+import io.vertx.ext.web.multipart.MultipartForm;
+import org.hamcrest.MatcherAssert;
+import org.hamcrest.Matchers;
+import org.junit.jupiter.api.Test;
+
+public class UploadCertificateIT extends TestBase {
+
+    static final String CERT_NAME = "cert";
+    static final String FILE_NAME = "empty.cer";
+    static final String TRUSTSTORE_CERT = "truststore/" + FILE_NAME;
+    static final String MEDIA_TYPE = "application/pkix-cert";
+
+    @Test
+    public void shouldNotAddEmptyCertToTrustStore() throws Exception {
+
+        CompletableFuture<Integer> uploadRespFuture = new CompletableFuture<>();
+        ClassLoader classLoader = getClass().getClassLoader();
+        File emptyCert = new File(classLoader.getResource(FILE_NAME).getFile());
+        String path = emptyCert.getAbsolutePath();
+
+        MultipartForm form =
+                MultipartForm.create()
+                        .attribute("name", CERT_NAME)
+                        .binaryFileUpload(CERT_NAME, FILE_NAME, path, MEDIA_TYPE);
+
+        webClient
+                .post(String.format("/api/v2/certificates"))
+                .sendMultipartForm(
+                        form,
+                        ar -> {
+                            if (ar.failed()) {
+                                uploadRespFuture.completeExceptionally(ar.cause());
+                                return;
+                            }
+                            HttpResponse<Buffer> result = ar.result();
+                            uploadRespFuture.complete(result.statusCode());
+                        });
+
+        int statusCode = uploadRespFuture.get(REQUEST_TIMEOUT_SECONDS, TimeUnit.SECONDS);
+
+        MatcherAssert.assertThat(statusCode, Matchers.equalTo(500));
+    }
+}