bugfix: allow rereading request body after it was validated before ha…

…ndler
getyourguide · Nov 23, 2023 · 2b8fa59 · 2b8fa59
1 parent 63f1c20
commit 2b8fa59
Show file tree

Hide file tree

Showing 6 changed files with 27 additions and 44 deletions.
diff --git a/...c/main/java/com/getyourguide/openapi/validation/factory/ContentCachingWrapperFactory.java b/...c/main/java/com/getyourguide/openapi/validation/factory/ContentCachingWrapperFactory.java
@@ -1,19 +1,19 @@
 package com.getyourguide.openapi.validation.factory;
 
-import com.getyourguide.openapi.validation.filter.MultiReadHttpServletRequestWrapper;
+import com.getyourguide.openapi.validation.filter.MultiReadContentCachingRequestWrapper;
 import jakarta.servlet.http.HttpServletRequest;
 import jakarta.servlet.http.HttpServletResponse;
 import javax.annotation.Nullable;
 import org.springframework.web.util.ContentCachingResponseWrapper;
 import org.springframework.web.util.WebUtils;
 
 public class ContentCachingWrapperFactory {
-    public MultiReadHttpServletRequestWrapper buildContentCachingRequestWrapper(HttpServletRequest request) {
-        if (request instanceof MultiReadHttpServletRequestWrapper) {
-            return (MultiReadHttpServletRequestWrapper) request;
+    public MultiReadContentCachingRequestWrapper buildContentCachingRequestWrapper(HttpServletRequest request) {
+        if (request instanceof MultiReadContentCachingRequestWrapper) {
+            return (MultiReadContentCachingRequestWrapper) request;
         }
 
-        return new MultiReadHttpServletRequestWrapper(request);
+        return new MultiReadContentCachingRequestWrapper(request);
     }
 
     public ContentCachingResponseWrapper buildContentCachingResponseWrapper(HttpServletResponse response) {
@@ -26,8 +26,8 @@ public ContentCachingResponseWrapper buildContentCachingResponseWrapper(HttpServ
     }
 
     @Nullable
-    public MultiReadHttpServletRequestWrapper getCachingRequest(HttpServletRequest request) {
-        return request instanceof MultiReadHttpServletRequestWrapper ? (MultiReadHttpServletRequestWrapper) request : null;
+    public MultiReadContentCachingRequestWrapper getCachingRequest(HttpServletRequest request) {
+        return request instanceof MultiReadContentCachingRequestWrapper ? (MultiReadContentCachingRequestWrapper) request : null;
     }
 
     @Nullable

diff --git a/...r/MultiReadHttpServletRequestWrapper.java → ...ultiReadContentCachingRequestWrapper.java b/...r/MultiReadHttpServletRequestWrapper.java → ...ultiReadContentCachingRequestWrapper.java
@@ -3,54 +3,37 @@
 import jakarta.servlet.ReadListener;
 import jakarta.servlet.ServletInputStream;
 import jakarta.servlet.http.HttpServletRequest;
-import jakarta.servlet.http.HttpServletRequestWrapper;
 import java.io.BufferedReader;
 import java.io.ByteArrayInputStream;
-import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.InputStreamReader;
-import org.apache.tomcat.util.http.fileupload.IOUtils;
+import org.springframework.web.util.ContentCachingRequestWrapper;
 
-/**
- * A class to provide the ability to read a {@link jakarta.servlet.ServletRequest}'s body multiple
- * times. via https://stackoverflow.com/a/36619972/2257038 and https://stackoverflow.com/a/30748533/2257038
- */
-public class MultiReadHttpServletRequestWrapper extends HttpServletRequestWrapper {
+public class MultiReadContentCachingRequestWrapper extends ContentCachingRequestWrapper {
 
-    private ByteArrayOutputStream cachedBytes;
-
-    /**
-     * Construct a new multi-read wrapper.
-     *
-     * @param request to wrap around
-     */
-    public MultiReadHttpServletRequestWrapper(HttpServletRequest request) {
+    public MultiReadContentCachingRequestWrapper(HttpServletRequest request) {
         super(request);
     }
 
+    public MultiReadContentCachingRequestWrapper(HttpServletRequest request, int contentCacheLimit) {
+        super(request, contentCacheLimit);
+    }
+
     @Override
     public ServletInputStream getInputStream() throws IOException {
-        if (cachedBytes == null) {
-            cacheInputStream();
+        var inputStream = super.getInputStream();
+        if (inputStream.isFinished()) {
+            return new CachedServletInputStream(getContentAsByteArray());
         }
 
-        return new CachedServletInputStream(cachedBytes.toByteArray());
+        return inputStream;
     }
 
     @Override
     public BufferedReader getReader() throws IOException {
         return new BufferedReader(new InputStreamReader(getInputStream()));
     }
 
-    private void cacheInputStream() throws IOException {
-        /* Cache the inputstream in order to read it multiple times. For
-         * convenience, I use apache.commons IOUtils
-         */
-        cachedBytes = new ByteArrayOutputStream();
-        IOUtils.copy(super.getInputStream(), cachedBytes);
-    }
-
-    /* An inputstream which reads the cached request body */
     private static class CachedServletInputStream extends ServletInputStream {
         private final ByteArrayInputStream buffer;
 

diff --git a/...rc/main/java/com/getyourguide/openapi/validation/filter/OpenApiValidationInterceptor.java b/...rc/main/java/com/getyourguide/openapi/validation/filter/OpenApiValidationInterceptor.java
@@ -127,7 +127,7 @@ private static RequestMetaData getRequestMetaData(HttpServletRequest request) {
     }
 
     private ValidationResult validateRequest(
-        MultiReadHttpServletRequestWrapper request,
+        MultiReadContentCachingRequestWrapper request,
         RequestMetaData requestMetaData,
         @Nullable ResponseMetaData responseMetaData,
         RunType runType
@@ -148,7 +148,7 @@ private ValidationResult validateRequest(
         }
     }
 
-    private static String readBodyCatchingException(MultiReadHttpServletRequestWrapper request) {
+    private static String readBodyCatchingException(MultiReadContentCachingRequestWrapper request) {
         try {
             return StreamUtils.copyToString(request.getInputStream(), StandardCharsets.UTF_8);
         } catch (IOException e) {

diff --git a/...-starter-web/src/test/java/com/getyourguide/openapi/validation/filter/BaseFilterTest.java b/...-starter-web/src/test/java/com/getyourguide/openapi/validation/filter/BaseFilterTest.java
@@ -50,7 +50,7 @@ private static void mockRequestAttributes(ServletRequest request, HashMap<String
     }
 
     protected MockSetupData mockSetup(MockConfiguration configuration) {
-        var request = mock(MultiReadHttpServletRequestWrapper.class);
+        var request = mock(MultiReadContentCachingRequestWrapper.class);
         var response = mock(ContentCachingResponseWrapper.class);
         var cachingRequest = mockContentCachingRequest(request, configuration);
         var cachingResponse = mockContentCachingResponse(response, configuration);
@@ -101,11 +101,11 @@ private ContentCachingResponseWrapper mockContentCachingResponse(
         return cachingResponse;
     }
 
-    private MultiReadHttpServletRequestWrapper mockContentCachingRequest(
+    private MultiReadContentCachingRequestWrapper mockContentCachingRequest(
         HttpServletRequest request,
         MockConfiguration configuration
     ) {
-        var cachingRequest = mock(MultiReadHttpServletRequestWrapper.class);
+        var cachingRequest = mock(MultiReadContentCachingRequestWrapper.class);
         when(contentCachingWrapperFactory.buildContentCachingRequestWrapper(request)).thenReturn(cachingRequest);
         if (configuration.requestBody != null) {
             try {

diff --git a/.../java/com/getyourguide/openapi/validation/integration/FailOnViolationIntegrationTest.java b/.../java/com/getyourguide/openapi/validation/integration/FailOnViolationIntegrationTest.java
@@ -42,10 +42,10 @@ public void setup() {
     @Test
     public void whenValidRequestThenReturnSuccessfully() throws Exception {
         mockMvc.perform(post("/test")
-                .content("{ \"value\": \"test\", \"responseStatusCode\": 200 }").contentType(MediaType.APPLICATION_JSON))
+                .content("{ \"value\": \"testing\", \"responseStatusCode\": 200 }").contentType(MediaType.APPLICATION_JSON))
             .andExpectAll(
                 status().isOk(),
-                jsonPath("$.value").value("test")
+                jsonPath("$.value").value("testing")
             );
         Thread.sleep(100);
 

diff --git a/.../java/com/getyourguide/openapi/validation/integration/FailOnViolationIntegrationTest.java b/.../java/com/getyourguide/openapi/validation/integration/FailOnViolationIntegrationTest.java
@@ -39,10 +39,10 @@ public void whenValidRequestThenReturnSuccessfully() throws Exception {
             .post().uri("/test")
             .accept(MediaType.APPLICATION_JSON)
             .contentType(MediaType.APPLICATION_JSON)
-            .bodyValue("{ \"value\": \"test\", \"responseStatusCode\": 200 }")
+            .bodyValue("{ \"value\": \"testing\", \"responseStatusCode\": 200 }")
             .exchange()
             .expectStatus().isOk()
-            .expectBody().jsonPath("$.value").isEqualTo("test");
+            .expectBody().jsonPath("$.value").isEqualTo("testing");
         Thread.sleep(100);
 
         assertEquals(0, openApiViolationLogger.getViolations().size());