[ 
https://issues.apache.org/jira/browse/HTTPCORE-796?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18089993#comment-18089993
 ] 

Ryan Schmitt commented on HTTPCORE-796:
---------------------------------------

[~olegk], I think that's a misread of the spec. I read §7.5 as an explanation 
of the interactions between things like connection reuse, request pipelining, 
full-duplex transmission, and server-side swallow behavior. The reason for the 
client to continue sending the request is to preserve the reusability of the 
connection by completing the transmission of the request so that the server can 
demarcate the end of that request and the start of the next one. I interpret 
"an explicit indication to the contrary" to refer to things like a 
{{Connection: close}} response header, a TCP FIN, or a TCP RST, all of which 
would render the connection ineligible for subsequent reuse, thus making it 
vain for the client to continue sending the request.

Looking at §10.1.1, it seems pretty clear what the intended expect-continue 
behavior is:
{quote}Upon receiving an HTTP/1.1 (or later) request that has a method, target 
URI, and complete header section that contains a 100-continue expectation and 
an indication that request content will follow, an origin server MUST send 
either:
 * an immediate response with a final status code, if that status can be 
determined by examining just the method, target URI, and header fields, or
 * an immediate 100 (Continue) response to encourage the client to send the 
request content.{quote}
The only reason it is not a blatant violation of the spec for the client to 
send the request anyway is due to the race condition implied by the following:
{quote}A client that sends a 100-continue expectation is not required to wait 
for any specific length of time; such a client MAY proceed to send the content 
even if it has not yet received a response. Furthermore, since 100 (Continue) 
responses cannot be sent through an HTTP/1.0 intermediary, such a client SHOULD 
NOT wait for an indefinite period before sending the content.
{quote}
Nevertheless, for the client to send the request anyway after receiving a 
"final status code" clearly defeats the purpose of the expect-continue feature. 
I agree with the other commenters: this is a regression and should be fixed.

> HttpRequestExecutor sends request body on 307 redirect with Expect: 
> 100-continue, causing Broken Pipe for payloads >100KB
> -------------------------------------------------------------------------------------------------------------------------
>
>                 Key: HTTPCORE-796
>                 URL: https://issues.apache.org/jira/browse/HTTPCORE-796
>             Project: HttpComponents HttpCore
>          Issue Type: Improvement
>          Components: HttpCore
>    Affects Versions: 5.2.4, 5.3.1, 5.4.2, 5.5-alpha1
>            Reporter: Haifeng Hu
>            Priority: Critical
>
> Issue Title
> HttpRequestExecutor sends request body on 307 redirect with Expect: 
> 100-continue, causing Broken Pipe for payloads >100KB
>  
> h2. Project
> HttpComponents HttpCore (HTTPCORE)
>  
> h2. Issue Type
> Bug
>  
> h2. Affects Versions
> 5.2.4, 5.3.1, 5.4.2, 5.5-beta1 (all versions from 5.2.x through latest master)
>  
> h2. Description
> h3. Summary
> HttpRequestExecutor.execute() in httpcore5 incorrectly sends the request body 
> when it receives a *3xx redirect response* (e.g., 307 Temporary Redirect) 
> while waiting for a 100-continue interim response. This causes a *Broken 
> Pipe* error for payloads exceeding the TCP send buffer (~100KB / 70 MSS), 
> because the server has already closed the connection.
> This bug is present in *all versions from 5.2.x through 5.5-beta1* (latest 
> master as of 2026-06-17).
>  
> h3. Affected File
> httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/HttpRequestExecutor.java
>  
> h3. Root Cause
> In the execute() method, when Expect: 100-continue is set and the server 
> responds with a non-1xx status code, the code branches as follows:
>  
> if (status == HttpStatus.SC_CONTINUE) {
>     // discard 100-continue
>     response = null;
>     conn.sendRequestEntity(request);         // 100 -> send body (correct)
> } else if (status < HttpStatus.SC_SUCCESS) {
>     // other 1xx -> continue waiting
>     if (informationCallback != null) {
>         informationCallback.execute(response, conn, context);
>     }
>     response = null;
>     continue;
> } else if (status >= HttpStatus.SC_CLIENT_ERROR) {
>     conn.terminateRequest(request);           // 4xx/5xx -> terminate 
> (correct)
> } else {
>     conn.sendRequestEntity(request);          // BUG: 2xx/3xx (including 307) 
> -> sends body!
> }
>  
>  
> The else branch treats 2xx and 3xx identically: it sends the request body. 
> For 2xx this is correct (the server accepted the request). But for 3xx 
> redirects (especially 307), the server is saying "I won't accept this body, 
> go elsewhere" -- yet httpcore5 sends the body anyway.
> h3. Impact
> When a server (e.g., StarRocks FE, or any load-balancer/front-end returning 
> 307 with Expect: 100-continue) closes the connection after sending the 
> redirect:
>  # *Payload < ~100KB (fits in TCP send buffer):* sendRequestEntity() writes 
> the entire body into the local TCP buffer without blocking, returns normally. 
> The 307 response is passed up to RedirectExec, which correctly redirects to 
> the new location. *Appears to work* (but wastes bandwidth sending body to the 
> wrong server).
>  # *Payload > ~100KB (exceeds TCP send buffer):* sendRequestEntity() blocks 
> waiting for buffer space. The server has already sent FIN, then RST (because 
> it received unexpected data on a closing connection). write() throws 
> SocketException: Broken pipe. The exception propagates up through 
> HttpRequestRetryExec, which retries the request to the *original URL* (not 
> the redirect target), resulting in a second failure.
> h3.  
> h3. Comparison with HttpClient 4.x (Correct Behavior)
> In httpcore 4.x, the equivalent code correctly handles this case:
>  
> // httpcore 4.x HttpRequestExecutor.java
> if (conn.isResponseAvailable(tms)) {
>     response = conn.receiveResponseHeader();
>     int status = response.getStatusLine().getStatusCode();
>     if (status < 200) {
>         if (status != HttpStatus.SC_CONTINUE) {
>             throw new ProtocolException("Unexpected response: " + 
> response.getStatusLine());
>         }
>         response = null;  // discard 100-continue
>     } else {
>         sendentity = false;  // ALL responses >= 200 -> do NOT send body 
> (correct)
>     }
> }
>  
> HttpClient 4.x sets sendentity = false for *all* responses with status >= 
> 200, which correctly prevents sending the body on 307 redirects.
>  
> h3.  
> h3. Suggested Fix
> Replace the else branch with conn.terminateRequest(request) for all non-100 
> responses >= 200:
>  
> if (status == HttpStatus.SC_CONTINUE) {
>     response = null;
>     conn.sendRequestEntity(request);
> } else if (status < HttpStatus.SC_SUCCESS) {
>     if (informationCallback != null) {
>         informationCallback.execute(response, conn, context);
>     }
>     response = null;
>     continue;
> } else {
>     // FIX: For ALL responses >= 200 (2xx, 3xx, 4xx, 5xx),
>     // do NOT send the request body. The server has already responded
>     // and does not expect the body.
>     conn.terminateRequest(request);
> }
> This aligns with: * 
> HttpClient 4.x behavior (sendentity = false for all status >= 200)
>  * 
> RFC 7231 Section 5.1.1: a server responding to Expect: 100-continue with a 
> final status code indicates it does not need to receive the request body
>  * 
>  
> h3. Reproduction
> The bug was discovered with *Apache HttpClient 5.3.1 + httpcore5 5.2.4* 
> during StarRocks Stream Load operations, where the FE (Frontend) node returns 
> 307 Temporary Redirect to a BE (Backend) node. The issue is reproducible with:
>  * 
> Single-threaded PUT request with Expect: 100-continue header
>  * 
> Payload > 101,360 bytes (70 x 1448 MSS, exceeding the default TCP send buffer)
>  * 
> Server returns 307 + FIN immediately after sending headers
>  * 
> Result: java.net.SocketException: Broken pipe
> The same code works correctly with HttpClient 4.x.
>  
> h3. Environment
>  * 
> httpcore5 versions tested: 5.2.4, 5.4.2, 5.5-beta1 (master)
>  * 
> httpclient5 version: 5.3.1
>  * 
> JDK: 17+
>  * 
> OS: Linux (TCP sndbuf ~87KB via kernel auto-tuning)
>  * 
>  
> h3. Workaround
> Users can subclass HttpRequestExecutor and override the execute() method to 
> call conn.terminateRequest(request) for all non-100 responses >= 200, then 
> inject the custom executor into their HttpClient configuration.
>  
>  



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to