Skip to content

Commit 8bf370d

Browse files
[Core] Honour Expect: 100-continue in the Vert.x HTTP client (#50312)
* Honour Expect: 100-continue in the Vert.x HTTP client 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. * Avoid the deprecated URL constructor in the 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. * Address review feedback 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. * Do not write the body once the exchange is terminal The fallback was cancelled only from continueHandler, so a service that rejected the headers with a final status left the timer armed. It would then write onto a finished exchange, and for a streaming body would subscribe the source needlessly. Cancel the fallback and claim the write when the response future reaches any terminal state, so neither the timer nor a late interim response can send the body afterwards. Adds a regression test for a service that answers the headers with 401 and never reads the body.
1 parent fbeb1a7 commit 8bf370d

3 files changed

Lines changed: 386 additions & 17 deletions

File tree

‎sdk/core/azure-core-http-vertx/CHANGELOG.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,11 @@
33
## 1.2.0-beta.1 (Unreleased)
44

55
### Features Added
6+
- Requests carrying the `Expect: 100-continue` header now perform the handshake: the headers are sent with
7+
`sendHead()` and the request body is written from `continueHandler()`, rather than sending headers and body
8+
together.
9+
If the service does not answer the expectation the body is sent after a one second fallback, so the request does
10+
not stall.
611

712
### Breaking Changes
813

‎sdk/core/azure-core-http-vertx/src/main/java/com/azure/core/http/vertx/VertxHttpClient.java‎

Lines changed: 86 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import com.azure.core.util.logging.ClientLogger;
2727
import io.vertx.core.Future;
2828
import io.vertx.core.Promise;
29+
import io.vertx.core.Vertx;
2930
import io.vertx.core.buffer.Buffer;
3031
import io.vertx.core.http.HttpClientOptions;
3132
import io.vertx.core.http.HttpClientRequest;
@@ -39,6 +40,7 @@
3940
import java.io.UncheckedIOException;
4041
import java.time.Duration;
4142
import java.util.Objects;
43+
import java.util.concurrent.atomic.AtomicBoolean;
4244
import java.util.concurrent.CompletableFuture;
4345
import java.util.concurrent.ExecutionException;
4446
import java.util.concurrent.TimeUnit;
@@ -49,6 +51,10 @@
4951
class VertxHttpClient implements HttpClient {
5052
private static final ClientLogger LOGGER = new ClientLogger(VertxHttpClient.class);
5153

54+
// How long to wait for an interim 100 Continue before sending the body anyway, matching the defaults used by
55+
// other HTTP stacks. RFC 9110 requires a client not to wait indefinitely.
56+
private static final Duration EXPECT_CONTINUE_TIMEOUT = Duration.ofSeconds(1);
57+
5258
final io.vertx.core.http.HttpClient client;
5359
final HttpClientOptions buildOptions;
5460
private final Duration responseTimeout;
@@ -183,26 +189,89 @@ private void sendBody(ContextView contextView, HttpRequest azureRequest, Progres
183189

184190
if (body == null) {
185191
vertxRequest.send().onFailure(promise::fail);
192+
return;
193+
}
194+
195+
if (expectsContinue(azureRequest.getHeaders().getValue(HttpHeaderName.EXPECT))) {
196+
// Send the headers and hold the body until the service accepts the expectation. If it answers with a
197+
// final status instead, that response completes the request and the body is never written.
198+
AtomicBoolean bodyWritten = new AtomicBoolean();
199+
Runnable writeOnce = () -> {
200+
if (bodyWritten.compareAndSet(false, true)) {
201+
writeBody(contextView, azureRequest, progressReporter, vertxRequest, promise, body, true);
202+
}
203+
};
204+
205+
// A service may ignore the expectation entirely and wait for the body, so the request cannot depend on
206+
// the interim response arriving. RFC 9110 allows the content to be sent after a short wait.
207+
io.vertx.core.Context vertxContext = Vertx.currentContext();
208+
Long fallbackTimerId = vertxContext == null
209+
? null
210+
: vertxContext.owner().setTimer(EXPECT_CONTINUE_TIMEOUT.toMillis(), ignored -> writeOnce.run());
211+
Runnable cancelFallback = () -> {
212+
if (fallbackTimerId != null && vertxContext != null) {
213+
vertxContext.owner().cancelTimer(fallbackTimerId);
214+
}
215+
};
216+
217+
// Once the exchange is terminal, for example the service rejected the headers outright or sending them
218+
// failed, there is nothing left to send. Claim the write so neither the fallback nor a late interim
219+
// response can subscribe the body onto a finished request.
220+
promise.future().onComplete(ignored -> {
221+
cancelFallback.run();
222+
bodyWritten.set(true);
223+
});
224+
225+
vertxRequest.continueHandler(ignored -> {
226+
cancelFallback.run();
227+
writeOnce.run();
228+
});
229+
vertxRequest.sendHead().onFailure(promise::fail);
186230
} else {
187-
BinaryDataContent bodyContent = BinaryDataHelper.getContent(body);
188-
if (bodyContent instanceof ByteArrayContent
189-
|| bodyContent instanceof ByteBufferContent
190-
|| bodyContent instanceof StringContent
191-
|| bodyContent instanceof SerializableContent) {
192-
// This cannot produce a NullPointerException for the BinaryDataContent types that trigger this as they
193-
// are in-memory and have a length.
194-
long contentLength = bodyContent.getLength();
195-
vertxRequest.send(Buffer.buffer(bodyContent.toBytes()))
196-
.onSuccess(ignored -> reportProgress(contentLength, progressReporter))
197-
.onFailure(promise::fail);
198-
} else {
199-
// Right now both Flux<ByteBuffer> and InputStream bodies are being handled reactively.
200-
azureRequest.getBody()
201-
.subscribe(new VertxRequestWriteSubscriber(vertxRequest::exceptionHandler,
202-
vertxRequest::drainHandler, vertxRequest::write, vertxRequest::writeQueueFull,
203-
vertxRequest::reset, vertxRequest::end, promise, progressReporter, contextView));
231+
writeBody(contextView, azureRequest, progressReporter, vertxRequest, promise, body, false);
232+
}
233+
}
234+
235+
/*
236+
* Expect is a comma separated list of expectations and values may carry surrounding whitespace, so the header
237+
* cannot be compared as a whole.
238+
*/
239+
private static boolean expectsContinue(String expectHeader) {
240+
if (expectHeader == null) {
241+
return false;
242+
}
243+
244+
for (String expectation : expectHeader.split(",")) {
245+
if ("100-continue".equalsIgnoreCase(expectation.trim())) {
246+
return true;
204247
}
205248
}
249+
250+
return false;
251+
}
252+
253+
private void writeBody(ContextView contextView, HttpRequest azureRequest, ProgressReporter progressReporter,
254+
HttpClientRequest vertxRequest, Promise<HttpResponse> promise, BinaryData body, boolean headAlreadySent) {
255+
BinaryDataContent bodyContent = BinaryDataHelper.getContent(body);
256+
if (bodyContent instanceof ByteArrayContent
257+
|| bodyContent instanceof ByteBufferContent
258+
|| bodyContent instanceof StringContent
259+
|| bodyContent instanceof SerializableContent) {
260+
// This cannot produce a NullPointerException for the BinaryDataContent types that trigger this as they
261+
// are in-memory and have a length.
262+
long contentLength = bodyContent.getLength();
263+
Buffer buffer = Buffer.buffer(bodyContent.toBytes());
264+
// Once the head has been sent the body must be written onto the open request rather than sent as a
265+
// whole new request.
266+
Future<Void> written = headAlreadySent ? vertxRequest.end(buffer) : vertxRequest.send(buffer).mapEmpty();
267+
written.onSuccess(ignored -> reportProgress(contentLength, progressReporter)).onFailure(promise::fail);
268+
} else {
269+
// Right now both Flux<ByteBuffer> and InputStream bodies are being handled reactively.
270+
azureRequest.getBody()
271+
.subscribe(new VertxRequestWriteSubscriber(vertxRequest::exceptionHandler, vertxRequest::drainHandler,
272+
vertxRequest::write, vertxRequest::writeQueueFull, vertxRequest::reset, vertxRequest::end, promise,
273+
progressReporter, contextView));
274+
}
206275
}
207276

208277
private static void reportProgress(long progress, ProgressReporter progressReporter) {

0 commit comments

Comments
 (0)