Skip to content

Commit

Permalink
Add Better Path Handling (netty#12533)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jkjk822 authored Jul 2, 2022
1 parent f4edab3 commit 7d540fc
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,6 @@
import static io.netty.handler.codec.http.HttpResponseStatus.parseLine;
import static io.netty.handler.codec.http.HttpScheme.HTTP;
import static io.netty.handler.codec.http.HttpScheme.HTTPS;
import static io.netty.handler.codec.http.HttpUtil.isAsteriskForm;
import static io.netty.handler.codec.http.HttpUtil.isOriginForm;
import static io.netty.handler.codec.http2.Http2Error.PROTOCOL_ERROR;
import static io.netty.handler.codec.http2.Http2Exception.connectionError;
import static io.netty.handler.codec.http2.Http2Exception.streamError;
Expand Down Expand Up @@ -435,16 +433,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());
Expand Down Expand Up @@ -602,9 +604,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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package io.netty.handler.codec.http2;

import io.netty.handler.codec.http.DefaultHttpHeaders;
import io.netty.handler.codec.http.DefaultHttpRequest;
import io.netty.handler.codec.http.HttpHeaders;
import io.netty.handler.codec.http.HttpMethod;
import io.netty.handler.codec.http.HttpRequest;
Expand Down Expand Up @@ -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, "https://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();
Expand Down

0 comments on commit 7d540fc

Please sign in to comment.