diff --git a/src/main/java/com/retailsvc/http/internal/RequestPreparationFilter.java b/src/main/java/com/retailsvc/http/internal/RequestPreparationFilter.java index 93ae30a..55fa164 100644 --- a/src/main/java/com/retailsvc/http/internal/RequestPreparationFilter.java +++ b/src/main/java/com/retailsvc/http/internal/RequestPreparationFilter.java @@ -11,6 +11,10 @@ import com.retailsvc.http.spec.Parameter; import com.retailsvc.http.spec.RequestBody; import com.retailsvc.http.spec.Spec; +import com.retailsvc.http.spec.schema.BooleanSchema; +import com.retailsvc.http.spec.schema.IntegerSchema; +import com.retailsvc.http.spec.schema.NumberSchema; +import com.retailsvc.http.spec.schema.Schema; import com.retailsvc.http.validate.ValidationError; import com.retailsvc.http.validate.Validator; import com.sun.net.httpserver.Filter; @@ -124,10 +128,48 @@ private void validateParameters( } continue; } - validator.validate(value, p.schema(), pointer); + validator.validate(coerceParameterValue(value, p.schema(), pointer), p.schema(), pointer); } } + /** + * Converts a raw parameter string into a typed value matching the schema's primitive kind, so the + * validator (which is faithful to JSON Schema {@code type} semantics) sees the value the spec + * describes rather than its string serialization. Strings that fail to parse are passed through + * unchanged so the validator surfaces a {@code type} error with the original input. + */ + private static Object coerceParameterValue(String raw, Schema schema, String pointer) { + return switch (schema) { + case IntegerSchema _ -> { + try { + yield Long.parseLong(raw); + } catch (NumberFormatException _) { + throw new ValidationException( + new ValidationError(pointer, "type", "expected integer", raw)); + } + } + case NumberSchema _ -> { + try { + yield Double.parseDouble(raw); + } catch (NumberFormatException _) { + throw new ValidationException( + new ValidationError(pointer, "type", "expected number", raw)); + } + } + case BooleanSchema _ -> { + if ("true".equals(raw)) { + yield Boolean.TRUE; + } + if ("false".equals(raw)) { + yield Boolean.FALSE; + } + throw new ValidationException( + new ValidationError(pointer, "type", "expected boolean", raw)); + } + default -> raw; + }; + } + private Object validateAndParseBody(HttpExchange exchange, Operation op, byte[] body) { Optional rb = op.requestBody(); if (rb.isEmpty()) { diff --git a/src/main/java/com/retailsvc/http/validate/DefaultValidator.java b/src/main/java/com/retailsvc/http/validate/DefaultValidator.java index a6e6ae2..e1e31fd 100644 --- a/src/main/java/com/retailsvc/http/validate/DefaultValidator.java +++ b/src/main/java/com/retailsvc/http/validate/DefaultValidator.java @@ -347,22 +347,11 @@ private static boolean isHextet(String hextet) { } private void validateInteger(Object value, IntegerSchema s, String pointer) { - long n; - switch (value) { - case Number num -> n = num.longValue(); - case String str -> { - try { - n = Long.parseLong(str); - } catch (NumberFormatException _) { - fail(pointer, "type", "expected integer", value); - return; - } - } - case null, default -> { - fail(pointer, "type", "expected integer", value); - return; - } + if (!(value instanceof Number num)) { + fail(pointer, "type", "expected integer", value); + return; } + long n = num.longValue(); if (s.minimum() != null && n < s.minimum()) { fail(pointer, "minimum", "integer below minimum " + s.minimum(), n); @@ -385,22 +374,11 @@ private void validateInteger(Object value, IntegerSchema s, String pointer) { } private void validateNumber(Object value, NumberSchema s, String pointer) { - double n; - switch (value) { - case Number num -> n = num.doubleValue(); - case String str -> { - try { - n = Double.parseDouble(str); - } catch (NumberFormatException _) { - fail(pointer, "type", "expected number", value); - return; - } - } - case null, default -> { - fail(pointer, "type", "expected number", value); - return; - } + if (!(value instanceof Number num)) { + fail(pointer, "type", "expected number", value); + return; } + double n = num.doubleValue(); if (s.minimum() != null && n < s.minimum().doubleValue()) { fail(pointer, "minimum", "number below minimum " + s.minimum(), n); diff --git a/src/test/java/com/retailsvc/http/HandlersDefaultExceptionTest.java b/src/test/java/com/retailsvc/http/HandlersDefaultExceptionTest.java index ff726ce..c9e29d7 100644 --- a/src/test/java/com/retailsvc/http/HandlersDefaultExceptionTest.java +++ b/src/test/java/com/retailsvc/http/HandlersDefaultExceptionTest.java @@ -1,6 +1,7 @@ package com.retailsvc.http; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; import com.retailsvc.http.spec.HttpMethod; import com.retailsvc.http.validate.ValidationError; @@ -13,7 +14,7 @@ class HandlersDefaultExceptionTest { private HttpExchange newExchange(ByteArrayOutputStream sink) { - HttpExchange ex = Mockito.mock(HttpExchange.class); + HttpExchange ex = mock(HttpExchange.class); Mockito.when(ex.getResponseHeaders()).thenReturn(new Headers()); Mockito.when(ex.getResponseBody()).thenReturn(sink); return ex; diff --git a/src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java b/src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java index d3654a7..b34740d 100644 --- a/src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java +++ b/src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java @@ -1,6 +1,7 @@ package com.retailsvc.http.internal; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.Mockito.mock; import com.retailsvc.http.MissingOperationHandlerException; import com.retailsvc.http.Request; @@ -20,8 +21,8 @@ private static void withOperationId( @Test void invokesRegisteredHandler() throws Exception { - HttpHandler handler = Mockito.mock(HttpHandler.class); - HttpExchange ex = Mockito.mock(HttpExchange.class); + HttpHandler handler = mock(HttpHandler.class); + HttpExchange ex = mock(HttpExchange.class); withOperationId( "get-x", @@ -37,7 +38,7 @@ void invokesRegisteredHandler() throws Exception { @Test void throwsWhenHandlerMissing() { DispatchHandler d = new DispatchHandler(Map.of()); - HttpExchange ex = Mockito.mock(HttpExchange.class); + HttpExchange ex = mock(HttpExchange.class); assertThatThrownBy( () -> diff --git a/src/test/java/com/retailsvc/http/internal/ExceptionFilterTest.java b/src/test/java/com/retailsvc/http/internal/ExceptionFilterTest.java index 6208a09..1b08ca3 100644 --- a/src/test/java/com/retailsvc/http/internal/ExceptionFilterTest.java +++ b/src/test/java/com/retailsvc/http/internal/ExceptionFilterTest.java @@ -1,5 +1,7 @@ package com.retailsvc.http.internal; +import static org.mockito.Mockito.mock; + import com.retailsvc.http.ExceptionHandler; import com.retailsvc.http.NotFoundException; import com.sun.net.httpserver.Filter; @@ -10,10 +12,10 @@ class ExceptionFilterTest { @Test void delegatesToExceptionHandler() throws Exception { - HttpExchange ex = Mockito.mock(HttpExchange.class); - ExceptionHandler handler = Mockito.mock(ExceptionHandler.class); + HttpExchange ex = mock(HttpExchange.class); + ExceptionHandler handler = mock(ExceptionHandler.class); Filter f = new ExceptionFilter(handler); - Filter.Chain chain = Mockito.mock(Filter.Chain.class); + Filter.Chain chain = mock(Filter.Chain.class); Mockito.doThrow(new NotFoundException("x")).when(chain).doFilter(ex); f.doFilter(ex, chain); Mockito.verify(handler).handle(Mockito.eq(ex), Mockito.any(NotFoundException.class)); @@ -21,10 +23,10 @@ void delegatesToExceptionHandler() throws Exception { @Test void passThroughOnSuccess() throws Exception { - HttpExchange ex = Mockito.mock(HttpExchange.class); - ExceptionHandler handler = Mockito.mock(ExceptionHandler.class); + HttpExchange ex = mock(HttpExchange.class); + ExceptionHandler handler = mock(ExceptionHandler.class); Filter f = new ExceptionFilter(handler); - Filter.Chain chain = Mockito.mock(Filter.Chain.class); + Filter.Chain chain = mock(Filter.Chain.class); f.doFilter(ex, chain); Mockito.verify(chain).doFilter(ex); Mockito.verifyNoInteractions(handler); diff --git a/src/test/java/com/retailsvc/http/internal/RequestPreparationFilterTest.java b/src/test/java/com/retailsvc/http/internal/RequestPreparationFilterTest.java index a4b888b..1878c46 100644 --- a/src/test/java/com/retailsvc/http/internal/RequestPreparationFilterTest.java +++ b/src/test/java/com/retailsvc/http/internal/RequestPreparationFilterTest.java @@ -2,6 +2,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.Mockito.mock; import com.retailsvc.http.JsonMapper; import com.retailsvc.http.MethodNotAllowedException; @@ -15,6 +16,9 @@ import com.retailsvc.http.spec.PathTemplate; import com.retailsvc.http.spec.Server; import com.retailsvc.http.spec.Spec; +import com.retailsvc.http.spec.schema.BooleanSchema; +import com.retailsvc.http.spec.schema.IntegerSchema; +import com.retailsvc.http.spec.schema.NumberSchema; import com.retailsvc.http.spec.schema.StringSchema; import com.retailsvc.http.spec.schema.TypeName; import com.retailsvc.http.validate.DefaultValidator; @@ -34,7 +38,7 @@ class RequestPreparationFilterTest { private HttpExchange exchange(String method, String path, byte[] body) { - HttpExchange ex = Mockito.mock(HttpExchange.class); + HttpExchange ex = mock(HttpExchange.class); Mockito.when(ex.getRequestMethod()).thenReturn(method); Mockito.when(ex.getRequestURI()).thenReturn(URI.create(path)); Mockito.when(ex.getRequestHeaders()).thenReturn(new Headers()); @@ -78,7 +82,7 @@ void successPathBindsRequestContextDuringChain() throws Exception { AtomicReference seenOpId = new AtomicReference<>(); AtomicReference> seenPathParams = new AtomicReference<>(); - Filter.Chain chain = Mockito.mock(Filter.Chain.class); + Filter.Chain chain = mock(Filter.Chain.class); Mockito.doAnswer( inv -> { seenOpId.set(Request.operationId()); @@ -109,7 +113,7 @@ void unknownPathThrowsNotFound() { Filter f = newFilter(spec); HttpExchange ex = exchange("GET", "/missing", new byte[0]); - assertThatThrownBy(() -> f.doFilter(ex, Mockito.mock(Filter.Chain.class))) + assertThatThrownBy(() -> f.doFilter(ex, mock(Filter.Chain.class))) .isInstanceOf(NotFoundException.class); } @@ -127,7 +131,7 @@ void wrongMethodThrowsMethodNotAllowed() { Filter f = newFilter(spec); HttpExchange ex = exchange("POST", "/x", new byte[0]); - assertThatThrownBy(() -> f.doFilter(ex, Mockito.mock(Filter.Chain.class))) + assertThatThrownBy(() -> f.doFilter(ex, mock(Filter.Chain.class))) .isInstanceOf(MethodNotAllowedException.class); } @@ -146,9 +150,136 @@ void invalidQueryParamThrowsValidation() { Filter f = newFilter(spec); HttpExchange ex = exchange("GET", "/x?q=ab", new byte[0]); - assertThatThrownBy(() -> f.doFilter(ex, Mockito.mock(Filter.Chain.class))) + assertThatThrownBy(() -> f.doFilter(ex, mock(Filter.Chain.class))) .isInstanceOf(ValidationException.class) .extracting(t -> ((ValidationException) t).error().pointer()) .isEqualTo("/query/q"); } + + @Test + void integerQueryParamIsCoercedFromStringBeforeValidation() throws Exception { + var intSchema = new IntegerSchema(Set.of(TypeName.INTEGER), 1L, 100L, null, null, null, null); + var op = + new Operation( + "a", + HttpMethod.GET, + PathTemplate.compile("/x"), + Optional.empty(), + List.of(new Parameter("n", Parameter.Location.QUERY, true, intSchema)), + Map.of()); + Spec spec = specWith(op); + Filter f = newFilter(spec); + + HttpExchange ex = exchange("GET", "/x?n=42", new byte[0]); + Filter.Chain chain = mock(Filter.Chain.class); + f.doFilter(ex, chain); + Mockito.verify(chain).doFilter(ex); + } + + @Test + void integerQueryParamRejectsNonNumericString() { + var intSchema = new IntegerSchema(Set.of(TypeName.INTEGER), null, null, null, null, null, null); + var op = + new Operation( + "a", + HttpMethod.GET, + PathTemplate.compile("/x"), + Optional.empty(), + List.of(new Parameter("n", Parameter.Location.QUERY, true, intSchema)), + Map.of()); + Spec spec = specWith(op); + Filter f = newFilter(spec); + + HttpExchange ex = exchange("GET", "/x?n=abc", new byte[0]); + assertThatThrownBy(() -> f.doFilter(ex, mock(Filter.Chain.class))) + .isInstanceOf(ValidationException.class) + .extracting(t -> ((ValidationException) t).error().keyword()) + .isEqualTo("type"); + } + + @Test + void numberQueryParamIsCoercedFromStringBeforeValidation() throws Exception { + var numSchema = new NumberSchema(Set.of(TypeName.NUMBER), null, null, null, null, null, null); + var op = + new Operation( + "a", + HttpMethod.GET, + PathTemplate.compile("/x"), + Optional.empty(), + List.of(new Parameter("n", Parameter.Location.QUERY, true, numSchema)), + Map.of()); + Spec spec = specWith(op); + Filter f = newFilter(spec); + + HttpExchange ex = exchange("GET", "/x?n=1.5", new byte[0]); + Filter.Chain chain = mock(Filter.Chain.class); + f.doFilter(ex, chain); + Mockito.verify(chain).doFilter(ex); + } + + @Test + void numberQueryParamRejectsNonNumericString() { + var numSchema = new NumberSchema(Set.of(TypeName.NUMBER), null, null, null, null, null, null); + var op = + new Operation( + "a", + HttpMethod.GET, + PathTemplate.compile("/x"), + Optional.empty(), + List.of(new Parameter("n", Parameter.Location.QUERY, true, numSchema)), + Map.of()); + Spec spec = specWith(op); + Filter f = newFilter(spec); + + HttpExchange ex = exchange("GET", "/x?n=abc", new byte[0]); + assertThatThrownBy(() -> f.doFilter(ex, mock(Filter.Chain.class))) + .isInstanceOf(ValidationException.class) + .extracting(t -> ((ValidationException) t).error().keyword()) + .isEqualTo("type"); + } + + @Test + void booleanQueryParamCoercesTrueAndFalse() throws Exception { + var boolSchema = new BooleanSchema(Set.of(TypeName.BOOLEAN)); + var op = + new Operation( + "a", + HttpMethod.GET, + PathTemplate.compile("/x"), + Optional.empty(), + List.of(new Parameter("b", Parameter.Location.QUERY, true, boolSchema)), + Map.of()); + Spec spec = specWith(op); + Filter f = newFilter(spec); + + Filter.Chain trueChain = mock(Filter.Chain.class); + Filter.Chain falseChain = mock(Filter.Chain.class); + HttpExchange trueEx = exchange("GET", "/x?b=true", new byte[0]); + HttpExchange falseEx = exchange("GET", "/x?b=false", new byte[0]); + f.doFilter(trueEx, trueChain); + f.doFilter(falseEx, falseChain); + Mockito.verify(trueChain).doFilter(trueEx); + Mockito.verify(falseChain).doFilter(falseEx); + } + + @Test + void booleanQueryParamRejectsNonBooleanString() { + var boolSchema = new BooleanSchema(Set.of(TypeName.BOOLEAN)); + var op = + new Operation( + "a", + HttpMethod.GET, + PathTemplate.compile("/x"), + Optional.empty(), + List.of(new Parameter("b", Parameter.Location.QUERY, true, boolSchema)), + Map.of()); + Spec spec = specWith(op); + Filter f = newFilter(spec); + + HttpExchange ex = exchange("GET", "/x?b=yes", new byte[0]); + assertThatThrownBy(() -> f.doFilter(ex, mock(Filter.Chain.class))) + .isInstanceOf(ValidationException.class) + .extracting(t -> ((ValidationException) t).error().keyword()) + .isEqualTo("type"); + } } diff --git a/src/test/java/com/retailsvc/http/validate/StringIntegerNumberTest.java b/src/test/java/com/retailsvc/http/validate/StringIntegerNumberTest.java index 07407ba..db4bdfd 100644 --- a/src/test/java/com/retailsvc/http/validate/StringIntegerNumberTest.java +++ b/src/test/java/com/retailsvc/http/validate/StringIntegerNumberTest.java @@ -312,4 +312,21 @@ void numberFormatUnknownIsIgnored() { Set.of(TypeName.NUMBER), null, null, null, null, null, "definitely-not-a-format"); assertThatCode(() -> v.validate(1.5, s, "/v")).doesNotThrowAnyException(); } + + @Test + void integerRejectsNumericLookingString() { + IntegerSchema s = + new IntegerSchema(Set.of(TypeName.INTEGER), null, null, null, null, null, null); + assertThatThrownBy(() -> v.validate("42", s, "/v")) + .extracting(t -> ((ValidationException) t).error().keyword()) + .isEqualTo("type"); + } + + @Test + void numberRejectsNumericLookingString() { + NumberSchema s = new NumberSchema(Set.of(TypeName.NUMBER), null, null, null, null, null, null); + assertThatThrownBy(() -> v.validate("1234567890", s, "/v")) + .extracting(t -> ((ValidationException) t).error().keyword()) + .isEqualTo("type"); + } }