Repository: netty/netty
Issue: HttpRequestEncoder erroneously appending '/' to absolute path requests that contain '://' in the path, query, or fragment

`io.netty.handler.codec.http.HttpRequestEncoder` contains the following code:

``` java
        // Add / as absolute path if no is present.
        // See http://tools.ietf.org/html/rfc2616#section-5.1.2
        String uri = request.getUri();
        int start = uri.indexOf("://");
        if (start != -1) {
            int startIndex = start + 3;
            if (uri.lastIndexOf(SLASH) <= startIndex) {
                uri += SLASH;
            }
        }
```

In the case where the Http Request line is in the absolute path format (that is, it does not contain 'http://', this will find other possible occurrences of '://' in the request path. If there are no other backslashes after this in the request line, then a trailing backslash will be appended to the request.

For example, the request:

`GET /example?uri=http://example.com&size=3`

will be transformed into:

`GET /example?uri=http://example.com&size=3/`

URLs containing `://` in the query or fragment are valid by RFC 3986, and I don't think netty should be modifying them.


Issue comments:
The following fix should work:

``` java
        String uri = request.getUri();

        if (uri.length() == 0) {
            uri += SLASH;
        } else {
            int start = uri.indexOf("://");
            if (start != -1 && uri.charAt(0) != SLASH) {
                int startIndex = start + 3;
                if (uri.lastIndexOf(SLASH) <= startIndex) {
                    uri += SLASH;
                }
            }
        }
```

 I will try to submit a pull request for this later; I don't have a working full netty build and should write tests.


Pull request review comments:

Patch that resolved the issue:
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpRequestEncoder.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpRequestEncoder.java
index c7aa051596e..3dc959bd1d1 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpRequestEncoder.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpRequestEncoder.java
@@ -41,13 +41,19 @@ protected void encodeInitialLine(ByteBuf buf, HttpRequest request) throws Except
         // Add / as absolute path if no is present.
         // See http://tools.ietf.org/html/rfc2616#section-5.1.2
         String uri = request.getUri();
-        int start = uri.indexOf("://");
-        if (start != -1) {
-            int startIndex = start + 3;
-            if (uri.lastIndexOf(SLASH) <= startIndex) {
-                uri += SLASH;
+
+        if (uri.length() == 0) {
+            uri += SLASH;
+        } else {
+            int start = uri.indexOf("://");
+            if (start != -1 && uri.charAt(0) != SLASH) {
+                int startIndex = start + 3;
+                if (uri.lastIndexOf(SLASH) <= startIndex) {
+                    uri += SLASH;
+                }
             }
         }
+
         buf.writeBytes(uri.getBytes(CharsetUtil.UTF_8));
 
         buf.writeByte(SP);


Test patch:
diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/HttpRequestEncoderTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/HttpRequestEncoderTest.java
index 3d13cf9faef..6ed9b95ff62 100644
--- a/codec-http/src/test/java/io/netty/handler/codec/http/HttpRequestEncoderTest.java
+++ b/codec-http/src/test/java/io/netty/handler/codec/http/HttpRequestEncoderTest.java
@@ -46,4 +46,34 @@ public void testUriWithPath() throws Exception {
         String req = buffer.toString(Charset.forName("US-ASCII"));
         assertEquals("GET http://localhost/ HTTP/1.1\r\n", req);
     }
+
+    @Test
+    public void testAbsPath() throws Exception {
+        HttpRequestEncoder encoder = new HttpRequestEncoder();
+        ByteBuf buffer = Unpooled.buffer(64);
+        encoder.encodeInitialLine(buffer, new DefaultHttpRequest(HttpVersion.HTTP_1_1,
+                HttpMethod.GET, "/"));
+        String req = buffer.toString(Charset.forName("US-ASCII"));
+        assertEquals("GET / HTTP/1.1\r\n", req);
+    }
+
+    @Test
+    public void testEmptyAbsPath() throws Exception {
+        HttpRequestEncoder encoder = new HttpRequestEncoder();
+        ByteBuf buffer = Unpooled.buffer(64);
+        encoder.encodeInitialLine(buffer, new DefaultHttpRequest(HttpVersion.HTTP_1_1,
+                HttpMethod.GET, ""));
+        String req = buffer.toString(Charset.forName("US-ASCII"));
+        assertEquals("GET / HTTP/1.1\r\n", req);
+    }
+
+    @Test
+    public void testQueryStringPath() throws Exception {
+        HttpRequestEncoder encoder = new HttpRequestEncoder();
+        ByteBuf buffer = Unpooled.buffer(64);
+        encoder.encodeInitialLine(buffer, new DefaultHttpRequest(HttpVersion.HTTP_1_1,
+                HttpMethod.GET, "/?url=http://example.com"));
+        String req = buffer.toString(Charset.forName("US-ASCII"));
+        assertEquals("GET /?url=http://example.com HTTP/1.1\r\n", req);
+    }
 }