Correctly handle IPV6 in HttpProxyHandler #6195
Conversation
|
@ZuluForce PTAL as well |
normanmaurer
force-pushed
the
http_proxy
branch
from
492ca97
to
9c7d807
Compare
|
@normanmaurer LGTM Thanks for taking this on. |
| InetAddress address = addr.getAddress(); | ||
| rhost = address instanceof Inet4Address ? toAddressString(address) : '[' + toAddressString(address) + ']'; | ||
| } | ||
| return rhost + ':' + addr.getPort(); |
There was a problem hiding this comment.
I wonder if it would should be more explicit about the string concatenation operations used here. For example it seems unlikely JIT will be able to combine all of these conditional concatenation operations into a single StringBuilder. Can we create a single StringBuilder and manually do the append calls (e.g. including what is now building rhost) to be sure we don't create multiple String/StringBuilder operations?
normanmaurer
force-pushed
the
http_proxy
branch
from
9c7d807
to
e57ce81
Compare
|
|
||
| private static StringBuilder newSocketAddressStringBuilder(String hostString, String port, boolean ipv4) { | ||
| if (ipv4) { | ||
| // Need to include enought space for hostString:port . |
There was a problem hiding this comment.
also remove the space before the period?
There was a problem hiding this comment.
I added the space before to make it clear it is not part of it... But I guess its ok to remove it as well.
| // Need to include enought space for hostString:port . | ||
| return new StringBuilder(hostString.length() + 1 + port.length()).append(hostString); | ||
| } else { | ||
| // Need to include enought space for [hostString]:port . |
There was a problem hiding this comment.
enought -> enough
also remove the space before the period?
| if (ipv4) { | ||
| // Need to include enought space for hostString:port . | ||
| return new StringBuilder(hostString.length() + 1 + port.length()).append(hostString); | ||
| } else { |
Motivation: The HttpProxyHandler is expected to be capable of issuing a valid CONNECT request for a tunneled connection to an IPv6 host. Modifications: - Correctly format the IPV6 address. - Add unit tests Result: HttpProxyHandler works with IPV6 as well. Fixes [#6152].
normanmaurer
force-pushed
the
http_proxy
branch
from
e57ce81
to
82cbd35
Compare
|
Cherry-picked into 4.1 (eb5dc4b) |
Motivation:
The HttpProxyHandler is expected to be capable of issuing a valid CONNECT request for a tunneled connection to an IPv6 host.
Modifications:
Result:
HttpProxyHandler works with IPV6 as well. Fixes [#6152].