Exclude false positive violations

openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/OpenApiRequestValidator.java

@@ -14,6 +14,7 @@
 import java.nio.charset.StandardCharsets;
 import java.util.concurrent.RejectedExecutionException;
 import java.util.concurrent.ThreadPoolExecutor;
+import javax.annotation.Nullable;
 import lombok.extern.slf4j.Slf4j;
 import org.apache.http.client.utils.URLEncodedUtils;
 
@@ -46,8 +47,8 @@ public boolean isReady() {
         return validator != null;
     }
 
-    public void validateRequestObjectAsync(final RequestMetaData request, String requestBody) {
-        executeAsync(() -> validateRequestObject(request, requestBody));
+    public void validateRequestObjectAsync(final RequestMetaData request, @Nullable ResponseMetaData response, String requestBody) {
+        executeAsync(() -> validateRequestObject(request, response, requestBody));
     }
 
     public void validateResponseObjectAsync(final RequestMetaData request, ResponseMetaData response, final String responseBody) {
@@ -63,10 +64,18 @@ private void executeAsync(Runnable command) {
     }
 
     public ValidationResult validateRequestObject(final RequestMetaData request, String requestBody) {
+        return validateRequestObject(request, null, requestBody);
+    }
+
+    public ValidationResult validateRequestObject(
+        final RequestMetaData request,
+        @Nullable final ResponseMetaData response,
+        String requestBody
+    ) {
         try {
             var simpleRequest = buildSimpleRequest(request, requestBody);
             var result = validator.validateRequest(simpleRequest);
-            validationReportHandler.handleValidationReport(request, Direction.REQUEST, requestBody, result);
+            validationReportHandler.handleValidationReport(request, response, Direction.REQUEST, requestBody, result);
             reportValidationHeartbeat();
             return buildValidationResult(result);
         } catch (Exception e) {
@@ -96,7 +105,7 @@ private static String nullSafeUrlDecode(String value) {
 
     public ValidationResult validateResponseObject(
         final RequestMetaData request,
-        ResponseMetaData response,
+        final ResponseMetaData response,
         final String responseBody
     ) {
         try {
@@ -112,7 +121,7 @@ public ValidationResult validateResponseObject(
                 Request.Method.valueOf(request.getMethod().toUpperCase()),
                 responseBuilder.build()
             );
-            validationReportHandler.handleValidationReport(request, Direction.RESPONSE, responseBody, result);
+            validationReportHandler.handleValidationReport(request, response, Direction.RESPONSE, responseBody, result);
             reportValidationHeartbeat();
             return buildValidationResult(result);
         } catch (Exception e) {

openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/ValidationReportHandler.java

@@ -1,27 +1,30 @@
 package com.getyourguide.openapi.validation.core;
 
 import com.atlassian.oai.validator.report.ValidationReport;
-import com.getyourguide.openapi.validation.api.exclusions.ViolationExclusions;
 import com.getyourguide.openapi.validation.api.log.LogLevel;
 import com.getyourguide.openapi.validation.api.log.ViolationLogger;
 import com.getyourguide.openapi.validation.api.metrics.MetricsReporter;
 import com.getyourguide.openapi.validation.api.model.Direction;
 import com.getyourguide.openapi.validation.api.model.OpenApiViolation;
 import com.getyourguide.openapi.validation.api.model.RequestMetaData;
+import com.getyourguide.openapi.validation.api.model.ResponseMetaData;
+import com.getyourguide.openapi.validation.core.exclusions.InternalViolationExclusions;
 import com.getyourguide.openapi.validation.core.throttle.ValidationReportThrottler;
 import io.swagger.v3.oas.models.parameters.Parameter;
 import java.util.Optional;
+import javax.annotation.Nullable;
 import lombok.AllArgsConstructor;
 
 @AllArgsConstructor
 public class ValidationReportHandler {
     private final ValidationReportThrottler throttleHelper;
     private final ViolationLogger logger;
     private final MetricsReporter metrics;
-    private final ViolationExclusions violationExclusions;
+    private final InternalViolationExclusions violationExclusions;
 
     public void handleValidationReport(
         RequestMetaData request,
+        @Nullable ResponseMetaData response,
         Direction direction,
         String body,
         ValidationReport result
@@ -30,8 +33,8 @@ public void handleValidationReport(
             result
                 .getMessages()
                 .stream()
-                .map(message -> buildOpenApiViolation(message, request, body, direction))
-                .filter(violation -> !isViolationExcluded(violation))
+                .map(message -> buildOpenApiViolation(message, request, response, body, direction))
+                .filter(violation -> !violationExclusions.isExcluded(violation))
                 .forEach(violation -> throttleHelper.throttle(violation, () -> logValidationError(violation)));
         }
     }
@@ -44,6 +47,7 @@ private void logValidationError(OpenApiViolation openApiViolation) {
     private OpenApiViolation buildOpenApiViolation(
         ValidationReport.Message message,
         RequestMetaData request,
+        @Nullable ResponseMetaData response,
         String body,
         Direction direction
     ) {
@@ -75,20 +79,12 @@ private OpenApiViolation buildOpenApiViolation(
             .instance(pointersInstance)
             .parameter(parameterName)
             .schema(getPointersSchema(message))
-            .responseStatus(getResponseStatus(message))
+            .responseStatus(getResponseStatus(response, message))
             .logMessage(logMessage)
             .message(message.getMessage())
             .build();
     }
 
-    private boolean isViolationExcluded(OpenApiViolation openApiViolation) {
-        return
-            violationExclusions.isExcluded(openApiViolation)
-                // If it matches more than 1, then we don't want to log a validation error
-                || openApiViolation.getMessage().matches(
-                ".*\\[Path '[^']+'] Instance failed to match exactly one schema \\(matched [1-9][0-9]* out of \\d\\).*");
-    }
-
     private static Optional<String> getPointersInstance(ValidationReport.Message message) {
         return message.getContext()
             .flatMap(ValidationReport.MessageContext::getPointers)
@@ -119,7 +115,14 @@ private static Optional<String> getNormalizedPath(ValidationReport.Message messa
             .map(apiOperation -> apiOperation.getApiPath().normalised());
     }
 
-    private static Optional<Integer> getResponseStatus(ValidationReport.Message message) {
+    private static Optional<Integer> getResponseStatus(
+        @Nullable ResponseMetaData response,
+        ValidationReport.Message message
+    ) {
+        if (response != null && response.getStatusCode() != null) {
+            return Optional.of(response.getStatusCode());
+        }
+
         return message.getContext().flatMap(ValidationReport.MessageContext::getResponseStatus);
     }
 

openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/exclusions/InternalViolationExclusions.java

@@ -0,0 +1,32 @@
+package com.getyourguide.openapi.validation.core.exclusions;
+
+import com.getyourguide.openapi.validation.api.exclusions.ViolationExclusions;
+import com.getyourguide.openapi.validation.api.model.Direction;
+import com.getyourguide.openapi.validation.api.model.OpenApiViolation;
+import lombok.AllArgsConstructor;
+
+@AllArgsConstructor
+public class InternalViolationExclusions {
+    private final ViolationExclusions customViolationExclusions;
+
+    public boolean isExcluded(OpenApiViolation violation) {
+        return falsePositive404(violation)
+            || falsePositive400(violation)
+            || customViolationExclusions.isExcluded(violation)
+            // If it matches more than 1, then we don't want to log a validation error
+            || violation.getMessage().matches(
+            ".*\\[Path '[^']+'] Instance failed to match exactly one schema \\(matched [1-9][0-9]* out of \\d\\).*");
+    }
+
+    private boolean falsePositive404(OpenApiViolation violation) {
+        return "validation.request.path.missing".equals(violation.getRule())
+            && (
+                violation.getDirection() == Direction.REQUEST
+                || (violation.getDirection() == Direction.RESPONSE && violation.getResponseStatus().orElse(0) == 404)
+            );
+    }
+
+    private boolean falsePositive400(OpenApiViolation violation) {
+        return violation.getDirection() == Direction.REQUEST && violation.getResponseStatus().orElse(0) == 400;
+    }
+}

openapi-validation-core/src/test/java/com/getyourguide/openapi/validation/core/OpenApiRequestValidatorTest.java

@@ -44,7 +44,7 @@ public void setup() {
     public void testWhenThreadPoolExecutorRejectsExecutionThenItShouldNotThrow() {
         Mockito.doThrow(new RejectedExecutionException()).when(threadPoolExecutor).execute(any());
 
-        openApiRequestValidator.validateRequestObjectAsync(mock(), null);
+        openApiRequestValidator.validateRequestObjectAsync(mock(), null, null);
     }
 
     @Test

openapi-validation-core/src/test/java/com/getyourguide/openapi/validation/core/ValidationReportHandlerTest.java

@@ -8,12 +8,12 @@
 import static org.mockito.Mockito.when;
 
 import com.atlassian.oai.validator.report.ValidationReport;
-import com.getyourguide.openapi.validation.api.exclusions.ViolationExclusions;
 import com.getyourguide.openapi.validation.api.log.ViolationLogger;
 import com.getyourguide.openapi.validation.api.metrics.MetricsReporter;
 import com.getyourguide.openapi.validation.api.model.Direction;
 import com.getyourguide.openapi.validation.api.model.OpenApiViolation;
 import com.getyourguide.openapi.validation.api.model.RequestMetaData;
+import com.getyourguide.openapi.validation.core.exclusions.InternalViolationExclusions;
 import com.getyourguide.openapi.validation.core.throttle.ValidationReportThrottler;
 import io.swagger.v3.oas.models.parameters.Parameter;
 import java.net.URI;
@@ -35,7 +35,7 @@ public void setUp() {
         throttleHelper = mock();
         logger = mock();
         MetricsReporter metrics = mock();
-        ViolationExclusions violationExclusions = mock();
+        InternalViolationExclusions violationExclusions = mock();
 
         validationReportHandler = new ValidationReportHandler(throttleHelper, logger, metrics, violationExclusions);
     }
@@ -46,7 +46,7 @@ public void testWhenParameterNameIsPresentThenItShouldAddItToTheMessage() {
         var request = mockRequestMetaData();
         var validationReport = mockValidationReport("parameterName");
 
-        validationReportHandler.handleValidationReport(request, Direction.REQUEST, null, validationReport);
+        validationReportHandler.handleValidationReport(request, null, Direction.REQUEST, null, validationReport);
 
         var argumentCaptor = ArgumentCaptor.forClass(OpenApiViolation.class);
         verify(logger).log(argumentCaptor.capture());

openapi-validation-core/src/test/java/com/getyourguide/openapi/validation/core/exclusions/InternalViolationExclusionsTest.java

@@ -0,0 +1,107 @@
+package com.getyourguide.openapi.validation.core.exclusions;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import com.getyourguide.openapi.validation.api.exclusions.ViolationExclusions;
+import com.getyourguide.openapi.validation.api.model.Direction;
+import com.getyourguide.openapi.validation.api.model.OpenApiViolation;
+import java.util.Optional;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+public class InternalViolationExclusionsTest {
+    private ViolationExclusions customViolationExclusions;
+    private InternalViolationExclusions violationExclusions;
+
+    @BeforeEach
+    public void setup() {
+        customViolationExclusions = mock();
+        violationExclusions = new InternalViolationExclusions(customViolationExclusions);
+    }
+
+    @Test
+    public void testWhenViolationThenViolationNotExcluded() {
+        when(customViolationExclusions.isExcluded(any())).thenReturn(false);
+
+        checkViolationNotExcluded(buildSimpleViolation(Direction.RESPONSE, 404));
+        checkViolationNotExcluded(buildSimpleViolation(Direction.RESPONSE, 400));
+        checkViolationNotExcluded(buildSimpleViolation(Direction.REQUEST, 200));
+        checkViolationNotExcluded(buildSimpleViolation(Direction.REQUEST, null));
+        checkViolationNotExcluded(buildSimpleViolation(Direction.RESPONSE, 200));
+    }
+
+    private static OpenApiViolation buildSimpleViolation(Direction direction, Integer responseStatus) {
+        return OpenApiViolation.builder()
+            .direction(direction)
+            .rule("validation." + (direction == Direction.REQUEST ? "request" : "response") + ".something")
+            .responseStatus(responseStatus != null ? Optional.of(responseStatus) : Optional.empty())
+            .message("Some violation message")
+            .build();
+    }
+
+    @Test
+    public void testWhenCustomViolationExclusionThenViolationExcluded() {
+        when(customViolationExclusions.isExcluded(any())).thenReturn(true);
+
+        checkViolationExcluded(OpenApiViolation.builder().build());
+    }
+
+    @Test
+    public void testWhenInstanceFailedToMatchExactlyOneThenViolationExcluded() {
+        when(customViolationExclusions.isExcluded(any())).thenReturn(false);
+
+        checkViolationExcluded(OpenApiViolation.builder()
+            .message("[Path '/v1/endpoint'] Instance failed to match exactly one schema (matched 2 out of 4)").build());
+    }
+
+    @Test
+    public void testWhen404ResponseWithApiPathNotSpecifiedThenViolationExcluded() {
+        when(customViolationExclusions.isExcluded(any())).thenReturn(false);
+
+        checkViolationExcluded(OpenApiViolation.builder()
+            .direction(Direction.RESPONSE)
+            .rule("validation.request.path.missing")
+            .responseStatus(Optional.of(404))
+            .message("No API path found that matches request '/nothing'")
+            .build());
+    }
+
+    @Test
+    public void testWhenRequestWithApiPathNotSpecifiedThenViolationExcluded() {
+        when(customViolationExclusions.isExcluded(any())).thenReturn(false);
+
+        checkViolationExcluded(OpenApiViolation.builder()
+            .direction(Direction.REQUEST)
+            .rule("validation.request.path.missing")
+            .responseStatus(Optional.empty())
+            .message("No API path found that matches request '/nothing'")
+            .build());
+    }
+
+    @Test
+    public void testWhenRequestViolationsAnd400ThenViolationExcluded() {
+        when(customViolationExclusions.isExcluded(any())).thenReturn(false);
+
+        checkViolationExcluded(OpenApiViolation.builder()
+            .direction(Direction.REQUEST)
+            .responseStatus(Optional.of(400))
+            .message("")
+            .build());
+    }
+
+    private void checkViolationNotExcluded(OpenApiViolation violation) {
+        var isExcluded = violationExclusions.isExcluded(violation);
+
+        assertFalse(isExcluded);
+    }
+
+    private void checkViolationExcluded(OpenApiViolation violation) {
+        var isExcluded = violationExclusions.isExcluded(violation);
+
+        assertTrue(isExcluded);
+    }
+}

spring-boot-starter/spring-boot-starter-core/src/main/java/com/getyourguide/openapi/validation/autoconfigure/LibraryAutoConfiguration.java

@@ -17,6 +17,7 @@
 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.exclusions.InternalViolationExclusions;
 import com.getyourguide.openapi.validation.core.throttle.RequestBasedValidationReportThrottler;
 import com.getyourguide.openapi.validation.core.throttle.ValidationReportThrottler;
 import com.getyourguide.openapi.validation.core.throttle.ValidationReportThrottlerNone;
@@ -81,7 +82,7 @@ public ValidationReportHandler validationReportHandler(
             validationReportThrottler,
             logger,
             metricsReporter,
-            violationExclusions.orElseGet(NoViolationExclusions::new)
+            new InternalViolationExclusions(violationExclusions.orElseGet(NoViolationExclusions::new))
         );
     }