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.

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

Fix #10306 getServerHost #10311

Merged
merged 7 commits into from
Aug 18, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
import org.eclipse.jetty.server.HttpChannel;
import org.eclipse.jetty.server.HttpConfiguration;
import org.eclipse.jetty.util.Attributes;
import org.eclipse.jetty.util.HostPort;
import org.eclipse.jetty.util.StringUtil;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -156,12 +155,6 @@ public SocketAddress getLocalSocketAddress()
return getEndPoint().getLocalSocketAddress();
}

@Override
public HostPort getServerAuthority()
{
return ConnectionMetaData.getServerAuthority(configuration, this);
}

@Override
public Object removeAttribute(String name)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@
import org.eclipse.jetty.util.Attributes;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.util.HostPort;
import org.eclipse.jetty.util.Promise;
import org.eclipse.jetty.util.StringUtil;
import org.slf4j.Logger;
Expand Down Expand Up @@ -417,12 +416,6 @@ public SocketAddress getLocalSocketAddress()
return getEndPoint().getLocalSocketAddress();
}

@Override
public HostPort getServerAuthority()
{
return ConnectionMetaData.getServerAuthority(httpConfig, this);
}

@Override
public Object getAttribute(String name)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,20 +67,21 @@ default boolean isPushSupported()

/**
* @return The URI authority that this server represents. By default, this is the address of the network socket on
* which the connection was accepted, but it may be wrapped to represent a virtual address.
* which the connection was accepted, but it may be configured to a specific address.
* @see HttpConfiguration#setServerAuthority(HostPort)
*/
HostPort getServerAuthority();

static HostPort getServerAuthority(HttpConfiguration httpConfiguration, ConnectionMetaData connectionMetaData)
default HostPort getServerAuthority()
{
HttpConfiguration httpConfiguration = getHttpConfiguration();
HostPort authority = httpConfiguration.getServerAuthority();
if (authority != null)
return authority;

SocketAddress local = connectionMetaData.getLocalSocketAddress();
if (local instanceof InetSocketAddress inet)
return new HostPort(inet.getHostString(), inet.getPort());

SocketAddress localSocketAddress = getLocalSocketAddress();
if (localSocketAddress instanceof InetSocketAddress inetSocketAddress)
return new HostPort(inetSocketAddress.getHostString(), inetSocketAddress.getPort());
else if (localSocketAddress != null)
return new HostPort(localSocketAddress.toString());
return null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,12 @@ static int getRemotePort(Request request)
return -1;
}

/**
* Get the logical name the request was sent to, which may be from the authority of the
* request; the configured server authority; the actual network name of the server;
* @param request The request to get the server name of
* @return The logical server name or null if it cannot be determined.
*/
static String getServerName(Request request)
{
if (request == null)
Expand All @@ -416,36 +422,40 @@ static String getServerName(Request request)
if (authority != null)
return HostPort.normalizeHost(authority.getHost());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to normalize again, as HostPort normalizes in the constructor.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sbordet Fixed, but will need a re-review to get green tick back


SocketAddress local = request.getConnectionMetaData().getLocalSocketAddress();
if (local instanceof InetSocketAddress)
return HostPort.normalizeHost(((InetSocketAddress)local).getHostString());

return local == null ? null : local.toString();
return null;
}

/**
* Get the logical port a request was received on, which may be from the authority of the request; the
* configured server authority; the default port for the scheme; or the actual network port.
* @param request The request to get the port of
* @return The port for the request if it can be determined, otherwise -1
*/
static int getServerPort(Request request)
{
if (request == null)
return -1;

// Does the request have an explicit port?
HttpURI uri = request.getHttpURI();
if (uri.hasAuthority() && uri.getPort() > 0)
return uri.getPort();

HostPort authority = request.getConnectionMetaData().getServerAuthority();
// Is there a configured server authority?
HostPort authority = request.getConnectionMetaData().getHttpConfiguration().getServerAuthority();
if (authority != null && authority.getPort() > 0)
return authority.getPort();

if (authority == null)
{
SocketAddress local = request.getConnectionMetaData().getLocalSocketAddress();
if (local instanceof InetSocketAddress)
return ((InetSocketAddress)local).getPort();
}

// Is there a scheme with a default port?
HttpScheme scheme = HttpScheme.CACHE.get(request.getHttpURI().getScheme());
if (scheme != null)
if (scheme != null && scheme.getDefaultPort() > 0)
return scheme.getDefaultPort();

// Is there a local port?
SocketAddress local = request.getConnectionMetaData().getLocalSocketAddress();
if (local instanceof InetSocketAddress inetSocketAddress && inetSocketAddress.getPort() > 0)
return inetSocketAddress.getPort();

return -1;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -288,15 +288,6 @@ public SocketAddress getLocalSocketAddress()
return getEndPoint().getLocalSocketAddress();
}

@Override
public HostPort getServerAuthority()
{
HostPort authority = ConnectionMetaData.getServerAuthority(getHttpConfiguration(), this);
if (authority == null)
authority = new HostPort(getLocalSocketAddress().toString(), -1);
return authority;
}

@Override
public Object removeAttribute(String name)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
package org.eclipse.jetty.server;

import java.io.IOException;
import java.net.InetSocketAddress;
import java.net.SocketAddress;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Consumer;
Expand All @@ -22,6 +24,9 @@

import org.eclipse.jetty.http.HttpCompliance;
import org.eclipse.jetty.http.HttpTester;
import org.eclipse.jetty.io.Connection;
import org.eclipse.jetty.io.EndPoint;
import org.eclipse.jetty.server.internal.HttpConnection;
import org.eclipse.jetty.util.Callback;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
Expand Down Expand Up @@ -75,7 +80,25 @@ public void init() throws Exception
server.addConnector(connector);

// Alternate behavior Connector
HttpConnectionFactory httpAlt = new HttpConnectionFactory();
HttpConnectionFactory httpAlt = new HttpConnectionFactory()
{
@Override
public Connection newConnection(Connector connector, EndPoint endPoint)
{
HttpConnection connection = new HttpConnection(getHttpConfiguration(), connector, endPoint, isRecordHttpComplianceViolations())
{
@Override
public SocketAddress getLocalSocketAddress()
{
return InetSocketAddress.createUnresolved("test", 42);
}
};
connection.setUseInputDirectByteBuffers(isUseInputDirectByteBuffers());
connection.setUseOutputDirectByteBuffers(isUseOutputDirectByteBuffers());
return configure(connection, connector, endPoint);
}
};

httpAlt.getHttpConfiguration().setSecurePort(8443);
httpAlt.getHttpConfiguration().setHttpCompliance(mismatchedAuthorityHttpCompliance);
customizerAlt = new ForwardedRequestCustomizer();
Expand Down Expand Up @@ -128,27 +151,6 @@ public void destroy() throws Exception
server.stop();
}

public static Stream<Arguments> cases2()
{
return Stream.of(
Arguments.of(new TestRequest("https initial authority, X-Forwarded-Proto on http, Proxy-Ssl-Id exists (setSslIsSecure==false)")
.configureCustomizer((customizer) -> customizer.setSslIsSecure(false))
.headers(
"GET https://alt.example.net/foo HTTP/1.1",
"Host: alt.example.net",
"X-Forwarded-Proto: http",
"Proxy-Ssl-Id: Wibble"
),
new Expectations()
.scheme("http").serverName("alt.example.net").serverPort(80)
.secure(false)
.requestURL("http://alt.example.net/foo")
.remoteAddr("0.0.0.0").remotePort(0)
.sslSession("Wibble")
)
);
}

public static Stream<Arguments> cases()
{
return Stream.of(
Expand Down Expand Up @@ -1032,7 +1034,6 @@ public void testDefaultBehavior(TestRequest request, Expectations expectations)
request.configure(customizer);

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

HttpTester.Response response = HttpTester.parseResponse(connector.getResponse(rawRequest));
assertThat("status", response.getStatus(), is(200));
Expand All @@ -1052,7 +1053,6 @@ public void testConfiguredBehavior(TestRequest request, Expectations expectation
.replaceFirst("X-Proxied-Https:", "Jetty-Proxied-Https:")
.replaceFirst("Proxy-Ssl-Id:", "Jetty-Proxy-Ssl-Id:")
.replaceFirst("Proxy-auth-cert:", "Jetty-Proxy-Auth-Cert:"));
// System.out.println(rawRequest);

HttpTester.Response response = HttpTester.parseResponse(connectorConfigured.getResponse(rawRequest));
assertThat("status", response.getStatus(), is(200));
Expand All @@ -1063,6 +1063,40 @@ public void testConfiguredBehavior(TestRequest request, Expectations expectation
public static Stream<Arguments> nonStandardPortCases()
{
return Stream.of(
// HTTP 1.0
Arguments.of(
new TestRequest("HTTP/1.0 - no Host header")
.headers(
"GET /example HTTP/1.0"
),
new Expectations()
.scheme("http").serverName("test").serverPort(42)
.secure(false)
.requestURL("http://test:42/example")
),
Arguments.of(
new TestRequest("HTTP/1.0 - Empty Host header")
.headers(
"GET scheme:///example HTTP/1.0",
"Host:"
),
new Expectations()
.scheme("scheme").serverName(null).serverPort(42)
.secure(false)
.requestURL("scheme:///example")
),
Arguments.of(
new TestRequest("HTTP/1.0 - Host header")
.headers(
"GET /example HTTP/1.0",
"Host: server:9999"
),
new Expectations()
.scheme("http").serverName("server").serverPort(9999)
.secure(false)
.requestURL("http://server:9999/example")
),

// RFC7239 Tests with https.
Arguments.of(new TestRequest("RFC7239 with https and h2")
.headers(
Expand Down Expand Up @@ -1106,7 +1140,7 @@ public static Stream<Arguments> nonStandardPortCases()

/**
* Tests against a Connector with a HttpConfiguration on non-standard ports.
* HttpConfiguration is set to securePort of 8443
* HttpConfiguration is set to securePort of 8443 and the local port is 42.
*/
@ParameterizedTest(name = "{0}")
@MethodSource("nonStandardPortCases")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1117,12 +1117,12 @@ public void test103RedirectHttp11Path() throws Exception
HttpTester.Response response = responses.get(0);
String specId = "10.3 Redirection HTTP/1.1 - basic (response 1)";
assertThat(specId, response.getStatus(), is(HttpStatus.FOUND_302));
assertEquals(getServer().getScheme() + "://localhost:" + getServer().getServerPort() + "/tests/", response.get("Location"), specId);
assertEquals(getServer().getScheme() + "://localhost/tests/", response.get("Location"), specId);

response = responses.get(1);
specId = "10.3 Redirection HTTP/1.1 - basic (response 2)";
assertThat(specId, response.getStatus(), is(HttpStatus.FOUND_302));
assertEquals(getServer().getScheme() + "://localhost:" + getServer().getServerPort() + "/tests/", response.get("Location"), specId);
assertEquals(getServer().getScheme() + "://localhost/tests/", response.get("Location"), specId);
assertEquals("close", response.get("Connection"), specId);
}

Expand All @@ -1146,7 +1146,7 @@ public void test103RedirectHttp10Resource() throws Exception

String specId = "10.3 Redirection HTTP/1.0 w/content";
assertThat(specId, response.getStatus(), is(HttpStatus.FOUND_302));
assertEquals(getServer().getScheme() + "://localhost:" + getServer().getServerPort() + "/tests/R1.txt", response.get("Location"), specId);
assertEquals(getServer().getScheme() + "://localhost/tests/R1.txt", response.get("Location"), specId);
}

/**
Expand All @@ -1169,7 +1169,7 @@ public void test103RedirectHttp11Resource() throws Exception

String specId = "10.3 Redirection HTTP/1.1 w/content";
assertThat(specId + " [status]", response.getStatus(), is(HttpStatus.FOUND_302));
assertThat(specId + " [location]", response.get("Location"), is(getServer().getScheme() + "://localhost:" + getServer().getServerPort() + "/tests/R2.txt"));
assertThat(specId + " [location]", response.get("Location"), is(getServer().getScheme() + "://localhost/tests/R2.txt"));
assertThat(specId + " [connection]", response.get("Connection"), is("close"));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1114,12 +1114,12 @@ public void test103RedirectHttp11Path() throws Exception
HttpTester.Response response = responses.get(0);
String specId = "10.3 Redirection HTTP/1.1 - basic (response 1)";
assertThat(specId, response.getStatus(), is(HttpStatus.FOUND_302));
assertEquals(getServer().getScheme() + "://localhost:" + getServer().getServerPort() + "/tests/", response.get("Location"), specId);
assertEquals(getServer().getScheme() + "://localhost/tests/", response.get("Location"), specId);

response = responses.get(1);
specId = "10.3 Redirection HTTP/1.1 - basic (response 2)";
assertThat(specId, response.getStatus(), is(HttpStatus.FOUND_302));
assertEquals(getServer().getScheme() + "://localhost:" + getServer().getServerPort() + "/tests/", response.get("Location"), specId);
assertEquals(getServer().getScheme() + "://localhost/tests/", response.get("Location"), specId);
assertEquals("close", response.get("Connection"), specId);
}

Expand All @@ -1143,7 +1143,7 @@ public void test103RedirectHttp10Resource() throws Exception

String specId = "10.3 Redirection HTTP/1.0 w/content";
assertThat(specId, response.getStatus(), is(HttpStatus.FOUND_302));
assertEquals(getServer().getScheme() + "://localhost:" + getServer().getServerPort() + "/tests/R1.txt", response.get("Location"), specId);
assertEquals(getServer().getScheme() + "://localhost/tests/R1.txt", response.get("Location"), specId);
}

/**
Expand All @@ -1166,7 +1166,7 @@ public void test103RedirectHttp11Resource() throws Exception

String specId = "10.3 Redirection HTTP/1.1 w/content";
assertThat(specId + " [status]", response.getStatus(), is(HttpStatus.FOUND_302));
assertThat(specId + " [location]", response.get("Location"), is(getServer().getScheme() + "://localhost:" + getServer().getServerPort() + "/tests/R2.txt"));
assertThat(specId + " [location]", response.get("Location"), is(getServer().getScheme() + "://localhost/tests/R2.txt"));
assertThat(specId + " [connection]", response.get("Connection"), is("close"));
}

Expand Down