allthingssecurity opened a new pull request, #27755: URL: https://github.com/apache/camel/pull/27755
# Description [CAMEL-25571](https://issues.apache.org/jira/browse/CAMEL-25571) `HttpConverter.toInputStream(HttpServletRequest, Exchange)` reads the request body for every consumer that uses `DefaultHttpBinding.parseBody` (`DefaultHttpBinding:715` -> `HttpHelper.readRequestBodyFromServletRequest`, `HttpHelper:163`): camel-servlet and camel-jetty. It returned the servlet stream as it is when `available() <= 0` ("there is no data, so we cannot uncompress"), and uncompressed a `Content-Encoding: gzip` body only otherwise. `available()` is the number of bytes that can be read without blocking, so it is also 0 while the body is still on its way: - a client that sends `Expect: 100-continue` waits for the interim response before it sends the body, so no byte of the body can be there when the consumer starts to read; - a client can send the body in a later packet than the headers. In that case the route got the compressed bytes of a gzip request, without an error, and whether it did depended on timing. History: the `available()` check was added to `HttpConverter` by CAMEL-5806 (commit 1ba5aa2f4b, 2015-02-16, GET with a body), together with a twin check in `DefaultHttpBinding.parseBody`. CAMEL-8643 ("Http Post from a streaming client sometimes fails to parse", 2.15.3 / 2.16.0, commit 7790c6e0fe) reported that twin check for exactly this reason, "on a slow or remote client the binary stream isn't always immediately available", and it was made opt-in (`eagerCheckContentAvailable`, default false). That commit did not touch `HttpConverter`, where the check stayed unconditional. This PR does not change `eagerCheckContentAvailable`. This change: for a request with gzip content encoding (and no `CamelSkipGzipEncoding` property on the exchange) the converter reads the first byte to find out whether there is a body, and only passes an empty body on as it is; a body that is there is uncompressed whenever it arrives. Requests without gzip content encoding, and exchanges with `CamelSkipGzipEncoding`, are not read here, as before. Behaviour changes, all in the 4.24 upgrade guide (`camel-4x-upgrade-guide-4_24.adoc`; if that file is not on main yet, this PR creates it with the header of the 4_23 guide and adds it to the list in `camel-4x-upgrade-guide.adoc`): - A gzip request body that arrives after the headers is now uncompressed. - A body that is labelled gzip but is not gzip data: when its bytes had arrived, main already failed with `ZipException` in the converter, and still does. When it arrives late, main passed the bytes to the route as they were (200); now it fails in the same way as the first case: Jetty answers 500 and the route is not called. - With `disableStreamCache=true`: for a gzip request the converter now waits for the first byte of the body (and `GZIPInputStream` for the GZIP header) before the route is called, where main returned the stream at once when nothing had arrived. With stream caching (the default) the consumer already read the whole body before the route. A request without a body cannot block here: with `Content-Length: 0` `DefaultHttpBinding.parseBody` returns before the converter is called (`DefaultHttpBinding:699`), and a chunked request without data gives `read() == -1` (both in `JettyGzipRequestEmptyBodyTest`). Scope: - Goes through `HttpConverter`: camel-servlet and camel-jetty (and what extends them, such as camel-atmosphere-websocket, whose consumer extends `ServletConsumer`). Outside this repository, `SpringBootPlatformHttpBinding` of camel-spring-boot extends `DefaultHttpBinding` and calls `super.parseBody` when the endpoint has `useStreaming=true` (read on camel-spring-boot main, `SpringBootPlatformHttpBinding.java:70,293`; not run). Without `useStreaming` it reads the bytes of the servlet stream itself and does not come here. - Does not: camel-platform-http-vertx (`VertxPlatformHttpConsumer:149-153`, Vert.x body handlers), camel-undertow (`DefaultUndertowHttpBinding:160,190`, `readFromChannel`), camel-netty-http (`DefaultNettyHttpBinding:107-139`, the Netty content). None of the three has a main dependency on camel-http-common. I did not look at how they treat a gzip request body. Tests: - `JettyGzipRequestExpectContinueTest` (camel-jetty, new): a socket client sends a POST with `Content-Encoding: gzip` and `Expect: 100-continue`, waits for the `100 Continue` of the server, then sends the body. There is no sleep in it. - `gzipBodySentAfter100ContinueIsUncompressed` fails on main in all 3 surefire runs (the first run and its 2 reruns): `mock://result Body of message: 0. Expected: <Hello World, sent with Content-Encoding: gzip> but was:` the compressed bytes. - `bodyThatIsNotGzipSentAfter100ContinueIsNotPassedToTheRoute` fails on main in all 3 runs (`Expected 500 but was: HTTP/1.1 200 OK`). It pins the second behaviour change above, it is not a bug of main by itself. - `JettyGzipRequestEmptyBodyTest` (camel-jetty, new, `disableStreamCache=true`): `contentLengthZero` and `chunkedWithoutData`. Both pass on main too; they guard that the new read does not wait for a body that does not come. - `HttpConverterGzipTest` (camel-http-common, new, 9 cases), the converter with a servlet stream whose `available()` is 0 or the real number: - fail on main (3): `gzipBodyIsUncompressedWhenNoByteIsAvailableYet` (exchange null), `gzipBodyIsUncompressedWhenNoByteIsAvailableYetAndTheExchangeDoesNotSkipGzip` (both `expected: <Hello World> but was:` the compressed bytes), `bodyThatIsNotGzipFailsWhenNoByteIsAvailableYet` (`Expected java.util.zip.ZipException to be thrown, but nothing was thrown`; the behaviour change again). - pass on main too (6), they pin what must not change: `gzipBodyIsUncompressedWhenItIsAvailable`, `emptyBodyWithGzipContentEncodingIsEmpty` (the new "empty body" branch, which would otherwise fail with `EOFException` in `GZIPInputStream`), `skipGzipEncodingLeavesTheBodyCompressedWhenNoByteIsAvailableYet`, `skipGzipEncodingLeavesTheBodyCompressedWhenItIsAvailable` (the restructured `CamelSkipGzipEncoding` branch: the stream of the request is returned unread), `bodyWithoutGzipContentEncodingIsNotRead`, `bodyThatIsNotGzipFailsWhenItIsAvailable`. - Existing gzip request tests in camel-jetty: `HttpGZipEncodingTest` is `@Disabled`. `JettyContentTypeTest.testContentTypeWithGZipEncoding` is enabled and sends a gzip request with the camel-http producer; it passes with this change (I did not run it separately on main; it is not one of the two tests that fail there), so it does not show the problem. The new test is the only one where the body is certain to arrive after the headers. - With the change: camel-http-common 48 tests, 0 failures; camel-servlet 110 tests, 0 failures, 2 skipped; camel-jetty 403 tests, 1 failure and 1 error, 23 skipped. The two that do not pass, `JettyEmptyCharsetParameterTest` (500 instead of 200) and `JettyXsltHttpTemplateTest` (`SocketException: Unexpected end of file from server`), fail in the same way on my machine without this change (I ran both against unchanged main code, `lean-gzip-main-baseline-two-tests.log`), so they are not caused by it. Not run: the other modules that depend on camel-http-common or camel-servlet (camel-http, camel-cxf-transport, camel-jetty-common, camel-atmosphere-websocket), and camel-spring-boot. Found with a small Lean 4 model of the converter (request = gzip flag, skip flag, body, bytes arrived): "a gzip body is uncompressed unless it is empty or skipped" fails on main for every non-empty gzip body of which nothing has arrived, the fix is proved to meet it for every request, and to equal main whenever a byte has arrived or the body is empty. The model does not cover a body that is labelled gzip but is not gzip. I then reproduced it with the Jetty consumer. # Target - [x] I checked that the commit is targeting the correct branch (Camel 4 uses the `main` branch) # Tracking - [x] If this is a large change, bug fix, or code improvement, I checked there is a [JIRA issue](https://issues.apache.org/jira/browse/CAMEL) filed for the change (usually before you start working on it). # Apache Camel coding standards and style - [x] I checked that each commit in the pull request has a meaningful subject line and body. - [ ] I have run `mvn clean install -DskipTests` locally from root folder and I have committed all auto-generated changes. (I built and tested camel-http-common, camel-jetty and camel-servlet, including the formatter and import-sort plugins. No generated file changes. I did not run the full root build.) # AI-assisted contributions - [x] If this PR includes AI-generated code, commits have proper co-authorship attribution (e.g., `Co-authored-by` trailers) and the PR description identifies the AI tool used. This PR was prepared with Claude Code (Claude Opus 5.5). The commit carries a `Co-Authored-By` trailer. _Claude Code on behalf of allthingssecurity_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
