bugfix: encoded query parameter (web) (#18)

pboos · web-flow · commit 2cccbb4f7609 · 2023-06-29T17:29:02.000+02:00
diff --git a/examples/example-spring-boot-starter-web/src/main/resources/application.properties b/examples/example-spring-boot-starter-web/src/main/resources/application.properties
@@ -1,4 +1,4 @@
-openapi.validation.sample-rate=0.5
+openapi.validation.sample-rate=1
 openapi.validation.specification-file-path=openapi.yaml
 openapi.validation.excluded-paths=/_readiness,/_liveness,/_metrics
 openapi.validation.validation-report-throttle-wait-seconds=10
diff --git a/examples/example-spring-boot-starter-webflux/src/main/resources/application.properties b/examples/example-spring-boot-starter-webflux/src/main/resources/application.properties
@@ -1,4 +1,4 @@
-openapi.validation.sample-rate=0.5
+openapi.validation.sample-rate=1
 openapi.validation.specification-file-path=openapi.yaml
 openapi.validation.excluded-paths=/_readiness,/_liveness,/_metrics
 openapi.validation.validation-report-throttle-wait-seconds=10
diff --git a/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/OpenApiRequestValidator.java b/openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/OpenApiRequestValidator.java
@@ -9,8 +9,8 @@
 import com.getyourguide.openapi.validation.api.model.RequestMetaData;
 import com.getyourguide.openapi.validation.api.model.ResponseMetaData;
 import com.getyourguide.openapi.validation.api.model.ValidationResult;
-import com.getyourguide.openapi.validation.api.model.ValidatorConfiguration;
 import com.getyourguide.openapi.validation.core.validator.OpenApiInteractionValidatorWrapper;
+import java.net.URLDecoder;
 import java.nio.charset.StandardCharsets;
 import java.util.concurrent.RejectedExecutionException;
 import java.util.concurrent.ThreadPoolExecutor;
@@ -32,11 +32,10 @@ public OpenApiRequestValidator(
         ThreadPoolExecutor threadPoolExecutor,
         ValidationReportHandler validationReportHandler,
         MetricsReporter metricsReporter,
-        String specificationFilePath,
-        ValidatorConfiguration configuration
+        OpenApiInteractionValidatorWrapper validator
     ) {
         this.threadPoolExecutor = threadPoolExecutor;
-        this.validator = new OpenApiInteractionValidatorFactory().build(specificationFilePath, configuration);
+        this.validator = validator;
         this.validationReportHandler = validationReportHandler;
         this.metricsReporter = metricsReporter;
 
@@ -79,14 +78,22 @@ public ValidationResult validateRequestObject(final RequestMetaData request, Str
     private static SimpleRequest buildSimpleRequest(RequestMetaData request, String requestBody) {
         var requestBuilder = new SimpleRequest.Builder(request.getMethod(), request.getUri().getPath());
         URLEncodedUtils.parse(request.getUri(), StandardCharsets.UTF_8)
-            .forEach(p -> requestBuilder.withQueryParam(p.getName(), p.getValue()));
+            .forEach(p -> requestBuilder.withQueryParam(p.getName(), nullSafeUrlDecode(p.getValue())));
         if (requestBody != null) {
             requestBuilder.withBody(requestBody);
         }
         request.getHeaders().forEach(requestBuilder::withHeader);
         return requestBuilder.build();
     }
 
+    private static String nullSafeUrlDecode(String value) {
+        if (value == null) {
+            return null;
+        }
+
+        return URLDecoder.decode(value, StandardCharsets.UTF_8);
+    }
+
     public ValidationResult validateResponseObject(
         final RequestMetaData request,
         ResponseMetaData response,
diff --git a/openapi-validation-core/src/test/java/com/getyourguide/openapi/validation/core/OpenApiRequestValidatorTest.java b/openapi-validation-core/src/test/java/com/getyourguide/openapi/validation/core/OpenApiRequestValidatorTest.java
@@ -1,40 +1,72 @@
 package com.getyourguide.openapi.validation.core;
 
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
 
+import com.atlassian.oai.validator.model.SimpleRequest;
 import com.getyourguide.openapi.validation.api.metrics.MetricsReporter;
-import com.getyourguide.openapi.validation.api.model.ValidatorConfiguration;
+import com.getyourguide.openapi.validation.api.model.RequestMetaData;
+import com.getyourguide.openapi.validation.core.validator.OpenApiInteractionValidatorWrapper;
+import java.net.URI;
+import java.util.HashMap;
 import java.util.concurrent.RejectedExecutionException;
 import java.util.concurrent.ThreadPoolExecutor;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.mockito.ArgumentCaptor;
 import org.mockito.Mockito;
 
 public class OpenApiRequestValidatorTest {
 
     private ThreadPoolExecutor threadPoolExecutor;
+    private OpenApiInteractionValidatorWrapper validator;
 
     private OpenApiRequestValidator openApiRequestValidator;
 
     @BeforeEach
     public void setup() {
         threadPoolExecutor = mock();
+        validator = mock();
         ValidationReportHandler validationReportHandler = mock();
         MetricsReporter metricsReporter = mock();
 
         openApiRequestValidator = new OpenApiRequestValidator(
             threadPoolExecutor,
             validationReportHandler,
             metricsReporter,
-            "",
-            new ValidatorConfiguration(null, null, null)
+            validator
         );
     }
 
     @Test
     public void testWhenThreadPoolExecutorRejectsExecutionThenItShouldNotThrow() {
-        Mockito.doThrow(new RejectedExecutionException()).when(threadPoolExecutor).execute(Mockito.any());
+        Mockito.doThrow(new RejectedExecutionException()).when(threadPoolExecutor).execute(any());
 
         openApiRequestValidator.validateRequestObjectAsync(mock(), null);
     }
+
+    @Test
+    public void testWhenEncodedQueryParamIsPassedThenValidationShouldHappenWithQueryParamDecoded() {
+        var uri = URI.create("https://api.example.com?ids=1%2C2%2C3&text=e%3Dmc2%20%26%20more&spaces=this+is+a+sparta");
+        var request = new RequestMetaData("GET", uri, new HashMap<>());
+
+        openApiRequestValidator.validateRequestObject(request, null);
+
+        var simpleRequestArgumentCaptor = ArgumentCaptor.forClass(SimpleRequest.class);
+        verify(validator).validateRequest(simpleRequestArgumentCaptor.capture());
+        verifyQueryParamValueEquals(simpleRequestArgumentCaptor, "ids", "1,2,3");
+        verifyQueryParamValueEquals(simpleRequestArgumentCaptor, "text", "e=mc2 & more");
+        verifyQueryParamValueEquals(simpleRequestArgumentCaptor, "spaces", "this is a sparta");
+    }
+
+    private void verifyQueryParamValueEquals(
+        ArgumentCaptor<SimpleRequest> simpleRequestArgumentCaptor,
+        String name,
+        String expected
+    ) {
+        var ids = simpleRequestArgumentCaptor.getValue().getQueryParameterValues(name).iterator().next();
+        assertEquals(expected, ids);
+    }
 }
diff --git a/spring-boot-starter/spring-boot-starter-core/src/main/java/com/getyourguide/openapi/validation/autoconfigure/LibraryAutoConfiguration.java b/spring-boot-starter/spring-boot-starter-core/src/main/java/com/getyourguide/openapi/validation/autoconfigure/LibraryAutoConfiguration.java
@@ -14,6 +14,7 @@
 import com.getyourguide.openapi.validation.api.model.ValidatorConfiguration;
 import com.getyourguide.openapi.validation.api.model.ValidatorConfigurationBuilder;
 import com.getyourguide.openapi.validation.core.DefaultViolationLogger;
+import com.getyourguide.openapi.validation.core.OpenApiInteractionValidatorFactory;
 import com.getyourguide.openapi.validation.core.OpenApiRequestValidator;
 import com.getyourguide.openapi.validation.core.ValidationReportHandler;
 import com.getyourguide.openapi.validation.core.throttle.RequestBasedValidationReportThrottler;
@@ -112,8 +113,8 @@ public OpenApiRequestValidator openApiRequestValidator(
             threadPoolExecutor,
             validationReportHandler,
             metricsReporter,
-            properties.getSpecificationFilePath(),
-            validatorConfiguration
+            new OpenApiInteractionValidatorFactory()
+                .build(properties.getSpecificationFilePath(), validatorConfiguration)
         );
     }
 }
diff --git a/spring-boot-starter/spring-boot-starter-web-spring2.7/src/main/java/com/getyourguide/openapi/validation/factory/ServletMetaDataFactory.java b/spring-boot-starter/spring-boot-starter-web-spring2.7/src/main/java/com/getyourguide/openapi/validation/factory/ServletMetaDataFactory.java
@@ -2,7 +2,6 @@
 
 import com.getyourguide.openapi.validation.api.model.RequestMetaData;
 import com.getyourguide.openapi.validation.api.model.ResponseMetaData;
-import java.util.Map;
 import java.util.TreeMap;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
@@ -13,7 +12,7 @@ public class ServletMetaDataFactory {
     public static final String HEADER_CONTENT_TYPE = "Content-Type";
 
     public RequestMetaData buildRequestMetaData(HttpServletRequest request) {
-        var uri = ServletUriComponentsBuilder.fromRequest(request).build(Map.of());
+        var uri = ServletUriComponentsBuilder.fromRequest(request).build().toUri();
         return new RequestMetaData(request.getMethod(), uri, getHeaders(request));
     }
 
diff --git a/spring-boot-starter/spring-boot-starter-web/build.gradle b/spring-boot-starter/spring-boot-starter-web/build.gradle
@@ -22,5 +22,6 @@ dependencies {
 
     testImplementation 'org.springframework.boot:spring-boot-starter-test'
     testImplementation 'org.springframework:spring-web'
+    testImplementation 'org.springframework:spring-webmvc'
     testImplementation 'org.apache.tomcat.embed:tomcat-embed-core' // For jakarta.servlet.ServletContext
 }
diff --git a/spring-boot-starter/spring-boot-starter-web/src/main/java/com/getyourguide/openapi/validation/factory/ServletMetaDataFactory.java b/spring-boot-starter/spring-boot-starter-web/src/main/java/com/getyourguide/openapi/validation/factory/ServletMetaDataFactory.java
@@ -4,7 +4,6 @@
 import com.getyourguide.openapi.validation.api.model.ResponseMetaData;
 import jakarta.servlet.http.HttpServletRequest;
 import jakarta.servlet.http.HttpServletResponse;
-import java.util.Map;
 import java.util.TreeMap;
 import org.springframework.web.servlet.support.ServletUriComponentsBuilder;
 
@@ -13,7 +12,7 @@ public class ServletMetaDataFactory {
     public static final String HEADER_CONTENT_TYPE = "Content-Type";
 
     public RequestMetaData buildRequestMetaData(HttpServletRequest request) {
-        var uri = ServletUriComponentsBuilder.fromRequest(request).build(Map.of());
+        var uri = ServletUriComponentsBuilder.fromRequest(request).build().toUri();
         return new RequestMetaData(request.getMethod(), uri, getHeaders(request));
     }
 

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-openapi.validation.sample-rate=0.5`
	`1`	`+openapi.validation.sample-rate=1`
`2`	`2`	`openapi.validation.specification-file-path=openapi.yaml`
`3`	`3`	`openapi.validation.excluded-paths=/_readiness,/_liveness,/_metrics`
`4`	`4`	`openapi.validation.validation-report-throttle-wait-seconds=10`
Original file line number	Diff line number	Diff line change
`@@ -22,5 +22,6 @@ dependencies {`
`22`	`22`
`23`	`23`	`testImplementation 'org.springframework.boot:spring-boot-starter-test'`
`24`	`24`	`testImplementation 'org.springframework:spring-web'`
	`25`	`+ testImplementation 'org.springframework:spring-webmvc'`
`25`	`26`	`testImplementation 'org.apache.tomcat.embed:tomcat-embed-core' // For jakarta.servlet.ServletContext`
`26`	`27`	`}`