Repository: netty/netty
Issue: Incorrect protocol version & "host" header in HttpProxyHandler
Labels: defect

[ netty-4.1.1.Final ]

There seem to be a couple of things in HttpProxyHandler that do not comply with [RFC 2817](http://www.faqs.org/rfcs/rfc2817.html) (Upgrading to TLS Within HTTP/1.1) 
- The HTTP version required to support the CONNECT method is 1.1,
  whereas Netty's HttpProxyHandler class specifies a 1.0 version (line 123)
- The "Host" header should apparently hold the name of the target host,
  whereas Netty specifies the proxy address instead, (lines 127-131).

Changing the original code in lines 122-131 :

```
  FullHttpRequest req = new DefaultFullHttpRequest(
          HttpVersion.HTTP_1_0, HttpMethod.CONNECT,
          rhost + ':' + raddr.getPort(),
          Unpooled.EMPTY_BUFFER, false);

  SocketAddress proxyAddress = proxyAddress();
  if (proxyAddress instanceof InetSocketAddress) {
      InetSocketAddress hostAddr = (InetSocketAddress) proxyAddress;
      req.headers().set(HttpHeaderNames.HOST, hostAddr.getHostString() + ':' + hostAddr.getPort());
  }
```

To something like this would solve both issues :

```
  final String host = rhost + ':' + raddr.getPort();

  FullHttpRequest req = new DefaultFullHttpRequest(
          HttpVersion.HTTP_1_1, HttpMethod.CONNECT,
          host,
          Unpooled.EMPTY_BUFFER, false);

  req.headers().set(HttpHeaderNames.HOST, host);
```


Issue comments:
@trustin PTAL

> Am 20.06.2016 um 16:09 schrieb Franck Chevassu notifications@github.com:
> 
> [ netty-4.1.1.Final ]
> 
> There seem to be a couple of things in HttpProxyHandler that do not comply with RFC 2817 (Upgrading to TLS Within HTTP/1.1)
> 
> The HTTP version required to support the CONNECT method is 1.1,
> whereas Netty's HttpProxyHandler class specifies a 1.0 version (line 123)
> 
> The "Host" header should apparently hold the name of the target host,
> whereas Netty specifies the proxy address instead, (lines 127-131).
> 
> Changing the original code in lines 122-131 :
> 
>   FullHttpRequest req = new DefaultFullHttpRequest(
>           HttpVersion.HTTP_1_0, HttpMethod.CONNECT,
>           rhost + ':' + raddr.getPort(),
>           Unpooled.EMPTY_BUFFER, false);
> 
>   SocketAddress proxyAddress = proxyAddress();
>   if (proxyAddress instanceof InetSocketAddress) {
>       InetSocketAddress hostAddr = (InetSocketAddress) proxyAddress;
>       req.headers().set(HttpHeaderNames.HOST, hostAddr.getHostString() + ':' + hostAddr.getPort());
>   }
> To something like this would solve both issues :
> 
>   final String host = rhost + ':' + raddr.getPort();
> 
>   FullHttpRequest req = new DefaultFullHttpRequest(
>           HttpVersion.HTTP_1_1, HttpMethod.CONNECT,
>           host,
>           Unpooled.EMPTY_BUFFER, false);
> 
>   req.headers().set(HttpHeaderNames.HOST, host);
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub, or mute the thread.

@fchevassu Care to send a PR? I don't have enough bandwidth these days.

Fixed by https://github.com/netty/netty/pull/5437


Pull request review comments:

Patch that resolved the issue:
diff --git a/handler-proxy/src/main/java/io/netty/handler/proxy/HttpProxyHandler.java b/handler-proxy/src/main/java/io/netty/handler/proxy/HttpProxyHandler.java
index 376b319c02a..53d6fbb823d 100644
--- a/handler-proxy/src/main/java/io/netty/handler/proxy/HttpProxyHandler.java
+++ b/handler-proxy/src/main/java/io/netty/handler/proxy/HttpProxyHandler.java
@@ -119,16 +119,13 @@ protected Object newInitialMessage(ChannelHandlerContext ctx) throws Exception {
             rhost = raddr.getAddress().getHostAddress();
         }
 
+        final String host = rhost + ':' + raddr.getPort();
         FullHttpRequest req = new DefaultFullHttpRequest(
-                HttpVersion.HTTP_1_0, HttpMethod.CONNECT,
-                rhost + ':' + raddr.getPort(),
+                HttpVersion.HTTP_1_1, HttpMethod.CONNECT,
+                host,
                 Unpooled.EMPTY_BUFFER, false);
 
-        SocketAddress proxyAddress = proxyAddress();
-        if (proxyAddress instanceof InetSocketAddress) {
-            InetSocketAddress hostAddr = (InetSocketAddress) proxyAddress;
-            req.headers().set(HttpHeaderNames.HOST, hostAddr.getHostString() + ':' + hostAddr.getPort());
-        }
+        req.headers().set(HttpHeaderNames.HOST, host);
 
         if (authorization != null) {
             req.headers().set(HttpHeaderNames.PROXY_AUTHORIZATION, authorization);


Test patch: