From a50137e3a2a6dbc2530bc18cdee540cf427c4247 Mon Sep 17 00:00:00 2001 From: River Date: Fri, 1 Jul 2022 23:17:02 -0700 Subject: [PATCH] Add Better Path Handling (#12533) Motivation: Paths, especially those of the form "//path/to/resource" are handled as "/path/to/resource" by most browsers and servers, but Netty fails on these, parsing them as having an authority. Modification: Directly forward the uri when it's a path instead of trying to parse it. Also clean up isOriginForm/isAsteriskForm as their logic seems to be incorrect (for example, URI#getSchemeSpecificPart() never returns null so they both return false trivially). Result: Netty forwards uris unchanged when they look to be in origin or asterisk form. --- .../codec/http2/HttpConversionUtil.java | 28 +++++++++------ .../codec/http2/HttpConversionUtilTest.java | 34 +++++++++++++++++++ 2 files changed, 52 insertions(+), 10 deletions(-) diff --git a/codec-http2/src/main/java/io/netty5/handler/codec/http2/HttpConversionUtil.java b/codec-http2/src/main/java/io/netty5/handler/codec/http2/HttpConversionUtil.java index dac3255a0cd..ad11024e12c 100644 --- a/codec-http2/src/main/java/io/netty5/handler/codec/http2/HttpConversionUtil.java +++ b/codec-http2/src/main/java/io/netty5/handler/codec/http2/HttpConversionUtil.java @@ -436,16 +436,20 @@ public static Http2Headers toHttp2Headers(HttpMessage in, boolean validateHeader final Http2Headers out = new DefaultHttp2Headers(validateHeaders, inHeaders.size()); if (in instanceof HttpRequest) { HttpRequest request = (HttpRequest) in; - URI requestTargetUri = URI.create(request.uri()); - out.path(toHttp2Path(requestTargetUri)); - out.method(request.method().asciiName()); - setHttp2Scheme(inHeaders, requestTargetUri, out); - - if (!isOriginForm(requestTargetUri) && !isAsteriskForm(requestTargetUri)) { - // Attempt to take from HOST header before taking from the request-line - String host = inHeaders.getAsString(HttpHeaderNames.HOST); - setHttp2Authority(host == null || host.isEmpty() ? requestTargetUri.getAuthority() : host, out); + String host = inHeaders.getAsString(HttpHeaderNames.HOST); + if (request.uri().startsWith("/") || "*".equals(request.uri())) { + // Origin or asterisk form + out.path(new AsciiString(request.uri())); + setHttp2Scheme(inHeaders, out); + } else { + URI requestTargetUri = URI.create(request.uri()); + out.path(toHttp2Path(requestTargetUri)); + // Take from the request-line if HOST header was empty + host = isNullOrEmpty(host) ? requestTargetUri.getAuthority() : host; + setHttp2Scheme(inHeaders, requestTargetUri, out); } + setHttp2Authority(host, out); + out.method(request.method().asciiName()); } else if (in instanceof HttpResponse) { HttpResponse response = (HttpResponse) in; out.status(response.status().codeAsText()); @@ -591,9 +595,13 @@ static void setHttp2Authority(String authority, Http2Headers out) { } } + private static void setHttp2Scheme(HttpHeaders in, Http2Headers out) { + setHttp2Scheme(in, URI.create(""), out); + } + private static void setHttp2Scheme(HttpHeaders in, URI uri, Http2Headers out) { String value = uri.getScheme(); - if (value != null) { + if (!isNullOrEmpty(value)) { out.scheme(new AsciiString(value)); return; } diff --git a/codec-http2/src/test/java/io/netty5/handler/codec/http2/HttpConversionUtilTest.java b/codec-http2/src/test/java/io/netty5/handler/codec/http2/HttpConversionUtilTest.java index 6ac8c74f27e..339760c33e8 100644 --- a/codec-http2/src/test/java/io/netty5/handler/codec/http2/HttpConversionUtilTest.java +++ b/codec-http2/src/test/java/io/netty5/handler/codec/http2/HttpConversionUtilTest.java @@ -16,6 +16,7 @@ package io.netty5.handler.codec.http2; import io.netty5.handler.codec.http.DefaultHttpHeaders; +import io.netty5.handler.codec.http.DefaultHttpRequest; import io.netty5.handler.codec.http.HttpHeaders; import io.netty5.handler.codec.http.HttpMethod; import io.netty5.handler.codec.http.HttpRequest; @@ -174,6 +175,39 @@ public void stripConnectionNomineesWithCsv() { assertSame("world", out.get("hello")); } + @Test + public void handlesRequest() throws Exception { + boolean validateHeaders = true; + HttpRequest msg = new DefaultHttpRequest( + HttpVersion.HTTP_1_1, HttpMethod.GET, "http://example.com/path/to/something", validateHeaders); + HttpHeaders inHeaders = msg.headers(); + inHeaders.add(CONNECTION, "foo, bar"); + inHeaders.add("hello", "world"); + Http2Headers out = HttpConversionUtil.toHttp2Headers(msg, validateHeaders); + assertEquals(new AsciiString("/path/to/something"), out.path()); + assertEquals(new AsciiString("http"), out.scheme()); + assertEquals(new AsciiString("example.com"), out.authority()); + assertEquals(HttpMethod.GET.asciiName(), out.method()); + assertEquals("world", out.get("hello")); + } + + @Test + public void handlesRequestWithDoubleSlashPath() throws Exception { + boolean validateHeaders = true; + HttpRequest msg = new DefaultHttpRequest( + HttpVersion.HTTP_1_1, HttpMethod.GET, "//path/to/something", validateHeaders); + HttpHeaders inHeaders = msg.headers(); + inHeaders.add(CONNECTION, "foo, bar"); + inHeaders.add(HOST, "example.com"); + inHeaders.add(HttpConversionUtil.ExtensionHeaderNames.SCHEME.text(), "http"); + inHeaders.add("hello", "world"); + Http2Headers out = HttpConversionUtil.toHttp2Headers(msg, validateHeaders); + assertEquals(new AsciiString("//path/to/something"), out.path()); + assertEquals(new AsciiString("http"), out.scheme()); + assertEquals(new AsciiString("example.com"), out.authority()); + assertEquals(HttpMethod.GET.asciiName(), out.method()); + } + @Test public void addHttp2ToHttpHeadersCombinesCookies() throws Http2Exception { Http2Headers inHeaders = new DefaultHttp2Headers();