[Core] Honour Expect: 100-continue in the Vert.x HTTP client - #50312
Open
gunjansingh-msft wants to merge 3 commits into
Open
[Core] Honour Expect: 100-continue in the Vert.x HTTP client#50312gunjansingh-msft wants to merge 3 commits into
gunjansingh-msft wants to merge 3 commits into
Conversation
sendBody always wrote the headers and the body together, so a request carrying Expect: 100-continue behaved exactly like one without it and the service had no opportunity to reject it before receiving the body. Vert.x already exposes the two pieces needed. When the header is present, send the head and write the body from continueHandler; otherwise keep the existing behaviour. The body writing is now shared by both paths, so this covers in-memory and streaming bodies alike. 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:28
|
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
Updates the Vert.x transport in azure-core to properly honor Expect: 100-continue by sending request headers first and deferring request-body transmission until the server signals 100 Continue, aligning the transport’s on-the-wire behavior with HTTP expectations.
Changes:
- Teach
VertxHttpClientto usesendHead()+continueHandler()whenExpect: 100-continueis present, and reuse a shared body-writing path. - Add a socket-level unit test validating that the body is withheld until
100 Continue, and that requests without the header remain unchanged. - Document the behavior 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-vertx/src/main/java/com/azure/core/http/vertx/VertxHttpClient.java | Defers body write on Expect: 100-continue using sendHead()/continueHandler() and factors body writing into a helper. |
| sdk/core/azure-core-http-vertx/src/test/java/com/azure/core/http/vertx/VertxHttpClientExpectContinueTests.java | Adds raw-socket tests that assert on wire timing and presence/absence of Expect behavior. |
| sdk/core/azure-core-http-vertx/CHANGELOG.md | Notes new Expect: 100-continue handshake behavior in Vert.x transport. |
💡 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/vertx-expect-continue
branch
from
September 1, 2026 18:44
7b72f71 to
7b51377
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.
Send the body after a one second fallback if the service never answers the expectation, so a server that ignores it cannot stall the request until the response timeout. The body is guarded so it is written once whether the interim response or the fallback comes first, and the timer is cancelled on continue. Expect is a comma separated list and values may carry whitespace, so compare each expectation rather than the header as a whole. The test server now reads for the whole settle window rather than taking an available() snapshot after a sleep. Adds cases for the header spelling variants and for a service that ignores the expectation entirely.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The fallback timer can write the body after the request has already received a final response or failed.
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
Comment on lines
+207
to
+210
| io.vertx.core.Context vertxContext = Vertx.currentContext(); | ||
| Long fallbackTimerId = vertxContext == null | ||
| ? null | ||
| : vertxContext.owner().setTimer(EXPECT_CONTINUE_TIMEOUT.toMillis(), ignored -> writeOnce.run()); |
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.
VertxHttpClient.sendBodyalways wrote the headers and the body together, so a request carryingExpect: 100-continuebehaved exactly like one without it: the body went out immediately and the service had no opportunity to reject it first.Vert.x already exposes what is needed -
sendHead()on the request andcontinueHandler()on the client stream. When the header is present, this sends the head and writes the body fromcontinueHandler; otherwise the existing path is unchanged.The body writing has been extracted so both branches share it, which means this covers both the in-memory bodies and the reactive
Flux<ByteBuffer>/InputStreampath. The in-memory branch switches betweensend(buffer)andend(buffer)depending on whether the head has already gone out; the streaming path useswrite/endand works unchanged in both cases.Verification
VertxHttpClientExpectContinueTestsdrives 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 a request without the header still sends its body immediately, so the behaviour is scoped to requests that opt in.
The 32 pre-existing errors in this module's suite are unrelated - they reproduce identically on
mainwithout this change (47 tests / 32 errors onmain, 49 tests / 32 errors here, the two extra being the new passing tests).Expect-continue fallback
RFC 9110 says a client "SHOULD NOT wait for an indefinite period before sending the content", and a service may ignore the expectation entirely and simply wait for the body. The body is therefore sent after a one second fallback if no interim response arrives, matching the defaults used by .NET and Go. The write is guarded so the body goes out once whether the interim response or the fallback wins, and the timer is cancelled when
continueHandlerfires.sendsBodyWhenTheServiceIgnoresTheExpectationcovers this against a server that never answers.The interval is a constant rather than a configurable option; happy to expose it if you would prefer, and it would be worth being consistent across the transports.
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. A companion change for the JDK client is in #50311.azure-core-http-nettyis a larger problem: neither netty nor reactor-netty has client side support, and the reactor-netty maintainers consider sending the body immediately to be RFC compliant (reactor/reactor-netty#4353).