Repository navigation
[Core] Honour Expect: 100-continue in the JDK HTTP client - #50311
Merged
Merged
Conversation
AzureJdkHttpRequest.expectContinue() returned a hardcoded false, so java.net.http never performed the handshake and the Expect header was dropped by the restricted header filter. The body was therefore always sent immediately, and a service could not reject a request before receiving it. Return true when the request carries the header. The JDK adds the header itself and withholds the body until the service responds 100 Continue. Adds wire level tests that drive the client against a socket which delays its 100 Continue, asserting that no body bytes arrive before the response and that the full body arrives after it.
gunjansingh-msft
requested review from
a team,
Jonathan Giles (JonathanGiles),
Alan Zimmer (alzimmermsft) and
vcolin7
as code owners
September 1, 2026 12:24
|
Azure Pipelines: Successfully started running 1 pipeline(s). 35 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes the azure-core-http-jdk-httpclient transport to correctly opt into the JDK Expect: 100-continue handshake when the Azure Core request includes the Expect: 100-continue header, preventing the request body from being sent until the server responds with 100 Continue.
Changes:
- Implemented
AzureJdkHttpRequest.expectContinue()to returntruewhen the request indicates100-continue. - Added socket-level tests that assert the
Expecthandshake behavior on the wire (header present and body deferred). - Documented the behavioral change in the module changelog.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| sdk/core/azure-core-http-jdk-httpclient/src/main/java/com/azure/core/http/jdk/httpclient/implementation/AzureJdkHttpRequest.java | Enables JDK-level expect-continue behavior based on request headers. |
| sdk/core/azure-core-http-jdk-httpclient/src/test/java/com/azure/core/http/jdk/httpclient/JdkHttpClientExpectContinueTests.java | Adds coverage verifying on-the-wire deferment of the body until 100 Continue. |
| sdk/core/azure-core-http-jdk-httpclient/CHANGELOG.md | Notes the new behavior for requests carrying Expect: 100-continue. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
The core modules compile with -Werror, and URL(String) is deprecated as of Java 20, so the test failed to compile on the JDK 25 build agents.
gunjansingh-msft
force-pushed
the
core/jdk-expect-continue
branch
from
September 1, 2026 18:44
ec0126a to
8e04c98
Compare
6 tasks
Member
Author
|
#50094 (Storage blob |
gunjansingh-msft
added a commit
that referenced
this pull request
Sep 1, 2026
Records the behaviour those transports have once the azure-core fixes in #50311 and #50312 ship. Until Storage picks up those versions the four cases fail, which is the intended signal rather than a silent mismatch. Netty stays as it is: the change there observes the interim response and does not defer the body.
Expect is a comma separated list and values may carry whitespace, so compare each expectation rather than the header as a whole. Otherwise "100-Continue", " 100-continue " or "100-continue, foo" would silently disable the handshake. The test server now reads for the whole settle window rather than taking an available() snapshot after a sleep, so a body already in flight is counted rather than missed. Adds the header spelling variants as cases.
JdkAsyncHttpClient is unsupported on Java 11 and below, so the tests errored on the JDK 11 agent instead of being skipped. The other test classes in this module already carry the same guard.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Bodyless requests can incorrectly enter the expect-continue wait path.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Balanced
The wrapper replaces the body with noBody() for GET and HEAD, and a request may carry no content at all. The JDK enters its wait path on the flag alone, so a bodyless request could emit the expectation and wait for a handshake with nothing to send. Gate the flag on a non-empty publisher, and cover GET and HEAD carrying the header.
Member
Author
|
/azp run java - pullrequest |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Alan Zimmer (alzimmermsft)
approved these changes
Sep 2, 2026
Puts the cheaper length check ahead of parsing the header, and lets expectsContinue pull the header itself so other callers do not have to.
Alan Zimmer (alzimmermsft)
approved these changes
Sep 9, 2026
Isabelle (ibrandes)
added a commit
that referenced
this pull request
Oct 28, 2026
* [Storage] Add Expect: 100-continue support Applies the HTTP header Expect: 100-continue to requests that carry a body, so that a body is not uploaded just to be rejected while the service is under load. The service can reject the request before a single byte of body is sent, which cuts the bandwidth wasted retrying large uploads against a throttling service. - Adds Request100ContinueOptions and Request100ContinueMode (AUTO, ALWAYS, NEVER) to azure-storage-common. AUTO, the default, applies the header only for a window after the service responds 429, 500, or 503, so the extra round trip is only paid when it earns its keep. - The policies are added below the retry policy so a throttling response takes effect on the very next attempt rather than only on a later call, and above the credential policies since headers may affect the string to sign. - Wired into the blob, data lake, file share, and queue pipelines, and exposed on the blob client builders via request100ContinueOptions. - Can be turned off without a code change by setting the system property or environment variable AZURE_STORAGE_DISABLE_EXPECT_CONTINUE_HEADER. Ported from azure-sdk-for-net #40611 and #52438. * [Storage] Address PR feedback on Expect: 100-continue - Rename Request100ContinueOptions/Request100ContinueMode to ExpectContinueOptions/ExpectContinueMode, the mode values to APPLY_ON_THROTTLE/ON/OFF, autoInterval to throttleInterval, and the builder method to expectContinueBehavior, matching the .NET names. - Make ExpectContinueOnThrottlePolicy extend HttpPipelineSyncPolicy rather than ExpectContinuePolicy. Both policies are now final and share their logic through a package private helper, so neither advertises an extension point that callers outside the package cannot actually use. - Allow the Expect header in the blob default log options, so it is visible when debugging whether a request negotiated the handshake. - Replace the unrelated timeouts link in the options javadoc with RFC 9110. - Add ExpectContinueTransportTests, which drives each HTTP client against a real socket and asserts on what reached the wire and when. The transport tests show that setting the header does not by itself make a client wait. Only okhttp performs the handshake. netty and vertx send the header and stream the body immediately, and jdk-httpclient drops the header entirely since Expect is restricted by java.net.http.HttpClient. That is recorded in the test matrix and documented on ExpectContinueOptions and in the changelog rather than left implicit. * [Storage] Test the allowed Expect log header, trim changelog Cover the default log options change with an assertion, and keep the changelog to the feature itself. The transport caveat stays on ExpectContinueOptions, where the behavior it describes lives. * [Storage] Match .NET exactly on enum names and log the Expect header everywhere Use the .NET casing for the mode values, ApplyOnThrottle / On / Off, rather than Java's upper snake case. Checkstyle accepts it. Allow the Expect header in the default log options of the queue, file share and data lake builders too, not only blob, as the review comment was a general one. * [Storage] Mirror the .NET expect-continue tests and cover the async path Restructure ExpectContinueTests so the first section mirrors ExpectContinueTests.cs test for test, with the same names and the same case values, making the two suites directly comparable. Java specific coverage, including the per retry behaviour that .NET has no test for, moves to a clearly separated second section. ThrottlePolicyRevertsAfterBackoff is disabled in .NET (Azure/azure-sdk-for-net#41368); the Java mirror is enabled, since the window is a monotonic nanoTime deadline and oversleeping it can only close the window. Extend the transport test to transport x sync/async, as those are separate code paths in every client. All four transports behave the same on both paths, so the earlier finding is unchanged. * [Storage] Scope expect-continue to blob only and trim comments Revert the queue, file share and data lake packages entirely, so the header is applied only where a customer can configure it. Drop the explanatory comments that were not carrying their weight, including the cross references to the .NET implementation. * [Storage] Remove an unrelated file that was committed by mistake sdk/storage/TYPESPEC_MIGRATION_ESTIMATE.md is unrelated to this change and was picked up accidentally. * [Storage] Move the wire level tests into ExpectContinueTests Keep them in the class the review comment was on rather than a separate file. Each one opens and closes its own server, so the socket fixture does not run for the tests that do not need it. * [Storage] Address review feedback on Expect: 100-continue - Drop the azure-storage-common changelog entry; the changes there are implementation level and the customer facing option is on the blob builders. - Remove the okhttp, vertx and jdk test dependencies. They already come transitively from azure-core-test, so they were redundant. - Move ExpectContinuePolicy and ExpectContinueOnThrottlePolicy into com.azure.storage.common.policy alongside the other storage policies, and rename the shared helper to ExpectContinuePolicyHelper. - Use upper snake case for the mode constants. - Reject a negative content length threshold or throttle interval rather than silently changing behaviour, with boundary tests for both. * [Storage] Do not apply Expect: 100-continue unless it is configured The handshake needs the HTTP client to withhold the request body until the service responds, and the default transport does not do that, so a default client would pay for the header and save nothing. The policy is now added only when expectContinueBehavior is set. * adding support for expect continue 100 to azure core netty * Avoid the deprecated URL constructor in the netty observation test The core modules compile with -Werror, and URL(String) is deprecated as of Java 20, so the test failed to compile on the JDK 25 build agents and took the whole build down with it. * Expect the JDK and Vert.x clients to defer the request body Records the behaviour those transports have once the azure-core fixes in #50311 and #50312 ship. Until Storage picks up those versions the four cases fail, which is the intended signal rather than a silent mismatch. Netty stays as it is: the change there observes the interim response and does not defer the body. * Let the JDK and Vert.x expectations follow the azure-core version Those two clients honour the handshake from azure-core 1.2.0-beta.1 onwards, so the behaviour depends on whether the build resolves a released artifact or the repository source. Asserting either one fails half the pipeline, so assert what holds either way: the exchange completes, the body arrives in full, and a withheld body implies the header reached the wire. Once Storage consumes a release carrying the fix these become DEFERS_BODY. Netty and okhttp keep their exact expectations. * Revert "Let the JDK and Vert.x expectations follow the azure-core version" This reverts commit 644c196. * Revert "Storage - STG105 NFS List V2 Files and Directories (#50036)" This reverts commit 83dc6e2. * add recordings for prev features that were missing * addressing comments and fixing problems left by base merge * addressing comment * preventing expect continute jdk tests from running on java 11 or lower * moving to internal.policy * addressing comments * removing dupe import * fixing flaky test * assets tag update --------- Co-authored-by: Isabelle <ibrandes@microsoft.com> Co-authored-by: browndav <browndav@microsoft.com>
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.
AzureJdkHttpRequest.expectContinue()returned a hardcodedfalse, sojava.net.httpnever performed theExpect: 100-continuehandshake. The header was also dropped by the restricted header filter, so it never reached the wire either. The result was that a request carrying the expectation behaved exactly like one without it: the body was sent immediately and the service had no opportunity to reject it first.The JDK implements the handshake itself once the request opts in, so the fix is to return
truewhen the request carries the header. The JDK then adds the header and withholds the body until the service responds100 Continue. The restricted header filter stripping our copy is harmless for the same reason.Verification
JdkHttpClientExpectContinueTestsdrives the client against a raw socket that deliberately delays its100 Continue, and asserts on what reached the wire and when:Expectheader on the wire100 Continue100 ContinueA second test asserts that a request without the header still sends its body immediately, so the behaviour is scoped to requests that opt in.
The 48 pre-existing errors in this module's suite are unrelated - they reproduce identically on
mainwithout this change (52 tests / 48 errors onmain, 54 tests / 48 errors here, the two extra being the new passing tests).Context
This came out of adding
Expect: 100-continuesupport to the Storage blob clients (#50094). Of the four transports, onlyazure-core-http-okhttpperforms the handshake today;azure-core-http-vertxsends the body immediately despite the Vert.x client exposingsendHead()andcontinueHandler(), andazure-core-http-nettyhas no client side support in netty or reactor-netty. A companion change for vertx follows.