From 954c6aa9ec5d0de2bf2f057609123dd5c1725507 Mon Sep 17 00:00:00 2001 From: Gavin Bunney Date: Fri, 19 Aug 2022 12:41:53 -0700 Subject: [PATCH] Improve uri parsing to better handle non-RFC2396 compliant host headers --- .../message/http/HttpRequestMessageImpl.java | 79 +++++++++++++--- .../http/HttpRequestMessageImplTest.java | 92 +++++++++++++++++++ 2 files changed, 158 insertions(+), 13 deletions(-) diff --git a/zuul-core/src/main/java/com/netflix/zuul/message/http/HttpRequestMessageImpl.java b/zuul-core/src/main/java/com/netflix/zuul/message/http/HttpRequestMessageImpl.java index dabd1dbaab..144b7b4168 100644 --- a/zuul-core/src/main/java/com/netflix/zuul/message/http/HttpRequestMessageImpl.java +++ b/zuul-core/src/main/java/com/netflix/zuul/message/http/HttpRequestMessageImpl.java @@ -20,6 +20,7 @@ import com.netflix.config.CachedDynamicBooleanProperty; import com.netflix.config.CachedDynamicIntProperty; import com.netflix.config.DynamicStringProperty; +import com.netflix.util.Pair; import com.netflix.zuul.context.CommonContextKeys; import com.netflix.zuul.context.SessionContext; import com.netflix.zuul.filters.ZuulFilter; @@ -53,6 +54,10 @@ public class HttpRequestMessageImpl implements HttpRequestMessage { private static final Logger LOG = LoggerFactory.getLogger(HttpRequestMessageImpl.class); + private static final CachedDynamicBooleanProperty STRICT_HOST_HEADER_VALIDATION = new CachedDynamicBooleanProperty( + "zuul.HttpRequestMessage.host.header.strict.validation", true + ); + private static final CachedDynamicIntProperty MAX_BODY_SIZE_PROP = new CachedDynamicIntProperty( "zuul.HttpRequestMessage.body.max.size", 15 * 1000 * 1024 ); @@ -493,13 +498,9 @@ static String getOriginalHost(Headers headers, String serverName) throws URISynt if (xForwardedHost != null) { return xForwardedHost; } - String host = headers.getFirst(HttpHeaderNames.HOST); - if (host != null) { - URI uri = new URI(/* scheme= */ null, host, /* path= */ null, /* query= */ null, /* fragment= */ null); - if (uri.getHost() == null) { - throw new URISyntaxException(host, "Bad host name"); - } - return uri.getHost(); + Pair host = parseHostHeader(headers); + if (host.first() != null) { + return host.first(); } return serverName; } @@ -542,14 +543,17 @@ static int getOriginalPort(SessionContext context, Headers headers, int serverPo if (portStr != null && !portStr.isEmpty()) { return Integer.parseInt(portStr); } - // Check if port was specified on a Host header. - String host = headers.getFirst(HttpHeaderNames.HOST); - if (host != null) { - URI uri = new URI(/* scheme= */ null, host, /* path= */ null, /* query= */ null, /* fragment= */ null); - if (uri.getPort() != -1) { - return uri.getPort(); + + try { + // Check if port was specified on a Host header. + Pair host = parseHostHeader(headers); + if (host.second() != -1) { + return host.second(); } + } catch (URISyntaxException e) { + LOG.debug("Invalid host header, falling back to serverPort", e); } + return serverPort; } @@ -563,6 +567,55 @@ public Optional getClientDestinationPort() { } } + /** + * Attempt to parse the Host header from the collection of headers + * and return the hostname and port components. + * + * @return Hostname and Port pair. + * Hostname may be null. Port may be -1 when no valid port is found in the host header. + */ + private static Pair parseHostHeader(Headers headers) throws URISyntaxException { + String host = headers.getFirst(HttpHeaderNames.HOST); + if (host == null) { + return new Pair<>(null, -1); + } + + try { + // attempt to use default URI parsing - this can fail when not strictly following RFC2396, + // for example, having underscores in host names will fail parsing + URI uri = new URI(/* scheme= */ null, host, /* path= */ null, /* query= */ null, /* fragment= */ null); + if (uri.getHost() != null) { + return new Pair<>(uri.getHost(), uri.getPort()); + } + } catch (URISyntaxException e) { + LOG.debug("URI parsing failed", e); + } + + if (STRICT_HOST_HEADER_VALIDATION.get()) { + throw new URISyntaxException(host, "Invalid host"); + } + + // fallback to using a colon split + // valid IPv6 addresses would have been handled already so any colon is safely assumed a port separator + String[] components = host.split(":"); + if (components.length > 2) { + // handle case with unbracketed IPv6 addresses + return new Pair<>(null, -1); + } + + String parsedHost = components[0]; + int parsedPort = -1; + if (components.length > 1) { + try { + parsedPort = Integer.parseInt(components[1]); + } catch (NumberFormatException e) { + // ignore failing to parse port numbers and fallback to default port + LOG.debug("Parsing of host port component failed", e); + } + } + return new Pair<>(parsedHost, parsedPort); + } + /** * Attempt to reconstruct the full URI that the client used. * diff --git a/zuul-core/src/test/java/com/netflix/zuul/message/http/HttpRequestMessageImplTest.java b/zuul-core/src/test/java/com/netflix/zuul/message/http/HttpRequestMessageImplTest.java index 79ed9c6471..2267366714 100644 --- a/zuul-core/src/test/java/com/netflix/zuul/message/http/HttpRequestMessageImplTest.java +++ b/zuul-core/src/test/java/com/netflix/zuul/message/http/HttpRequestMessageImplTest.java @@ -26,6 +26,7 @@ import static org.mockito.Mockito.when; import com.google.common.net.InetAddresses; +import com.netflix.config.ConfigurationManager; import com.netflix.zuul.context.CommonContextKeys; import com.netflix.zuul.context.SessionContext; import com.netflix.zuul.message.Headers; @@ -36,6 +37,9 @@ import java.net.SocketAddress; import java.net.URISyntaxException; import java.util.Optional; + +import org.apache.commons.configuration.AbstractConfiguration; +import org.junit.After; import org.junit.Assert; import org.junit.Test; import org.junit.runner.RunWith; @@ -45,6 +49,12 @@ public class HttpRequestMessageImplTest { HttpRequestMessageImpl request; + private final AbstractConfiguration config = ConfigurationManager.getConfigInstance(); + + @After + public void resetConfig() { + config.clearProperty("zuul.HttpRequestMessage.host.header.strict.validation"); + } @Test public void testOriginalRequestInfo() { @@ -260,6 +270,40 @@ public void testGetOriginalHost() { Assert.assertEquals("blah.netflix.com", request.getOriginalHost()); } + @Test + public void testGetOriginalHost_handlesNonRFC2396Hostnames() { + config.setProperty("zuul.HttpRequestMessage.host.header.strict.validation", false); + + HttpQueryParams queryParams = new HttpQueryParams(); + Headers headers = new Headers(); + headers.add("Host", "my_underscore_endpoint.netflix.com"); + request = new HttpRequestMessageImpl(new SessionContext(), "HTTP/1.1", "POST", "/some/where", queryParams, + headers, + "192.168.0.2", "https", 7002, "localhost"); + Assert.assertEquals("my_underscore_endpoint.netflix.com", request.getOriginalHost()); + + headers = new Headers(); + headers.add("Host", "my_underscore_endpoint.netflix.com:8080"); + request = new HttpRequestMessageImpl(new SessionContext(), "HTTP/1.1", "POST", "/some/where", queryParams, + headers, + "192.168.0.2", "https", 7002, "localhost"); + Assert.assertEquals("my_underscore_endpoint.netflix.com", request.getOriginalHost()); + + headers = new Headers(); + headers.add("Host", "my_underscore_endpoint^including~more-chars.netflix.com"); + request = new HttpRequestMessageImpl(new SessionContext(), "HTTP/1.1", "POST", "/some/where", queryParams, + headers, + "192.168.0.2", "https", 7002, "localhost"); + Assert.assertEquals("my_underscore_endpoint^including~more-chars.netflix.com", request.getOriginalHost()); + + headers = new Headers(); + headers.add("Host", "hostname%5Ewith-url-encoded.netflix.com"); + request = new HttpRequestMessageImpl(new SessionContext(), "HTTP/1.1", "POST", "/some/where", queryParams, + headers, + "192.168.0.2", "https", 7002, "localhost"); + Assert.assertEquals("hostname%5Ewith-url-encoded.netflix.com", request.getOriginalHost()); + } + @Test public void getOriginalHost_failsOnUnbracketedIpv6Address() { HttpQueryParams queryParams = new HttpQueryParams(); @@ -272,6 +316,20 @@ public void getOriginalHost_failsOnUnbracketedIpv6Address() { assertThrows(URISyntaxException.class, () -> HttpRequestMessageImpl.getOriginalHost(headers, "server")); } + @Test + public void getOriginalHost_fallsBackOnUnbracketedIpv6Address_WithNonStrictValidation() { + config.setProperty("zuul.HttpRequestMessage.host.header.strict.validation", false); + + HttpQueryParams queryParams = new HttpQueryParams(); + Headers headers = new Headers(); + headers.add("Host", "ba::dd"); + request = new HttpRequestMessageImpl(new SessionContext(), "HTTP/1.1", "POST", "/some/where", queryParams, + headers, + "192.168.0.2", "https", 7002, "server"); + + Assert.assertEquals("server", request.getOriginalHost()); + } + @Test public void testGetOriginalPort() { HttpQueryParams queryParams = new HttpQueryParams(); @@ -331,6 +389,40 @@ public void testGetOriginalPort() { headers, "192.168.0.2", "https", 7002, "localhost"); Assert.assertEquals(7005, request.getOriginalPort()); + + headers = new Headers(); + headers.add("Host", "host_with_underscores.netflix.com:8080"); + request = new HttpRequestMessageImpl(new SessionContext(), "HTTP/1.1", "POST", "/some/where", queryParams, + headers, + "192.168.0.2", "https", 7002, "localhost"); + Assert.assertEquals("should fallback to server port", 7002, request.getOriginalPort()); + } + + @Test + public void testGetOriginalPort_NonStrictValidation() { + config.setProperty("zuul.HttpRequestMessage.host.header.strict.validation", false); + + HttpQueryParams queryParams = new HttpQueryParams(); + Headers headers = new Headers(); + headers.add("Host", "host_with_underscores.netflix.com:8080"); + request = new HttpRequestMessageImpl(new SessionContext(), "HTTP/1.1", "POST", "/some/where", queryParams, + headers, + "192.168.0.2", "https", 7002, "localhost"); + Assert.assertEquals(8080, request.getOriginalPort()); + + headers = new Headers(); + headers.add("Host", "host-with-carrots^1.0.0.netflix.com:8080"); + request = new HttpRequestMessageImpl(new SessionContext(), "HTTP/1.1", "POST", "/some/where", queryParams, + headers, + "192.168.0.2", "https", 7002, "localhost"); + Assert.assertEquals(8080, request.getOriginalPort()); + + headers = new Headers(); + headers.add("Host", "host-with-carrots-no-port^1.0.0.netflix.com"); + request = new HttpRequestMessageImpl(new SessionContext(), "HTTP/1.1", "POST", "/some/where", queryParams, + headers, + "192.168.0.2", "https", 7002, "localhost"); + Assert.assertEquals(7002, request.getOriginalPort()); } @Test