Skip to content

Commit

Permalink
Issue #325 - fixes for HttpServletRequest getServerName
Browse files Browse the repository at this point in the history 
Signed-off-by: Lachlan Roberts <[email protected]>
  • Loading branch information
@lachlan-roberts
lachlan-roberts committed Dec 20, 2024
1 parent 8409da8 commit b3de558
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 21 deletions.
7 changes: 4 additions & 3 deletions ...e_impl_jetty9/src/main/java/com/google/apphosting/runtime/jetty9/JettyRequestAPIData.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
import static com.google.apphosting.runtime.AppEngineConstants.X_FORWARDED_PROTO;
import static com.google.apphosting.runtime.AppEngineConstants.X_GOOGLE_INTERNAL_PROFILER;
import static com.google.apphosting.runtime.AppEngineConstants.X_GOOGLE_INTERNAL_SKIPADMINCHECK;
import static com.google.apphosting.runtime.jetty9.RpcConnection.NORMALIZE_INET_ADDR;

import com.google.apphosting.base.protos.HttpPb;
import com.google.apphosting.base.protos.RuntimePb;
Expand Down Expand Up @@ -286,7 +287,7 @@ public JettyRequestAPIData(
traceContext = TraceContextProto.getDefaultInstance();
}

String normalizeUserIp = HostPort.normalizeHost(userIp);
String finalUserIp = NORMALIZE_INET_ADDR ? HostPort.normalizeHost(userIp) : userIp;
this.httpServletRequest =
new HttpServletRequestWrapper(httpServletRequest) {

Expand Down Expand Up @@ -332,12 +333,12 @@ public boolean isSecure() {

@Override
public String getRemoteAddr() {
return normalizeUserIp;
return finalUserIp;
}

@Override
public String getRemoteHost() {
return normalizeUserIp;
return finalUserIp;
}

@Override
Expand Down
67 changes: 49 additions & 18 deletions runtime/test/src/test/java/com/google/apphosting/runtime/jetty9/RemoteAddressTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.util.Collection;
import org.eclipse.jetty.client.HttpClient;
import org.eclipse.jetty.client.api.ContentResponse;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.http.HttpVersion;
import org.junit.After;
Expand Down Expand Up @@ -100,24 +101,26 @@ public void testWithHostHeader() throws Exception {

@Test
public void testWithIPv6() throws Exception {
// Test the host header to be IPv6 with a port.
ContentResponse response = httpClient.newRequest(url)
.header("Host", "[2001:db8:85a3:8d3:1319:8a2e:370:7348]:1234")
.header("X-AppEngine-User-IP", "203.0.113.1")
.send();
assertThat(response.getStatus(), equalTo(HttpStatus.OK_200));
String contentReceived = response.getContentAsString();
assertThat(contentReceived, containsString("getRemoteAddr: 203.0.113.1"));
assertThat(contentReceived, containsString("getRemoteHost: 203.0.113.1"));
assertThat(contentReceived, containsString("getRemotePort: 0"));
assertThat(contentReceived, containsString("getLocalAddr: 0.0.0.0"));
assertThat(contentReceived, containsString("getLocalName: 0.0.0.0"));
assertThat(contentReceived, containsString("getLocalPort: 0"));
assertThat(contentReceived, containsString("getServerName: [2001:db8:85a3:8d3:1319:8a2e:370:7348]"));
assertThat(contentReceived, containsString("getServerPort: 1234"));
// Test the host header to be IPv6 with a port.
ContentResponse response = httpClient.newRequest(url)
.header("Host", "[2001:db8:85a3:8d3:1319:8a2e:370:7348]:1234")
.header("X-AppEngine-User-IP", "203.0.113.1")
.send();
assertThat(response.getStatus(), equalTo(HttpStatus.OK_200));
String contentReceived = response.getContentAsString();
assertThat(contentReceived, containsString("getRemoteAddr: 203.0.113.1"));
assertThat(contentReceived, containsString("getRemoteHost: 203.0.113.1"));
assertThat(contentReceived, containsString("getRemotePort: 0"));
assertThat(contentReceived, containsString("getLocalAddr: 0.0.0.0"));
assertThat(contentReceived, containsString("getLocalName: 0.0.0.0"));
assertThat(contentReceived, containsString("getLocalPort: 0"));
assertThat(contentReceived, containsString("getServerName: [2001:db8:85a3:8d3:1319:8a2e:370:7348]"));
assertThat(contentReceived, containsString("getServerPort: 1234"));

// Test the user IP to be IPv6 with a port.
response = httpClient.newRequest(url)
response =
httpClient
.newRequest(url)
.header("Host", "203.0.113.1:1234")
.header("X-AppEngine-User-IP", "2001:db8:85a3:8d3:1319:8a2e:370:7348")
.send();
Expand All @@ -126,8 +129,9 @@ public void testWithIPv6() throws Exception {
if ("jetty94".equals(environment)) {
assertThat(contentReceived, containsString("getRemoteAddr: [2001:db8:85a3:8d3:1319:8a2e:370:7348]"));
assertThat(contentReceived, containsString("getRemoteHost: [2001:db8:85a3:8d3:1319:8a2e:370:7348]"));
}
else {
} else {
// The correct behaviour for getRemoteAddr and getRemoteHost is to not include []
// because they return raw IP/hostname and not URI-formatted addresses.
assertThat(contentReceived, containsString("getRemoteAddr: 2001:db8:85a3:8d3:1319:8a2e:370:7348"));
assertThat(contentReceived, containsString("getRemoteHost: 2001:db8:85a3:8d3:1319:8a2e:370:7348"));
}
Expand Down Expand Up @@ -161,6 +165,33 @@ public void testWithoutHostHeader() throws Exception {
assertThat(contentReceived, containsString("getServerPort: " + runtime.getPort()));
}

@Test
public void testForwardedHeadersIgnored() throws Exception {
ContentResponse response =
httpClient
.newRequest(url)
.header("Host", "foobar:1234")
.header("X-AppEngine-User-IP", "203.0.113.1")
.header(HttpHeader.X_FORWARDED_FOR, "test1:2221")
.header(HttpHeader.X_FORWARDED_PROTO, "test2:2222")
.header(HttpHeader.X_FORWARDED_HOST, "test3:2223")
.header(HttpHeader.X_FORWARDED_PORT, "test4:2224")
.header(HttpHeader.FORWARDED, "test5:2225")
.send();

assertThat(response.getStatus(), equalTo(HttpStatus.OK_200));
String contentReceived = response.getContentAsString();
assertThat(contentReceived, containsString("getRemoteAddr: 203.0.113.1"));
assertThat(contentReceived, containsString("getRemoteHost: 203.0.113.1"));
assertThat(contentReceived, containsString("getRemotePort: 0"));
assertThat(contentReceived, containsString("getLocalAddr: 0.0.0.0"));
assertThat(contentReceived, containsString("getLocalName: 0.0.0.0"));
assertThat(contentReceived, containsString("getLocalPort: 0"));
assertThat(contentReceived, containsString("getServerName: foobar"));
assertThat(contentReceived, containsString("getServerPort: 1234"));
}


private RuntimeContext<?> runtimeContext() throws Exception {
RuntimeContext.Config<?> config =
RuntimeContext.Config.builder().setApplicationPath(temp.getRoot().toString()).build();
Expand Down

0 comments on commit b3de558

Please sign in to comment.