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 0db8a4ce34..95e92100fe 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 @@ -58,12 +58,7 @@ public class HttpRequestEncoder extends HttpObjectEncoder { } } else { if (uri.lastIndexOf(SLASH, index) <= startIndex) { - int len = uri.length(); - StringBuilder sb = new StringBuilder(len + 1); - sb.append(uri, 0, index) - .append(SLASH) - .append(uri, index, len); - uri = sb.toString(); + uri = new StringBuilder(uri).insert(index, SLASH).toString(); } } } 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 4852fe48eb..08aee7b2ca 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 @@ -35,6 +35,7 @@ public class HttpRequestEncoderTest { HttpMethod.GET, "http://localhost")); String req = buffer.toString(Charset.forName("US-ASCII")); assertEquals("GET http://localhost/ HTTP/1.1\r\n", req); + buffer.release(); } @Test @@ -45,6 +46,18 @@ public class HttpRequestEncoderTest { "http://localhost:9999?p1=v1")); String req = buffer.toString(Charset.forName("US-ASCII")); assertEquals("GET http://localhost:9999/?p1=v1 HTTP/1.1\r\n", req); + buffer.release(); + } + + @Test + public void testUriWithEmptyPath() throws Exception { + HttpRequestEncoder encoder = new HttpRequestEncoder(); + ByteBuf buffer = Unpooled.buffer(64); + encoder.encodeInitialLine(buffer, new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, + "http://localhost:9999/?p1=v1")); + String req = buffer.toString(Charset.forName("US-ASCII")); + assertEquals("GET http://localhost:9999/?p1=v1 HTTP/1.1\r\n", req); + buffer.release(); } @Test @@ -55,6 +68,7 @@ public class HttpRequestEncoderTest { HttpMethod.GET, "http://localhost/")); String req = buffer.toString(Charset.forName("US-ASCII")); assertEquals("GET http://localhost/ HTTP/1.1\r\n", req); + buffer.release(); } @Test @@ -65,6 +79,7 @@ public class HttpRequestEncoderTest { HttpMethod.GET, "/")); String req = buffer.toString(Charset.forName("US-ASCII")); assertEquals("GET / HTTP/1.1\r\n", req); + buffer.release(); } @Test @@ -75,6 +90,7 @@ public class HttpRequestEncoderTest { HttpMethod.GET, "")); String req = buffer.toString(Charset.forName("US-ASCII")); assertEquals("GET / HTTP/1.1\r\n", req); + buffer.release(); } @Test @@ -85,5 +101,6 @@ public class HttpRequestEncoderTest { 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); + buffer.release(); } } diff --git a/microbench/src/main/java/io/netty/handler/codec/http/HttpRequestEncoderInsertBenchmark.java b/microbench/src/main/java/io/netty/handler/codec/http/HttpRequestEncoderInsertBenchmark.java new file mode 100644 index 0000000000..ee9c714ea1 --- /dev/null +++ b/microbench/src/main/java/io/netty/handler/codec/http/HttpRequestEncoderInsertBenchmark.java @@ -0,0 +1,117 @@ +/* + * Copyright 2017 The Netty Project + * + * The Netty Project licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ +package io.netty.handler.codec.http; + +import io.netty.buffer.ByteBuf; +import io.netty.buffer.ByteBufUtil; +import io.netty.buffer.Unpooled; +import io.netty.microbench.util.AbstractMicrobenchmark; +import io.netty.util.AsciiString; +import io.netty.util.CharsetUtil; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Warmup; + +import static io.netty.handler.codec.http.HttpConstants.SP; + +@State(Scope.Benchmark) +@Warmup(iterations = 10) +@Measurement(iterations = 20) +public class HttpRequestEncoderInsertBenchmark extends AbstractMicrobenchmark { + + private String uri = "http://localhost?eventType=CRITICAL&from=0&to=1497437160327&limit=10&offset=0"; + private OldHttpRequestEncoder encoderOld = new OldHttpRequestEncoder(); + private HttpRequestEncoder encoderNew = new HttpRequestEncoder(); + + @Benchmark + public ByteBuf oldEncoder() throws Exception { + ByteBuf buffer = Unpooled.buffer(100); + try { + encoderOld.encodeInitialLine(buffer, new DefaultHttpRequest(HttpVersion.HTTP_1_1, + HttpMethod.GET, uri)); + return buffer; + } finally { + buffer.release(); + } + } + + @Benchmark + public ByteBuf newEncoder() throws Exception { + ByteBuf buffer = Unpooled.buffer(100); + try { + encoderNew.encodeInitialLine(buffer, new DefaultHttpRequest(HttpVersion.HTTP_1_1, + HttpMethod.GET, uri)); + return buffer; + } finally { + buffer.release(); + } + } + + private class OldHttpRequestEncoder extends HttpObjectEncoder { + private static final char SLASH = '/'; + private static final char QUESTION_MARK = '?'; + + @Override + public boolean acceptOutboundMessage(Object msg) throws Exception { + return super.acceptOutboundMessage(msg) && !(msg instanceof HttpResponse); + } + + @Override + protected void encodeInitialLine(ByteBuf buf, HttpRequest request) throws Exception { + AsciiString method = request.method().asciiName(); + ByteBufUtil.copy(method, method.arrayOffset(), buf, method.length()); + buf.writeByte(SP); + + // Add / as absolute path if no is present. + // See http://tools.ietf.org/html/rfc2616#section-5.1.2 + String uri = request.uri(); + + if (uri.isEmpty()) { + uri += SLASH; + } else { + int start = uri.indexOf("://"); + if (start != -1 && uri.charAt(0) != SLASH) { + int startIndex = start + 3; + // Correctly handle query params. + // See https://github.com/netty/netty/issues/2732 + int index = uri.indexOf(QUESTION_MARK, startIndex); + if (index == -1) { + if (uri.lastIndexOf(SLASH) <= startIndex) { + uri += SLASH; + } + } else { + if (uri.lastIndexOf(SLASH, index) <= startIndex) { + int len = uri.length(); + StringBuilder sb = new StringBuilder(len + 1); + sb.append(uri, 0, index) + .append(SLASH) + .append(uri, index, len); + uri = sb.toString(); + } + } + } + } + + buf.writeBytes(uri.getBytes(CharsetUtil.UTF_8)); + + buf.writeByte(SP); + request.protocolVersion().encode(buf); + buf.writeBytes(CRLF); + } + } +} diff --git a/microbench/src/main/java/io/netty/handler/codec/http/package-info.java b/microbench/src/main/java/io/netty/handler/codec/http/package-info.java new file mode 100644 index 0000000000..3a6014bc16 --- /dev/null +++ b/microbench/src/main/java/io/netty/handler/codec/http/package-info.java @@ -0,0 +1,19 @@ +/* + * Copyright 2017 The Netty Project + * + * The Netty Project licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ +/** + * Benchmarks for {@link io.netty.handler.codec.http}. + */ +package io.netty.handler.codec.http;