allthingssecurity commented on code in PR #27924:
URL: https://github.com/apache/camel/pull/27924#discussion_r4291296958
##########
components/camel-jetty/src/main/java/org/apache/camel/component/jetty12/AttachmentHttpBinding.java:
##########
@@ -157,11 +161,45 @@ protected void
populateRequestParameters(HttpServletRequest request, Message mes
}
}
} catch (Exception e) {
- // In Jetty 12.1.0+, getParameterNames() may fail for multipart
requests
- // when multipart filter is disabled. This is expected behavior
when
- // enableMultipartFilter=false is used, so we silently skip
parameter processing
- if (LOG.isDebugEnabled()) {
- LOG.debug("Failed to populate request parameters, likely due
to multipart parsing failure: {}", e.getMessage());
+ // In Jetty 12.1.0+, getParameterNames() reads the query and the
body parameters together and fails for the
+ // whole request when it cannot read the body parameters, for
example a multipart request when the
+ // multipart filter is disabled (enableMultipartFilter=false), or
a form larger than Jetty's
+ // maxFormContentSize. The body parameters are skipped (a form
body is mapped by readFormUrlEncodedBody).
+ if (names == null) {
+ // nothing has been mapped yet, and the query does not depend
on the body, so map its parameters
+ LOG.debug("Failed to read the request parameters, mapping the
query parameters only: {}", e.getMessage());
+ populateQueryParameters(request, message);
+ } else if (LOG.isDebugEnabled()) {
+ LOG.debug("Failed to populate request parameters: {}",
e.getMessage());
+ }
+ }
+ }
+
+ private void populateQueryParameters(HttpServletRequest request, Message
message) {
Review Comment:
Good point. I kept the fallback separate so the change to the main loop
stays zero, but a shared `mapParameterValues(...)` helper is cleaner. Happy to
do that refactor as a small follow-up so both paths share the sanitize / filter
/ append code.
_Claude Code on behalf of allthingssecurity_
##########
components/camel-jetty/src/test/java/org/apache/camel/component/jetty/HttpBindingQueryParametersUnreadableBodyTest.java:
##########
@@ -0,0 +1,134 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.component.jetty;
+
+import java.net.URI;
+import java.net.http.HttpClient;
+import java.net.http.HttpRequest;
+import java.net.http.HttpResponse;
+import java.nio.charset.StandardCharsets;
+import java.util.List;
+
+import org.apache.camel.Exchange;
+import org.apache.camel.Message;
+import org.apache.camel.builder.RouteBuilder;
+import org.apache.camel.component.mock.MockEndpoint;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
+
+/**
+ * A form that is larger than Jetty's maxFormContentSize (200000 bytes by
default), or a multipart request with the
+ * multipart filter disabled, makes Jetty fail to read the request parameters.
The query parameters must still be
+ * mapped.
+ */
+public class HttpBindingQueryParametersUnreadableBodyTest extends
BaseJettyTest {
+
+ // larger than the default maxFormContentSize of Jetty
+ private static final String LARGE_VALUE = "x".repeat(250_000);
+
+ @Test
+ public void queryParametersOfLargeFormAreMapped() throws Exception {
+ MockEndpoint mock = getMockEndpoint("mock:form");
+ mock.expectedMessageCount(1);
+
+ int status = post("?id=42&tag=a&tag=b&text=hello+world",
"name=camel&data=" + LARGE_VALUE);
+
+ mock.assertIsSatisfied();
+ assertEquals(200, status);
+ Message in = mock.getReceivedExchanges().get(0).getIn();
+ assertEquals("42", in.getHeader("id"));
+ assertEquals(List.of("a", "b"), in.getHeader("tag"));
+ assertEquals("hello world", in.getHeader("text"));
+ // the form is mapped from the body as before
+ assertEquals("camel", in.getHeader("name"));
+ assertEquals(LARGE_VALUE.length(), in.getHeader("data",
String.class).length());
+ }
+
+ @Test
+ public void queryParametersOfLargeFormAreFiltered() throws Exception {
+ MockEndpoint mock = getMockEndpoint("mock:form");
+ mock.expectedMessageCount(1);
+
+ int status = post("?id=42&CamelHttpResponseCode=500", "data=" +
LARGE_VALUE);
Review Comment:
The headers map is case-insensitive, so the existing
`assertFalse(...containsKey(Exchange.HTTP_RESPONSE_CODE))` already covers a
mixed-case `camelhttpresponsecode` as well; I left the test as it is.
_Claude Code on behalf of allthingssecurity_
--
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]
