From 237b5f85f743c94d9be2d26d396130bc2053f2ef Mon Sep 17 00:00:00 2001 From: Arturo Bernal Date: Sat, 19 Jul 2025 19:03:58 +0200 Subject: [PATCH] HTTPCLIENT-1482: added ExpectContinueTrigger, extended RequestConfig, and modified RequestExpectContinue so that Expect: 100-continue is emitted only when the underlying connection has already processed at least one request; default behaviour (ALWAYS) preserved, API remains binary-compatible. --- .../http/config/ExpectContinueTrigger.java | 50 ++++++++++ .../hc/client5/http/config/RequestConfig.java | 33 ++++++- .../http/protocol/RequestExpectContinue.java | 18 +++- .../protocol/TestRequestExpectContinue.java | 95 +++++++++++++++++++ 4 files changed, 189 insertions(+), 7 deletions(-) create mode 100644 httpclient5/src/main/java/org/apache/hc/client5/http/config/ExpectContinueTrigger.java diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/config/ExpectContinueTrigger.java b/httpclient5/src/main/java/org/apache/hc/client5/http/config/ExpectContinueTrigger.java new file mode 100644 index 0000000000..555bb4e69a --- /dev/null +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/config/ExpectContinueTrigger.java @@ -0,0 +1,50 @@ +/* + * ==================================================================== + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * ==================================================================== + * + * This software consists of voluntary contributions made by many + * individuals on behalf of the Apache Software Foundation. For more + * information on the Apache Software Foundation, please see + * . + * + */ +package org.apache.hc.client5.http.config; + +/** + * Enumeration of strategies that govern automatic inclusion of the + * {@code Expect: 100-continue} request header when + * {@link org.apache.hc.client5.http.config.RequestConfig#isExpectContinueEnabled() + * expect-continue support} is enabled. + * + * @since 5.6 + */ +public enum ExpectContinueTrigger { + + /** + * Always add {@code Expect: 100-continue} to every entity-enclosing request. + */ + ALWAYS, + + /** + * Add {@code Expect: 100-continue} only when the underlying + * connection has already processed at least one request (that is, when the + * socket has been taken from the connection pool and may be stale). + */ + IF_REUSED +} \ No newline at end of file diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/config/RequestConfig.java b/httpclient5/src/main/java/org/apache/hc/client5/http/config/RequestConfig.java index b18a7b122d..f13d940fe3 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/config/RequestConfig.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/config/RequestConfig.java @@ -34,6 +34,7 @@ import org.apache.hc.core5.annotation.Contract; import org.apache.hc.core5.annotation.ThreadingBehavior; import org.apache.hc.core5.http.HttpHost; +import org.apache.hc.core5.util.Args; import org.apache.hc.core5.util.TimeValue; import org.apache.hc.core5.util.Timeout; @@ -66,12 +67,15 @@ public class RequestConfig implements Cloneable { private final boolean protocolUpgradeEnabled; private final Path unixDomainSocket; + private final ExpectContinueTrigger expectContinueTrigger; + /** * Intended for CDI compatibility */ protected RequestConfig() { this(false, null, null, false, false, 0, false, null, null, - DEFAULT_CONNECTION_REQUEST_TIMEOUT, null, null, DEFAULT_CONN_KEEP_ALIVE, false, false, false, null); + DEFAULT_CONNECTION_REQUEST_TIMEOUT, null, null, DEFAULT_CONN_KEEP_ALIVE, false, false, false, null, + ExpectContinueTrigger.ALWAYS); } RequestConfig( @@ -91,7 +95,8 @@ protected RequestConfig() { final boolean contentCompressionEnabled, final boolean hardCancellationEnabled, final boolean protocolUpgradeEnabled, - final Path unixDomainSocket) { + final Path unixDomainSocket, + final ExpectContinueTrigger expectContinueTrigger) { super(); this.expectContinueEnabled = expectContinueEnabled; this.proxy = proxy; @@ -110,6 +115,7 @@ protected RequestConfig() { this.hardCancellationEnabled = hardCancellationEnabled; this.protocolUpgradeEnabled = protocolUpgradeEnabled; this.unixDomainSocket = unixDomainSocket; + this.expectContinueTrigger = expectContinueTrigger; } /** @@ -238,6 +244,10 @@ public Path getUnixDomainSocket() { return unixDomainSocket; } + public ExpectContinueTrigger getExpectContinueTrigger() { + return expectContinueTrigger; + } + @Override protected RequestConfig clone() throws CloneNotSupportedException { return (RequestConfig) super.clone(); @@ -312,6 +322,7 @@ public static class Builder { private boolean hardCancellationEnabled; private boolean protocolUpgradeEnabled; private Path unixDomainSocket; + private ExpectContinueTrigger expectContinueTrigger; Builder() { super(); @@ -322,6 +333,7 @@ public static class Builder { this.contentCompressionEnabled = true; this.hardCancellationEnabled = true; this.protocolUpgradeEnabled = true; + this.expectContinueTrigger = ExpectContinueTrigger.ALWAYS; } /** @@ -668,6 +680,20 @@ public Builder setUnixDomainSocket(final Path unixDomainSocket) { return this; } + /** + * Defines under which circumstances the client should add the + * {@code Expect: 100-continue} header to entity-enclosing requests. + * + * @param trigger expectation-continue trigger strategy + * @return this builder + * @see ExpectContinueTrigger + * @since 5.6 + */ + public Builder setExpectContinueTrigger(final ExpectContinueTrigger trigger) { + this.expectContinueTrigger = Args.notNull(trigger, "ExpectContinueTrigger"); + return this; + } + public RequestConfig build() { return new RequestConfig( expectContinueEnabled, @@ -686,7 +712,8 @@ public RequestConfig build() { contentCompressionEnabled, hardCancellationEnabled, protocolUpgradeEnabled, - unixDomainSocket); + unixDomainSocket, + expectContinueTrigger); } } diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/protocol/RequestExpectContinue.java b/httpclient5/src/main/java/org/apache/hc/client5/http/protocol/RequestExpectContinue.java index 6e2879a552..a2c744f2db 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/protocol/RequestExpectContinue.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/protocol/RequestExpectContinue.java @@ -29,9 +29,11 @@ import java.io.IOException; +import org.apache.hc.client5.http.config.ExpectContinueTrigger; import org.apache.hc.client5.http.config.RequestConfig; import org.apache.hc.core5.annotation.Contract; import org.apache.hc.core5.annotation.ThreadingBehavior; +import org.apache.hc.core5.http.EndpointDetails; import org.apache.hc.core5.http.EntityDetails; import org.apache.hc.core5.http.HeaderElements; import org.apache.hc.core5.http.HttpException; @@ -41,6 +43,7 @@ import org.apache.hc.core5.http.HttpVersion; import org.apache.hc.core5.http.ProtocolVersion; import org.apache.hc.core5.http.protocol.HttpContext; +import org.apache.hc.core5.http.protocol.HttpCoreContext; import org.apache.hc.core5.util.Args; /** @@ -68,13 +71,20 @@ public void process(final HttpRequest request, final EntityDetails entity, final if (!request.containsHeader(HttpHeaders.EXPECT)) { final HttpClientContext clientContext = HttpClientContext.cast(context); final ProtocolVersion version = request.getVersion() != null ? request.getVersion() : clientContext.getProtocolVersion(); + final RequestConfig config = clientContext.getRequestConfigOrDefault(); + if (!config.isExpectContinueEnabled()) { + return; + } + if (config.getExpectContinueTrigger() == ExpectContinueTrigger.IF_REUSED) { + final EndpointDetails details = HttpCoreContext.cast(context).getEndpointDetails(); + if (details != null && details.getRequestCount() == 0) { + return; + } + } // Do not send the expect header if request body is known to be empty if (entity != null && entity.getContentLength() != 0 && !version.lessEquals(HttpVersion.HTTP_1_0)) { - final RequestConfig config = clientContext.getRequestConfigOrDefault(); - if (config.isExpectContinueEnabled()) { - request.addHeader(HttpHeaders.EXPECT, HeaderElements.CONTINUE); - } + request.addHeader(HttpHeaders.EXPECT, HeaderElements.CONTINUE); } } } diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/protocol/TestRequestExpectContinue.java b/httpclient5/src/test/java/org/apache/hc/client5/http/protocol/TestRequestExpectContinue.java index 9a02f14e64..2d3db218cd 100644 --- a/httpclient5/src/test/java/org/apache/hc/client5/http/protocol/TestRequestExpectContinue.java +++ b/httpclient5/src/test/java/org/apache/hc/client5/http/protocol/TestRequestExpectContinue.java @@ -27,16 +27,21 @@ package org.apache.hc.client5.http.protocol; +import java.net.InetSocketAddress; import java.nio.charset.StandardCharsets; +import org.apache.hc.client5.http.config.ExpectContinueTrigger; import org.apache.hc.client5.http.config.RequestConfig; import org.apache.hc.core5.http.ClassicHttpRequest; import org.apache.hc.core5.http.Header; import org.apache.hc.core5.http.HeaderElements; +import org.apache.hc.core5.http.HttpConnectionMetrics; import org.apache.hc.core5.http.HttpHeaders; import org.apache.hc.core5.http.HttpVersion; +import org.apache.hc.core5.http.impl.BasicEndpointDetails; import org.apache.hc.core5.http.io.entity.StringEntity; import org.apache.hc.core5.http.message.BasicClassicHttpRequest; +import org.apache.hc.core5.util.Timeout; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; @@ -119,4 +124,94 @@ void testRequestExpectContinueIgnoreNonenclosingRequests() throws Exception { Assertions.assertEquals(0, request.getHeaders().length); } + + @Test + void testRequestExpectContinueIfReused() throws Exception { + final HttpClientContext context = HttpClientContext.create(); + final RequestConfig config = RequestConfig.custom() + .setExpectContinueEnabled(true) + .setExpectContinueTrigger(ExpectContinueTrigger.IF_REUSED) + .build(); + context.setRequestConfig(config); + + final HttpConnectionMetrics metrics = new HttpConnectionMetrics() { + @Override + public long getRequestCount() { + return 1; + } + + @Override + public long getResponseCount() { + return 0; + } + + @Override + public long getSentBytesCount() { + return 0; + } + + @Override + public long getReceivedBytesCount() { + return 0; + } + }; + + final BasicEndpointDetails reused = new BasicEndpointDetails( + new InetSocketAddress("localhost", 0), + new InetSocketAddress("localhost", 80), + metrics, + Timeout.ofSeconds(30)); + context.setEndpointDetails(reused); + + final ClassicHttpRequest request = new BasicClassicHttpRequest("POST", "/"); + request.setEntity(new StringEntity("data", StandardCharsets.US_ASCII)); + + new RequestExpectContinue().process(request, request.getEntity(), context); + + final Header header = request.getFirstHeader(HttpHeaders.EXPECT); + Assertions.assertNotNull(header); + Assertions.assertEquals(HeaderElements.CONTINUE, header.getValue()); + } + + @Test + void testNoExpectContinueFreshConnectionWithIfReused() throws Exception { + final HttpClientContext context = HttpClientContext.create(); + final RequestConfig cfg = RequestConfig.custom() + .setExpectContinueEnabled(true) + .setExpectContinueTrigger(ExpectContinueTrigger.IF_REUSED) + .build(); + context.setRequestConfig(cfg); + + // fresh endpoint: requestCount == 0 + context.setEndpointDetails(new BasicEndpointDetails( + new InetSocketAddress("localhost", 0), + new InetSocketAddress("localhost", 80), + null, + Timeout.ofSeconds(30))); + + final ClassicHttpRequest req = new BasicClassicHttpRequest("POST", "/"); + req.setEntity(new StringEntity("data", StandardCharsets.US_ASCII)); + + new RequestExpectContinue().process(req, req.getEntity(), context); + + Assertions.assertNull(req.getFirstHeader(HttpHeaders.EXPECT)); + } + + @Test + void testHeaderAlreadyPresentIsNotDuplicated() throws Exception { + final HttpClientContext context = HttpClientContext.create(); + final RequestConfig cfg = RequestConfig.custom() + .setExpectContinueEnabled(true) + .build(); // default trigger = ALWAYS + context.setRequestConfig(cfg); + + final ClassicHttpRequest req = new BasicClassicHttpRequest("POST", "/"); + req.setEntity(new StringEntity("data", StandardCharsets.US_ASCII)); + req.addHeader(HttpHeaders.EXPECT, HeaderElements.CONTINUE); // pre-existing + + new RequestExpectContinue().process(req, req.getEntity(), context); + + final Header[] headers = req.getHeaders(HttpHeaders.EXPECT); + Assertions.assertEquals(1, headers.length); // no duplicates + } }