fix(templates): Throw 400 when posting unexpected file upload name (#602

)

* Throw when posting unexpected file upload

* Add itest, fix formatting

* Rename itest to match unit test

* Remove second unused file upload

src/main/java/io/cryostat/net/web/http/api/v1/TemplatesPostHandler.java

@@ -110,8 +110,10 @@ public void handleAuthenticated(RoutingContext ctx) throws Exception {
                 Path path = fs.pathOf(u.uploadedFileName());
                 try (InputStream is = fs.newInputStream(path)) {
                     if (!"template".equals(u.name())) {
-                        logger.info("Received unexpected file upload named {}", u.name());
-                        continue;
+                        throw new HttpStatusException(
+                                400,
+                                String.format(
+                                        "Received unexpected file upload named {}", u.name()));
                     }
                     notificationFactory
                             .createBuilder()

src/test/java/io/cryostat/net/web/http/api/v1/TemplatesPostHandlerTest.java

@@ -149,39 +149,53 @@ void shouldThrowIfXmlInvalid() throws Exception {
         Mockito.verify(fs).deleteIfExists(uploadPath);
     }
 
+    @Test
+    void shouldThrowIfTemplateUploadNameInvalid() throws Exception {
+        RoutingContext ctx = Mockito.mock(RoutingContext.class);
+
+        FileUpload upload = Mockito.mock(FileUpload.class);
+        Mockito.when(upload.name()).thenReturn("invalidUploadName");
+        Mockito.when(upload.uploadedFileName()).thenReturn("/file-uploads/abcd-1234");
+
+        Mockito.when(ctx.fileUploads()).thenReturn(Set.of(upload));
+
+        Path uploadPath = Mockito.mock(Path.class);
+        Mockito.when(fs.pathOf("/file-uploads/abcd-1234")).thenReturn(uploadPath);
+
+        InputStream stream = Mockito.mock(InputStream.class);
+        Mockito.when(fs.newInputStream(Mockito.any())).thenReturn(stream);
+
+        HttpStatusException ex =
+                Assertions.assertThrows(
+                        HttpStatusException.class, () -> handler.handleAuthenticated(ctx));
+        MatcherAssert.assertThat(ex.getStatusCode(), Matchers.equalTo(400));
+        Mockito.verify(fs).deleteIfExists(uploadPath);
+    }
+
     @Test
     void shouldProcessGoodRequest() throws Exception {
         RoutingContext ctx = Mockito.mock(RoutingContext.class);
 
         HttpServerResponse resp = Mockito.mock(HttpServerResponse.class);
         Mockito.when(ctx.response()).thenReturn(resp);
 
-        FileUpload upload1 = Mockito.mock(FileUpload.class);
-        Mockito.when(upload1.name()).thenReturn("template");
-        Mockito.when(upload1.uploadedFileName()).thenReturn("/file-uploads/abcd-1234");
-
-        FileUpload upload2 = Mockito.mock(FileUpload.class);
-        Mockito.when(upload2.name()).thenReturn("unused");
-        Mockito.when(upload2.uploadedFileName()).thenReturn("/file-uploads/wxyz-9999");
+        FileUpload upload = Mockito.mock(FileUpload.class);
+        Mockito.when(upload.name()).thenReturn("template");
+        Mockito.when(upload.uploadedFileName()).thenReturn("/file-uploads/abcd-1234");
 
-        Mockito.when(ctx.fileUploads()).thenReturn(Set.of(upload1, upload2));
+        Mockito.when(ctx.fileUploads()).thenReturn(Set.of(upload));
 
-        Path uploadPath1 = Mockito.mock(Path.class);
-        Path uploadPath2 = Mockito.mock(Path.class);
-        Mockito.when(fs.pathOf("/file-uploads/abcd-1234")).thenReturn(uploadPath1);
-        Mockito.when(fs.pathOf("/file-uploads/wxyz-9999")).thenReturn(uploadPath2);
+        Path uploadPath = Mockito.mock(Path.class);
+        Mockito.when(fs.pathOf("/file-uploads/abcd-1234")).thenReturn(uploadPath);
 
-        InputStream stream1 = Mockito.mock(InputStream.class);
-        InputStream stream2 = Mockito.mock(InputStream.class);
-        Mockito.when(fs.newInputStream(uploadPath1)).thenReturn(stream1);
-        Mockito.when(fs.newInputStream(uploadPath2)).thenReturn(stream2);
+        InputStream stream = Mockito.mock(InputStream.class);
+        Mockito.when(fs.newInputStream(uploadPath)).thenReturn(stream);
 
         handler.handleAuthenticated(ctx);
 
-        Mockito.verify(templateService).addTemplate(stream1);
+        Mockito.verify(templateService).addTemplate(stream);
         Mockito.verifyNoMoreInteractions(templateService);
-        Mockito.verify(fs).deleteIfExists(uploadPath1);
-        Mockito.verify(fs).deleteIfExists(uploadPath2);
+        Mockito.verify(fs).deleteIfExists(uploadPath);
         Mockito.verify(ctx).response();
         Mockito.verify(resp).end();
     }
@@ -193,25 +207,17 @@ void shouldSendNotifcationOnTemplateDeletion() throws Exception {
         HttpServerResponse resp = Mockito.mock(HttpServerResponse.class);
         Mockito.when(ctx.response()).thenReturn(resp);
 
-        FileUpload upload1 = Mockito.mock(FileUpload.class);
-        Mockito.when(upload1.name()).thenReturn("template");
-        Mockito.when(upload1.uploadedFileName()).thenReturn("/file-uploads/abcd-1234");
-
-        FileUpload upload2 = Mockito.mock(FileUpload.class);
-        Mockito.when(upload2.name()).thenReturn("unused");
-        Mockito.when(upload2.uploadedFileName()).thenReturn("/file-uploads/wxyz-9999");
+        FileUpload upload = Mockito.mock(FileUpload.class);
+        Mockito.when(upload.name()).thenReturn("template");
+        Mockito.when(upload.uploadedFileName()).thenReturn("/file-uploads/abcd-1234");
 
-        Mockito.when(ctx.fileUploads()).thenReturn(Set.of(upload1, upload2));
+        Mockito.when(ctx.fileUploads()).thenReturn(Set.of(upload));
 
-        Path uploadPath1 = Mockito.mock(Path.class);
-        Path uploadPath2 = Mockito.mock(Path.class);
-        Mockito.when(fs.pathOf("/file-uploads/abcd-1234")).thenReturn(uploadPath1);
-        Mockito.when(fs.pathOf("/file-uploads/wxyz-9999")).thenReturn(uploadPath2);
+        Path uploadPath = Mockito.mock(Path.class);
+        Mockito.when(fs.pathOf("/file-uploads/abcd-1234")).thenReturn(uploadPath);
 
-        InputStream stream1 = Mockito.mock(InputStream.class);
-        InputStream stream2 = Mockito.mock(InputStream.class);
-        Mockito.when(fs.newInputStream(uploadPath1)).thenReturn(stream1);
-        Mockito.when(fs.newInputStream(uploadPath2)).thenReturn(stream2);
+        InputStream stream = Mockito.mock(InputStream.class);
+        Mockito.when(fs.newInputStream(uploadPath)).thenReturn(stream);
 
         handler.handleAuthenticated(ctx);
 

src/test/java/itest/TemplatePostIT.java

@@ -0,0 +1,84 @@
+/*
+ * 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.ExecutionException;
+
+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 TemplatePostIT extends StandardSelfTest {
+    static final String FILE_NAME = "invalidTemplate.xml";
+    static final String TEMPLATE_NAME = "invalidTemplate";
+    static final String MEDIA_TYPE = "application/xml";
+    static final String REQ_URL = "/api/v1/templates";
+
+    @Test
+    public void shouldThrowIfTemplateUploadNameInvalid() throws Exception {
+
+        CompletableFuture<Integer> response = new CompletableFuture<>();
+        ClassLoader classLoader = getClass().getClassLoader();
+        File emptyTemplate = new File(classLoader.getResource(FILE_NAME).getFile());
+        String path = emptyTemplate.getAbsolutePath();
+
+        MultipartForm form =
+                MultipartForm.create()
+                        .attribute("invalidTemplateAttribute", FILE_NAME)
+                        .binaryFileUpload(TEMPLATE_NAME, FILE_NAME, path, MEDIA_TYPE);
+
+        webClient
+                .post(REQ_URL)
+                .sendMultipartForm(
+                        form,
+                        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"));
+    }
+}

src/test/resources/invalidTemplate.xml

@@ -0,0 +1 @@
+This is not valid xml.