Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better support for implied ports on ForwardedRequestCustomizer #5419

Merged
merged 2 commits into from
Oct 13, 2020
Merged

Better support for implied ports on ForwardedRequestCustomizer #5419

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
149f389
Issue #5417 - Honoring implied ports on ForwardedRequestCustomizer be…
joakime Oct 9, 2020
c2266fd
Issue #5417 - Cleanup of PR from Review
joakime Oct 12, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions jetty-server/src/main/java/org/eclipse/jetty/server/ForwardedRequestCustomizer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
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;
}
// Don't change port if port == IMPLIED.

// Update authority if different from metadata
if (!host.equalsIgnoreCase(requestURI.getHost()) ||
Expand Down
81 changes: 73 additions & 8 deletions jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
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 All @@ -72,22 +74,22 @@ public void init() throws Exception

// Default behavior Connector
HttpConnectionFactory http = new HttpConnectionFactory();
http.setInputBufferSize(1024);
http.getHttpConfiguration().setRequestHeaderSize(512);
http.getHttpConfiguration().setResponseHeaderSize(512);
http.getHttpConfiguration().setOutputBufferSize(2048);
http.getHttpConfiguration().setSecurePort(443);
customizer = new ForwardedRequestCustomizer();
http.getHttpConfiguration().addCustomizer(customizer);
connector = new LocalConnector(server, http);
server.addConnector(connector);

// Alternate behavior Connector
HttpConnectionFactory httpAlt = new HttpConnectionFactory();
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);
http.getHttpConfiguration().setRequestHeaderSize(512);
http.getHttpConfiguration().setResponseHeaderSize(512);
http.getHttpConfiguration().setOutputBufferSize(2048);
customizerConfigured = new ForwardedRequestCustomizer();
customizerConfigured.setForwardedHeader("Jetty-Forwarded");
customizerConfigured.setForwardedHostHeader("Jetty-Forwarded-Host");
Expand Down Expand Up @@ -783,6 +785,69 @@ 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.example.net",
"Forwarded: for=192.168.2.6;host=web.example.net;proto=https;proto-version=h2"
),
new Expectations()
.scheme("https").serverName("web.example.net").serverPort(443)
.requestURL("https://web.example.net/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.example.net:9443",
"Forwarded: for=192.168.2.6;host=web.example.net:9443;proto=https;proto-version=h2"
),
new Expectations()
.scheme("https").serverName("web.example.net").serverPort(9443)
.requestURL("https://web.example.net: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.example.net",
"Forwarded: for=192.168.2.6;host=new.example.net:7443;proto=https;proto-version=h2"
// Client: https://web.example.net/test/forwarded.jsp
// Proxy Requests: https://new.example.net/test/forwarded.jsp
),
new Expectations()
.scheme("https").serverName("new.example.net").serverPort(7443)
.requestURL("https://new.example.net: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