[HTTPCLIENT-2195] [regression] Logic error in proxy connection error handling in ConnectExec.createTunnelToTarget

• Type: Bug
• Status: Resolved
• Priority: Major
• Resolution: Fixed
• Affects Version/s: 5.1.2
• Fix Version/s: 5.1.3, 5.2-beta1
• Component/s: HttpClient (classic)
• Labels: regresion
• Assignee: Unassigned
• Reporter: Henry Tung
• Created: 11/Jan/22 23:21
• Updated: 13/Jan/22 14:43
• Resolved: 13/Jan/22 14:43

Description

This moved block seems suspect:

https://github.com/apache/httpcomponents-client/commit/90d89af98e6cb9e4437d178acf084259607a20c3#diff-81d61e0ba1700fd4dcfb00bf8a8943beec35d825546d1a6b8e35fc5796902656R230

It always consumes the response and/or disconnects the connection (code apparently moved from proxy auth challenge retry handling?), but the error handling block below (code != SC_OK) attempts to read the response, triggering a non-informative StreamClosedException. The original, useful error (i.e. the response + code) is lost in the process.

I'm not clear on what the moved block does, but it seems like the response content should be saved for later use or only conditionally-consumed later (when the response is known to be successful). Maybe instead of moving the block out of proxy-auth-retry, it should be duplicated at the end of the function instead (after error handling)?

Example stacktrace from a proxy tunneling error:

Caused by: org.apache.hc.core5.http.StreamClosedException: Stream already closed
    at org.apache.hc.core5.http.impl.io.ContentLengthInputStream.read(ContentLengthInputStream.java:162)
    at sun.nio.cs.StreamDecoder.readBytes(StreamDecoder.java:284)
    at sun.nio.cs.StreamDecoder.implRead(StreamDecoder.java:326)
    at sun.nio.cs.StreamDecoder.read(StreamDecoder.java:178)
    at java.io.InputStreamReader.read(InputStreamReader.java:181)
    at java.io.Reader.read(Reader.java:229)
    at org.apache.hc.core5.http.io.entity.EntityUtils.toCharArrayBuffer(EntityUtils.java:178)
    at org.apache.hc.core5.http.io.entity.EntityUtils.toString(EntityUtils.java:221)
    at org.apache.hc.core5.http.io.entity.EntityUtils.toString(EntityUtils.java:361)
    at org.apache.hc.core5.http.io.entity.EntityUtils.toString(EntityUtils.java:341)
    at org.apache.hc.client5.http.impl.classic.ConnectExec.createTunnelToTarget(ConnectExec.java:258)
    at org.apache.hc.client5.http.impl.classic.ConnectExec.execute(ConnectExec.java:145)
    at org.apache.hc.client5.http.impl.classic.ExecChainElement.execute(ExecChainElement.java:51)
    at org.apache.hc.client5.http.impl.classic.ExecChainElement$1.proceed(ExecChainElement.java:57)
    at org.apache.hc.client5.http.impl.classic.ProtocolExec.execute(ProtocolExec.java:175)
    at org.apache.hc.client5.http.impl.classic.ExecChainElement.execute(ExecChainElement.java:51)
    at org.apache.hc.client5.http.impl.classic.ExecChainElement$1.proceed(ExecChainElement.java:57)
    at org.apache.hc.client5.http.impl.classic.RedirectExec.execute(RedirectExec.java:115)
    at org.apache.hc.client5.http.impl.classic.ExecChainElement.execute(ExecChainElement.java:51)
    at org.apache.hc.client5.http.impl.classic.InternalHttpClient.doExecute(InternalHttpClient.java:170)
    at org.apache.hc.client5.http.impl.classic.CloseableHttpClient.execute(CloseableHttpClient.java:75)
    at org.apache.hc.client5.http.impl.classic.CloseableHttpClient.execute(CloseableHttpClient.java:89)