tools400 commented on code in PR #27099:
URL: https://github.com/apache/camel/pull/27099#discussion_r4143216241
##########
components/camel-openapi-validator/src/main/java/org/apache/camel/component/rest/openapi/validator/client/OpenApiRestClientRequestValidator.java:
##########
@@ -104,7 +107,21 @@ public ValidationError validate(Exchange exchange,
ValidationContext validationC
boolean customHeader
= !startsWithIgnoreCase(key, "Camel") &&
!filter.applyFilterToCamelHeaders(key, value, exchange);
if (customHeader) {
- builder.withHeader(key, exchange.getMessage().getHeader(key,
String.class));
+ if (value instanceof Collection<?> values) {
+ // A header sent more than once arrives as a Collection
(CollectionHelper.appendEntry).
+ // Converting that to a single String would hand the
validator the collection's
+ // toString(), such as "[a, b]" - a value the client never
sent - so the schema would
+ // be checked against fabricated data, and a repeated
scalar parameter would never be
+ // reported. Pass the values on instead, as the query
parameters below already do.
+ List<String> headerValues = new ArrayList<>(values.size());
+ for (Object headerValue : values) {
+
headerValues.add(exchange.getContext().getTypeConverter()
+ .convertTo(String.class, exchange,
headerValue));
+ }
+ builder.withHeader(key, headerValues);
Review Comment:
Agreed, and thanks for catching it. It is changed now:
- `null` elements are skipped.
- If the operation the request resolves to declares the header as an array,
the values are joined with `,` and passed as one header (RFC 9110 ยง5.3).
Scalars keep the list, so more than one value is still reported.
- The operation is resolved with swagger-request-validator's
`ApiOperationResolver`, built the same way the validator builds it, so both
look at the same parameters. It only runs when a header has more than one
value, and it is cached per `OpenAPI`.
Caveat for OpenAPI 3.1: swagger-parser produces `JsonSchema` there, and the
validator detects arrays only with `instanceof ArraySchema`. It treats every
3.1 header array as a scalar and parses the value as JSON, so even `X-Ids: 1,2`
fails with `Unable to parse JSON`, with or without this PR. I kept the same
`instanceof` check so the adapter agrees with the validator. The only
difference in 3.1: a repeated array header with numeric items used to pass by
accident, because `[1, 2]` from `toString()` happens to be valid JSON, and is
now reported. A proper fix for 3.1 belongs in swagger-request-validator.
_Claude Code on behalf of @tools400_
##########
components/camel-openapi-validator/src/test/java/org/apache/camel/component/rest/openapi/validator/client/OpenApiRestClientRequestValidatorTest.java:
##########
@@ -123,4 +124,104 @@ public void testValidateHeader() {
"application/json", "application/json", true, null, null,
null, null));
Assertions.assertNull(error);
}
+
+ @Test
+ public void testValidateRepeatedScalarHeader() {
+ exchange.setProperty(Exchange.REST_OPENAPI, openAPI);
+ exchange.setProperty(Exchange.CONTENT_TYPE, "application/json");
+ exchange.getMessage().setHeader(Exchange.HTTP_METHOD, "DELETE");
+ exchange.getMessage().setHeader(Exchange.HTTP_PATH, "pet/123");
+ exchange.getMessage().setHeader("Accept", "application/json");
+ exchange.getMessage().setBody("");
+
+ // A header sent more than once arrives as a List, exactly as
CollectionHelper.appendEntry
+ // leaves it. api_key is declared "type": "string", so two values
violate the contract.
+ exchange.getMessage().setHeader("api_key", List.of("key-one",
"key-two"));
+
+ RestClientRequestValidator.ValidationError error
+ = validator.validate(exchange, new
RestClientRequestValidator.ValidationContext(
+ "application/json", "application/json", false, null,
null, null, null));
+
+ Assertions.assertNotNull(error, "a repeated scalar header parameter
must be reported");
+ Assertions.assertEquals(400, error.statusCode());
+ Assertions.assertFalse(error.body().contains("[key-one, key-two]"),
+ "the collection must not be stringified into the validated
value");
+ }
+
+ @Test
+ public void testValidateSingleScalarHeaderStillPasses() {
+ exchange.setProperty(Exchange.REST_OPENAPI, openAPI);
+ exchange.setProperty(Exchange.CONTENT_TYPE, "application/json");
+ exchange.getMessage().setHeader(Exchange.HTTP_METHOD, "DELETE");
+ exchange.getMessage().setHeader(Exchange.HTTP_PATH, "pet/123");
+ exchange.getMessage().setHeader("Accept", "application/json");
+ exchange.getMessage().setHeader("api_key", "key-one");
+ exchange.getMessage().setBody("");
+
+ RestClientRequestValidator.ValidationError error
+ = validator.validate(exchange, new
RestClientRequestValidator.ValidationContext(
+ "application/json", "application/json", false, null,
null, null, null));
+
+ Assertions.assertNull(error);
+ }
+
+ @Test
+ public void testValidateRepeatedArrayHeaderIsReported() {
Review Comment:
Flipped: it is now `testValidateRepeatedArrayHeaderIsAccepted`. I also added
`testValidateRepeatedArrayHeaderWithInvalidItemIsReported` (a small inline
contract with `X-Ids: array<integer>`: `1`, `abc` is reported and `1`, `2`
passes) to show the joined values are still checked, and
`testValidateRepeatedArrayHeaderWithNullValueIsSkipped`.
_Claude Code on behalf of @tools400_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
