Skip to content

Commit

Permalink
Issue #5417 - Honoring implied ports on ForwardedRequestCustomizer be…
Browse files Browse the repository at this point in the history
…tter

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
  • Loading branch information
joakime committed Oct 9, 2020
1 parent ef06184 commit 149f389
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 6 deletions.
Expand Up @@ -512,12 +512,7 @@ public void customize(Connector connector, HttpConfiguration config, Request req
{
port = requestURI.getPort();
}
if (port == MutableHostPort.IMPLIED) // is implied
{
// get Implied port (from protocol / scheme) and HttpConfiguration
int defaultPort = 80;
port = proto.equalsIgnoreCase(config.getSecureScheme()) ? getSecurePort(config) : defaultPort;
}
// if (port == MutableHostPort.IMPLIED) // is implied, no port change needed

// Update authority if different from metadata
if (!host.equalsIgnoreCase(requestURI.getHost()) ||
Expand Down
Expand Up @@ -46,8 +46,10 @@ public class ForwardedRequestCustomizerTest
private Server server;
private RequestHandler handler;
private LocalConnector connector;
private LocalConnector connectorAlt;
private LocalConnector connectorConfigured;
private ForwardedRequestCustomizer customizer;
private ForwardedRequestCustomizer customizerAlt;
private ForwardedRequestCustomizer customizerConfigured;

private static class Actual
Expand Down Expand Up @@ -82,6 +84,15 @@ public void init() throws Exception
connector = new LocalConnector(server, http);
server.addConnector(connector);

// Alternate behavior Connector
HttpConnectionFactory httpAlt = new HttpConnectionFactory();
httpAlt.setInputBufferSize(1024);
httpAlt.getHttpConfiguration().setSecurePort(8443);
customizerAlt = new ForwardedRequestCustomizer();
httpAlt.getHttpConfiguration().addCustomizer(customizerAlt);
connectorAlt = new LocalConnector(server, httpAlt);
server.addConnector(connectorAlt);

// Configured behavior Connector
http = new HttpConnectionFactory();
http.setInputBufferSize(1024);
Expand Down Expand Up @@ -783,6 +794,71 @@ public void testConfiguredBehavior(Request request, Expectations expectations) t
expectations.accept(actual);
}

public static Stream<Arguments> nonStandardPortCases()
{
return Stream.of(
// RFC7239 Tests with https.
Arguments.of(new Request("RFC7239 with https and h2")
.headers(
"GET /test/forwarded.jsp HTTP/1.1",
"Host: web.euro.de",
"Forwarded: for=192.168.2.6;host=web.euro.de;proto=https;proto-version=h2"
// Client: https://web.euro.de/test/forwarded.jsp
),
new Expectations()
.scheme("https").serverName("web.euro.de").serverPort(443)
.requestURL("https://web.euro.de/test/forwarded.jsp")
.remoteAddr("192.168.2.6").remotePort(0)
),
// RFC7239 Tests with https and proxy provided port
Arguments.of(new Request("RFC7239 with proxy provided port on https and h2")
.headers(
"GET /test/forwarded.jsp HTTP/1.1",
"Host: web.euro.de:9443",
"Forwarded: for=192.168.2.6;host=web.euro.de:9443;proto=https;proto-version=h2"
// Client: https://web.euro.de:9443/test/forwarded.jsp
),
new Expectations()
.scheme("https").serverName("web.euro.de").serverPort(9443)
.requestURL("https://web.euro.de:9443/test/forwarded.jsp")
.remoteAddr("192.168.2.6").remotePort(0)
),
// RFC7239 Tests with https, no port in Host, but proxy provided port
Arguments.of(new Request("RFC7239 with client provided host and different proxy provided port on https and h2")
.headers(
"GET /test/forwarded.jsp HTTP/1.1",
"Host: web.euro.de",
"Forwarded: for=192.168.2.6;host=new.euro.de:7443;proto=https;proto-version=h2"
// Client: https://web.euro.de/test/forwarded.jsp
// Proxy Requests: https://new.euro.de/test/forwarded.jsp
),
new Expectations()
.scheme("https").serverName("new.euro.de").serverPort(7443)
.requestURL("https://new.euro.de:7443/test/forwarded.jsp")
.remoteAddr("192.168.2.6").remotePort(0)
)
);
}

/**
* Tests against a Connector with a HttpConfiguration on non-standard ports.
* HttpConfiguration is set to securePort of 8443
*/
@ParameterizedTest(name = "{0}")
@MethodSource("nonStandardPortCases")
public void testNonStandardPortBehavior(Request request, Expectations expectations) throws Exception
{
request.configure(customizerAlt);

String rawRequest = request.getRawRequest((header) -> header);
// System.out.println(rawRequest);

HttpTester.Response response = HttpTester.parseResponse(connectorAlt.getResponse(rawRequest));
assertThat("status", response.getStatus(), is(200));

expectations.accept(actual);
}

@Test
public void testBadInput() throws Exception
{
Expand Down

0 comments on commit 149f389

Please sign in to comment.