fix: stream non-replayable request bodies in the JDK transport#141
Open
OmarAlJarrah wants to merge 1 commit into
Open
fix: stream non-replayable request bodies in the JDK transport#141OmarAlJarrah wants to merge 1 commit into
OmarAlJarrah wants to merge 1 commit into
Conversation
The java.net.http transport routed every request body above the 64 KiB eager threshold (or of unknown length) through a "streaming" publisher that first forced the body replayable via toReplayable(), draining the entire body into an in-memory Buffer before publishing a single byte. For the bodies that path exists to serve — large or unknown-length one-shot uploads — this defeated streaming: a 2 GiB upload needed ~2 GiB of contiguous heap, and a body above the byte-array/segment limits failed outright. The same SDK request streamed fine under the OkHttp transport, so the two transports diverged on the one-shot streaming contract. Stream the first subscription directly from the body, with no up-front buffering: - Replayable bodies are unchanged — each subscription re-reads the body from its source, so proxy-auth (407) retries and HTTP/2 GOAWAY replays still carry the full bytes. - Non-replayable (one-shot) bodies stream their first subscription straight from the source. A consumed one-shot body cannot be replayed, so a second subscription fails loudly with a clear "supply a replayable body" error rather than shipping a truncated or empty body. This matches the consume-once discipline of OneShotInputStreamRequestBody / BufferedSourceRequestBody in sdk-core and keeps internal resends correct by failing visibly instead of corrupting the request. The resend refusal is raised as an UncheckedIOException wrapping the explanatory IOException: the JDK 11 ofInputStream reader catches a checked IOException from read() and swallows it as a silent end-of-stream, which would let the resend complete truncated. An unchecked throw propagates — synchronously on the request stack under JDK 11, via onError on later JDKs. Tests: a non-replayable streaming body's writeTo is asserted not to have run after adaptBody returns (proving no up-front drain), a one-shot body's resend is asserted to fail loudly with the replayable-body message, and a mid-write failure is asserted to surface as an IOException. Closes #113
Member
Author
|
This stops the JDK transport from draining a large/unknown-length non-replayable body fully into heap before publishing — the old Issues
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The
java.net.httptransport routed every request body above the 64 KiB eager threshold (or of unknown declared length) through a "streaming" publisher that first forced the body replayable viaRequestBody.toReplayable(). That drains the entire body into an in-memoryBufferbefore publishing a single byte.For exactly the bodies that path exists to serve — large or unknown-length one-shot uploads — this defeated streaming:
Change
sdk-transport-jdkhttp/.../internal/BodyPublishers.ktnow streams the first subscription directly from the body, with no up-front buffering:GOAWAYreplay still carry the full bytes.OneShotInputStreamRequestBody/BufferedSourceRequestBodyinsdk-core: a resend of a one-shot body is a caller error, surfaced visibly instead of corrupting the request. Callers that need resend support supply a replayable body.JDK 11 detail
The resend refusal is raised as an
UncheckedIOExceptionwrapping the explanatoryIOException, not a bare checkedIOException. The JDK 11ofInputStreamreader (StreamIterator) catches a checkedIOExceptionfromread()and swallows it as a silent end-of-stream, which would let the resend complete with a truncated body. An unchecked throw is not caught there, so it propagates — synchronously on the subscriber'srequeststack under JDK 11, and viaonErroron later JDKs — with the original cause preserved.No public/explicit API change (
BodyPublishersis internal);apiCheckpasses with noapiDump.Tests
nonReplayableStreamingBodyIsNotBufferedUpFront— asserts the body'swriteTohas not run afteradaptBodyreturns (the decisive, non-flaky signal that the body was not drained up front), then that it streams the full body on subscription.oneShotStreamingBodyRefusesResendLoudly— first subscription streams the full one-shot body; the second fails loudly with anIOExceptioncarrying the replayable-body message.streamingBodyThatFailsMidWriteFailsLoudly— a non-replayable body that aborts mid-write surfaces as anIOException(no silent truncation).Gated build
Ran (module-scoped,
--no-daemon):Result: BUILD SUCCESSFUL (detekt is skipped on this module per the repo's JDK-11 toolchain note).
Closes #113