diff --git a/jetty-core/jetty-fcgi/jetty-fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/internal/ServerFCGIConnection.java b/jetty-core/jetty-fcgi/jetty-fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/internal/ServerFCGIConnection.java index 17ac56fa952f..b38304c2420c 100644 --- a/jetty-core/jetty-fcgi/jetty-fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/internal/ServerFCGIConnection.java +++ b/jetty-core/jetty-fcgi/jetty-fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/internal/ServerFCGIConnection.java @@ -13,7 +13,6 @@ package org.eclipse.jetty.fcgi.server.internal; -import java.net.SocketAddress; import java.nio.ByteBuffer; import java.util.Set; import java.util.concurrent.TimeoutException; @@ -25,12 +24,11 @@ import org.eclipse.jetty.http.BadMessageException; import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpVersion; -import org.eclipse.jetty.io.AbstractConnection; import org.eclipse.jetty.io.ByteBufferPool; -import org.eclipse.jetty.io.Connection; import org.eclipse.jetty.io.Content; import org.eclipse.jetty.io.EndPoint; import org.eclipse.jetty.io.RetainableByteBuffer; +import org.eclipse.jetty.server.AbstractMetaDataConnection; import org.eclipse.jetty.server.ConnectionMetaData; import org.eclipse.jetty.server.Connector; import org.eclipse.jetty.server.HttpChannel; @@ -40,7 +38,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class ServerFCGIConnection extends AbstractConnection implements ConnectionMetaData +public class ServerFCGIConnection extends AbstractMetaDataConnection implements ConnectionMetaData { private static final Logger LOG = LoggerFactory.getLogger(ServerFCGIConnection.class); @@ -60,7 +58,7 @@ public class ServerFCGIConnection extends AbstractConnection implements Connecti public ServerFCGIConnection(Connector connector, EndPoint endPoint, HttpConfiguration configuration, boolean sendStatus200) { - super(endPoint, connector.getExecutor()); + super(connector, configuration, endPoint); this.connector = connector; this.networkByteBufferPool = connector.getByteBufferPool(); this.flusher = new Flusher(endPoint); @@ -106,12 +104,6 @@ public String getId() return id; } - @Override - public HttpConfiguration getHttpConfiguration() - { - return configuration; - } - @Override public HttpVersion getHttpVersion() { @@ -124,18 +116,6 @@ public String getProtocol() return "fcgi/1.0"; } - @Override - public Connection getConnection() - { - return this; - } - - @Override - public Connector getConnector() - { - return connector; - } - @Override public boolean isPersistent() { @@ -148,18 +128,6 @@ public boolean isSecure() return false; } - @Override - public SocketAddress getRemoteSocketAddress() - { - return getEndPoint().getRemoteSocketAddress(); - } - - @Override - public SocketAddress getLocalSocketAddress() - { - return getEndPoint().getLocalSocketAddress(); - } - @Override public Object removeAttribute(String name) { diff --git a/jetty-core/jetty-http2/jetty-http2-server/src/main/java/org/eclipse/jetty/http2/server/internal/HTTP2ServerConnection.java b/jetty-core/jetty-http2/jetty-http2-server/src/main/java/org/eclipse/jetty/http2/server/internal/HTTP2ServerConnection.java index c9b2caa67ad5..c4f978484356 100644 --- a/jetty-core/jetty-http2/jetty-http2-server/src/main/java/org/eclipse/jetty/http2/server/internal/HTTP2ServerConnection.java +++ b/jetty-core/jetty-http2/jetty-http2-server/src/main/java/org/eclipse/jetty/http2/server/internal/HTTP2ServerConnection.java @@ -66,6 +66,8 @@ public class HTTP2ServerConnection extends HTTP2Connection implements Connection private final ServerSessionListener listener; private final HttpConfiguration httpConfig; private final String id; + private final SocketAddress localSocketAddress; + private final SocketAddress remoteSocketAddress; public HTTP2ServerConnection(Connector connector, EndPoint endPoint, HttpConfiguration httpConfig, HTTP2ServerSession session, int inputBufferSize, ServerSessionListener listener) { @@ -74,6 +76,8 @@ public HTTP2ServerConnection(Connector connector, EndPoint endPoint, HttpConfigu this.listener = listener; this.httpConfig = httpConfig; this.id = StringUtil.randomAlphaNumeric(16); + localSocketAddress = httpConfig.getLocalAddress() != null ? httpConfig.getLocalAddress() : endPoint.getLocalSocketAddress(); + remoteSocketAddress = endPoint.getRemoteSocketAddress(); } @Override @@ -400,13 +404,13 @@ public boolean isPushSupported() @Override public SocketAddress getRemoteSocketAddress() { - return getEndPoint().getRemoteSocketAddress(); + return remoteSocketAddress; } @Override public SocketAddress getLocalSocketAddress() { - return getEndPoint().getLocalSocketAddress(); + return localSocketAddress; } @Override diff --git a/jetty-core/jetty-http3/jetty-http3-server/src/main/java/org/eclipse/jetty/http3/server/internal/ServerHTTP3StreamConnection.java b/jetty-core/jetty-http3/jetty-http3-server/src/main/java/org/eclipse/jetty/http3/server/internal/ServerHTTP3StreamConnection.java index 754948318ab1..5ded0ab5e897 100644 --- a/jetty-core/jetty-http3/jetty-http3-server/src/main/java/org/eclipse/jetty/http3/server/internal/ServerHTTP3StreamConnection.java +++ b/jetty-core/jetty-http3/jetty-http3-server/src/main/java/org/eclipse/jetty/http3/server/internal/ServerHTTP3StreamConnection.java @@ -33,7 +33,7 @@ import org.eclipse.jetty.util.Attributes; import org.eclipse.jetty.util.HostPort; -public class ServerHTTP3StreamConnection extends HTTP3StreamConnection implements ConnectionMetaData +public class ServerHTTP3StreamConnection extends HTTP3StreamConnection { private final HttpChannel.Factory httpChannelFactory = new HttpChannel.DefaultFactory(); private final Attributes attributes = new Attributes.Lazy(); @@ -51,7 +51,8 @@ public ServerHTTP3StreamConnection(Connector connector, HttpConfiguration httpCo public Runnable onRequest(HTTP3StreamServer stream, HeadersFrame frame) { - HttpChannel httpChannel = httpChannelFactory.newHttpChannel(this); + // Create new metadata for every request as the local or remote address may have changed. + HttpChannel httpChannel = httpChannelFactory.newHttpChannel(new MetaData()); HttpStreamOverHTTP3 httpStream = new HttpStreamOverHTTP3(this, httpChannel, stream); httpChannel.setHttpStream(httpStream); stream.setAttachment(httpStream); @@ -87,107 +88,119 @@ void offer(Runnable task) session.offer(task, false); } - @Override - public String getId() - { - return session.getQuicSession().getConnectionId().toString(); - } - - @Override - public HttpConfiguration getHttpConfiguration() - { - return httpConfiguration; - } - - @Override - public HttpVersion getHttpVersion() - { - return HttpVersion.HTTP_3; - } - - @Override - public String getProtocol() - { - return getHttpVersion().asString(); - } - - @Override - public Connection getConnection() - { - return getEndPoint().getConnection(); - } - - @Override - public Connector getConnector() - { - return connector; - } - - @Override - public boolean isPersistent() - { - return true; - } - - @Override - public boolean isSecure() - { - return true; - } - - @Override - public SocketAddress getRemoteSocketAddress() - { - return getEndPoint().getRemoteSocketAddress(); - } - - @Override - public SocketAddress getLocalSocketAddress() - { - return getEndPoint().getLocalSocketAddress(); - } - - @Override - public HostPort getServerAuthority() - { - HostPort override = httpConfiguration.getServerAuthority(); - if (override != null) - return override; - - // TODO cache the HostPort? - SocketAddress addr = getLocalSocketAddress(); - if (addr instanceof InetSocketAddress inet) - return new HostPort(inet.getHostString(), inet.getPort()); - return new HostPort(addr.toString(), -1); - } - - @Override - public Object getAttribute(String name) - { - return attributes.getAttribute(name); - } - - @Override - public Object setAttribute(String name, Object attribute) - { - return attributes.setAttribute(name, attribute); - } - - @Override - public Object removeAttribute(String name) - { - return attributes.removeAttribute(name); - } - - @Override - public Set getAttributeNameSet() - { - return attributes.getAttributeNameSet(); - } - - @Override - public void clearAttributes() - { - attributes.clearAttributes(); + private class MetaData implements ConnectionMetaData + { + private final SocketAddress localSocketAddress; + private final SocketAddress remoteSocketAddress; + + private MetaData() + { + this.localSocketAddress = httpConfiguration.getLocalAddress() == null ? getEndPoint().getLocalSocketAddress() : httpConfiguration.getLocalAddress(); + this.remoteSocketAddress = getEndPoint().getRemoteSocketAddress(); + } + + @Override + public String getId() + { + return session.getQuicSession().getConnectionId().toString(); + } + + @Override + public HttpConfiguration getHttpConfiguration() + { + return httpConfiguration; + } + + @Override + public HttpVersion getHttpVersion() + { + return HttpVersion.HTTP_3; + } + + @Override + public String getProtocol() + { + return getHttpVersion().asString(); + } + + @Override + public Connection getConnection() + { + return getEndPoint().getConnection(); + } + + @Override + public Connector getConnector() + { + return connector; + } + + @Override + public boolean isPersistent() + { + return true; + } + + @Override + public boolean isSecure() + { + return true; + } + + @Override + public SocketAddress getRemoteSocketAddress() + { + return remoteSocketAddress; + } + + @Override + public SocketAddress getLocalSocketAddress() + { + return localSocketAddress; + } + + @Override + public HostPort getServerAuthority() + { + HostPort override = httpConfiguration.getServerAuthority(); + if (override != null) + return override; + + // TODO cache the HostPort? + SocketAddress addr = getLocalSocketAddress(); + if (addr instanceof InetSocketAddress inet) + return new HostPort(inet.getHostString(), inet.getPort()); + return new HostPort(addr.toString(), -1); + } + + @Override + public Object getAttribute(String name) + { + return attributes.getAttribute(name); + } + + @Override + public Object setAttribute(String name, Object attribute) + { + return attributes.setAttribute(name, attribute); + } + + @Override + public Object removeAttribute(String name) + { + return attributes.removeAttribute(name); + } + + @Override + public Set getAttributeNameSet() + { + return attributes.getAttributeNameSet(); + } + + @Override + public void clearAttributes() + { + attributes.clearAttributes(); + } } } diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractEndPoint.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractEndPoint.java index 894ccf08344d..688663511f76 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractEndPoint.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractEndPoint.java @@ -69,10 +69,7 @@ public InetSocketAddress getLocalAddress() } @Override - public SocketAddress getLocalSocketAddress() - { - return null; - } + public abstract SocketAddress getLocalSocketAddress(); @Override public InetSocketAddress getRemoteAddress() @@ -84,10 +81,7 @@ public InetSocketAddress getRemoteAddress() } @Override - public SocketAddress getRemoteSocketAddress() - { - return null; - } + public abstract SocketAddress getRemoteSocketAddress(); protected final void shutdownInput() { diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/DatagramChannelEndPoint.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/DatagramChannelEndPoint.java index fe01bc08da07..4bdced9d9b32 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/DatagramChannelEndPoint.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/DatagramChannelEndPoint.java @@ -46,6 +46,21 @@ public DatagramChannel getChannel() return (DatagramChannel)super.getChannel(); } + @Override + public SocketAddress getRemoteSocketAddress() + { + try + { + return getChannel().getRemoteAddress(); + } + catch (Exception e) + { + if (LOG.isTraceEnabled()) + LOG.trace("ignored", e); + } + return null; + } + /** *

Receives data into the given buffer from the returned address.

*

This method should be used to receive UDP data.

diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/SelectableChannelEndPoint.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/SelectableChannelEndPoint.java index d75e0c37fd21..467fa70a1221 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/SelectableChannelEndPoint.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/SelectableChannelEndPoint.java @@ -131,9 +131,9 @@ public SocketAddress getLocalSocketAddress() try { SelectableChannel channel = getChannel(); - if (channel instanceof NetworkChannel) - return ((NetworkChannel)channel).getLocalAddress(); - return super.getLocalSocketAddress(); + if (channel instanceof NetworkChannel networkChannel) + return networkChannel.getLocalAddress(); + return null; } catch (Throwable x) { diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractMetaDataConnection.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractMetaDataConnection.java new file mode 100644 index 000000000000..e963aa48c7be --- /dev/null +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractMetaDataConnection.java @@ -0,0 +1,71 @@ +// +// ======================================================================== +// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.server; + +import java.net.SocketAddress; + +import org.eclipse.jetty.io.AbstractConnection; +import org.eclipse.jetty.io.Connection; +import org.eclipse.jetty.io.EndPoint; + +/** + * An {@link AbstractConnection} that also implements {@link ConnectionMetaData} with fixed + * local and remote addresses. + */ +public abstract class AbstractMetaDataConnection extends AbstractConnection implements ConnectionMetaData +{ + private final Connector _connector; + private final HttpConfiguration _httpConfiguration; + private final SocketAddress _localSocketAddress; + private final SocketAddress _remoteSocketAddress; + + public AbstractMetaDataConnection(Connector connector, HttpConfiguration httpConfiguration, EndPoint endPoint) + { + super(endPoint, connector.getExecutor()); + _connector = connector; + _httpConfiguration = httpConfiguration; + _localSocketAddress = httpConfiguration.getLocalAddress() != null ? httpConfiguration.getLocalAddress() : endPoint.getLocalSocketAddress(); + _remoteSocketAddress = endPoint.getRemoteSocketAddress(); + } + + @Override + public SocketAddress getRemoteSocketAddress() + { + return _remoteSocketAddress; + } + + @Override + public SocketAddress getLocalSocketAddress() + { + return _localSocketAddress; + } + + @Override + public HttpConfiguration getHttpConfiguration() + { + return _httpConfiguration; + } + + @Override + public Connection getConnection() + { + return this; + } + + @Override + public Connector getConnector() + { + return _connector; + } +} diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java index 6c82083e48a6..cd2b9b711fc0 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java @@ -435,19 +435,25 @@ public void fail(Throwable failure) }; } + static String getHostName(InetSocketAddress inetSocketAddress) + { + if (inetSocketAddress.isUnresolved()) + return inetSocketAddress.getHostString(); + + InetAddress address = inetSocketAddress.getAddress(); + String result = address == null + ? inetSocketAddress.getHostString() + : address.getHostAddress(); + return HostPort.normalizeHost(result); + } + static String getLocalAddr(Request request) { if (request == null) return null; SocketAddress local = request.getConnectionMetaData().getLocalSocketAddress(); - if (local instanceof InetSocketAddress) - { - InetAddress address = ((InetSocketAddress)local).getAddress(); - String result = address == null - ? ((InetSocketAddress)local).getHostString() - : address.getHostAddress(); - return HostPort.normalizeHost(result); - } + if (local instanceof InetSocketAddress inetSocketAddress) + return getHostName(inetSocketAddress); return local == null ? null : local.toString(); } @@ -467,16 +473,7 @@ static String getRemoteAddr(Request request) return null; SocketAddress remote = request.getConnectionMetaData().getRemoteSocketAddress(); if (remote instanceof InetSocketAddress inetSocketAddress) - { - if (inetSocketAddress.isUnresolved()) - return inetSocketAddress.getHostString(); - - InetAddress address = inetSocketAddress.getAddress(); - String result = address == null - ? inetSocketAddress.getHostString() - : address.getHostAddress(); - return HostPort.normalizeHost(result); - } + return getHostName(inetSocketAddress); return remote == null ? null : remote.toString(); } diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpConnection.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpConnection.java index 662b66c89f82..08a83cd567d1 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpConnection.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpConnection.java @@ -14,7 +14,6 @@ package org.eclipse.jetty.server.internal; import java.io.IOException; -import java.net.SocketAddress; import java.nio.ByteBuffer; import java.nio.channels.WritePendingException; import java.util.ArrayList; @@ -47,7 +46,6 @@ import org.eclipse.jetty.http.MetaData; import org.eclipse.jetty.http.Trailers; import org.eclipse.jetty.http.UriCompliance; -import org.eclipse.jetty.io.AbstractConnection; import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.io.Connection; import org.eclipse.jetty.io.Content; @@ -57,6 +55,7 @@ import org.eclipse.jetty.io.RuntimeIOException; import org.eclipse.jetty.io.WriteFlusher; import org.eclipse.jetty.io.ssl.SslConnection; +import org.eclipse.jetty.server.AbstractMetaDataConnection; import org.eclipse.jetty.server.ConnectionFactory; import org.eclipse.jetty.server.ConnectionMetaData; import org.eclipse.jetty.server.Connector; @@ -82,7 +81,7 @@ /** *

A {@link Connection} that handles the HTTP protocol.

*/ -public class HttpConnection extends AbstractConnection implements Runnable, WriteFlusher.Listener, Connection.UpgradeFrom, Connection.UpgradeTo, ConnectionMetaData +public class HttpConnection extends AbstractMetaDataConnection implements Runnable, WriteFlusher.Listener, Connection.UpgradeFrom, Connection.UpgradeTo, ConnectionMetaData { private static final Logger LOG = LoggerFactory.getLogger(HttpConnection.class); private static final HttpField PREAMBLE_UPGRADE_H2C = new HttpField(HttpHeader.UPGRADE, "h2c"); @@ -92,8 +91,6 @@ public class HttpConnection extends AbstractConnection implements Runnable, Writ private final TunnelSupport _tunnelSupport = new TunnelSupportOverHTTP1(); private final AtomicLong _streamIdGenerator = new AtomicLong(); private final long _id; - private final HttpConfiguration _configuration; - private final Connector _connector; private final HttpChannel _httpChannel; private final RequestHandler _requestHandler; private final HttpParser _parser; @@ -138,11 +135,9 @@ protected static HttpConnection setCurrentConnection(HttpConnection connection) public HttpConnection(HttpConfiguration configuration, Connector connector, EndPoint endPoint, boolean recordComplianceViolations) { - super(endPoint, connector.getExecutor()); + super(connector, configuration, endPoint); _id = __connectionIdGenerator.getAndIncrement(); - _configuration = configuration; - _connector = connector; - _bufferPool = _connector.getByteBufferPool(); + _bufferPool = connector.getByteBufferPool(); _generator = newHttpGenerator(); _httpChannel = newHttpChannel(connector.getServer(), configuration); _requestHandler = newRequestHandler(); @@ -193,12 +188,7 @@ protected RequestHandler newRequestHandler() public Server getServer() { - return _connector.getServer(); - } - - public Connector getConnector() - { - return _connector; + return getConnector().getServer(); } public HttpChannel getHttpChannel() @@ -220,7 +210,7 @@ public HttpGenerator getGenerator() public String getId() { StringBuilder builder = new StringBuilder(); - builder.append(getEndPoint().getRemoteSocketAddress()).append('@'); + builder.append(getRemoteSocketAddress()).append('@'); try { TypeUtil.toHex(hashCode(), builder); @@ -232,12 +222,6 @@ public String getId() return builder.toString(); } - @Override - public HttpConfiguration getHttpConfiguration() - { - return _configuration; - } - @Override public HttpVersion getHttpVersion() { @@ -251,37 +235,12 @@ public String getProtocol() return getHttpVersion().asString(); } - @Override - public Connection getConnection() - { - return this; - } - @Override public boolean isPersistent() { return _generator.isPersistent(getHttpVersion()); } - @Override - public SocketAddress getRemoteSocketAddress() - { - return getEndPoint().getRemoteSocketAddress(); - } - - @Override - public SocketAddress getLocalSocketAddress() - { - HttpConfiguration config = getHttpConfiguration(); - if (config != null) - { - SocketAddress override = config.getLocalAddress(); - if (override != null) - return override; - } - return getEndPoint().getLocalSocketAddress(); - } - @Override public Object removeAttribute(String name) { @@ -831,15 +790,15 @@ public Action process() throws Exception case NEED_HEADER: { - _header = _bufferPool.acquire(Math.min(_configuration.getResponseHeaderSize(), _configuration.getOutputBufferSize()), useDirectByteBuffers); + _header = _bufferPool.acquire(Math.min(getHttpConfiguration().getResponseHeaderSize(), getHttpConfiguration().getOutputBufferSize()), useDirectByteBuffers); continue; } case HEADER_OVERFLOW: { - if (_header.capacity() >= _configuration.getResponseHeaderSize()) + if (_header.capacity() >= getHttpConfiguration().getResponseHeaderSize()) throw new HttpException.RuntimeException(INTERNAL_SERVER_ERROR_500, "Response header too large"); releaseHeader(); - _header = _bufferPool.acquire(_configuration.getResponseHeaderSize(), useDirectByteBuffers); + _header = _bufferPool.acquire(getHttpConfiguration().getResponseHeaderSize(), useDirectByteBuffers); continue; } case NEED_CHUNK: @@ -850,7 +809,7 @@ public Action process() throws Exception case NEED_CHUNK_TRAILER: { releaseChunk(); - _chunk = _bufferPool.acquire(_configuration.getResponseHeaderSize(), useDirectByteBuffers); + _chunk = _bufferPool.acquire(getHttpConfiguration().getResponseHeaderSize(), useDirectByteBuffers); continue; } case FLUSH: @@ -1220,7 +1179,7 @@ public Runnable headerComplete() UriCompliance compliance; if (_uri.hasViolations()) { - compliance = _configuration.getUriCompliance(); + compliance = getHttpConfiguration().getUriCompliance(); String badMessage = UriCompliance.checkUriCompliance(compliance, _uri); if (badMessage != null) throw new BadMessageException(badMessage); diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/RequestLogTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/RequestLogTest.java index fa63273d017f..b8ed1ebd884d 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/RequestLogTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/RequestLogTest.java @@ -13,23 +13,24 @@ package org.eclipse.jetty.server; -import java.io.ByteArrayOutputStream; import java.io.InputStream; import java.io.OutputStream; import java.net.Socket; +import java.net.SocketAddress; import java.net.URI; -import java.util.List; +import java.time.Duration; +import java.util.Objects; import java.util.concurrent.TimeUnit; -import java.util.stream.Collectors; +import org.awaitility.Awaitility; import org.eclipse.jetty.http.HttpHeader; -import org.eclipse.jetty.http.MimeTypes; +import org.eclipse.jetty.http.HttpTester; import org.eclipse.jetty.io.Content; +import org.eclipse.jetty.io.EndPoint; import org.eclipse.jetty.util.BlockingArrayQueue; import org.eclipse.jetty.util.Callback; -import org.eclipse.jetty.util.IO; +import org.eclipse.jetty.util.Fields; import org.eclipse.jetty.util.component.LifeCycle; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; @@ -39,6 +40,10 @@ import static java.nio.charset.StandardCharsets.UTF_8; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; /** * Test raw behaviors of RequestLog and how Request / Response objects behave during @@ -49,7 +54,6 @@ * in the request or response objects. *

*/ -@Disabled // TODO public class RequestLogTest { private static final Logger LOG = LoggerFactory.getLogger(RequestLogTest.class); @@ -99,39 +103,27 @@ public void testNormalGetRequest() throws Exception OutputStream out = socket.getOutputStream(); InputStream in = socket.getInputStream()) { - StringBuilder req = new StringBuilder(); - req.append("GET /hello HTTP/1.1\r\n"); - req.append("Host: ").append(baseURI.getRawAuthority()).append("\r\n"); - req.append("Connection: close\r\n"); - req.append("\r\n"); - - byte[] bufRequest = req.toString().getBytes(UTF_8); - - if (LOG.isDebugEnabled()) - LOG.debug("--Request--\n" + req); - out.write(bufRequest); + String rawRequest = """ + GET /hello HTTP/1.1 + Host: %s + Connection: close + + """.formatted(baseURI.getRawAuthority()); + out.write(rawRequest.getBytes(UTF_8)); out.flush(); - ByteArrayOutputStream outBuf = new ByteArrayOutputStream(); - IO.copy(in, outBuf); - String response = outBuf.toString(UTF_8); - if (LOG.isDebugEnabled()) - LOG.debug("--Response--\n" + response); - - List responseLines = response.lines() - .map(String::trim) - .collect(Collectors.toList()); + String expectedURI = "http://%s/hello".formatted(baseURI.getRawAuthority()); + HttpTester.Response response = HttpTester.parseResponse(in); // Find status code - String responseStatusLine = responseLines.get(0); - assertThat("Status Code Response", responseStatusLine, containsString("HTTP/1.1 200")); + assertThat("Status Code Response", response.getStatus(), is(200)); // Find body content (always last line) - String bodyContent = responseLines.get(responseLines.size() - 1); - assertThat("Body Content", bodyContent, containsString("Got GET to /hello")); + String bodyContent = response.getContent(); + assertThat("Body Content", bodyContent, containsString("Got GET to " + expectedURI)); String reqlog = requestLogLines.poll(5, TimeUnit.SECONDS); - assertThat("RequestLog", reqlog, containsString("method:GET|uri:/hello|status:200")); + assertThat("RequestLog", reqlog, containsString("method:GET|uri:%s|status:200".formatted(expectedURI))); } } finally @@ -153,14 +145,21 @@ public void testNormalPostFormRequest(String requestPath) throws Exception { BlockingArrayQueue requestLogLines = new BlockingArrayQueue<>(); - // Use a Servlet API that would cause a read of the Request inputStream. - // This should result in no paramNames, as nothing is read during RequestLog execution server = createServer((request, response1) -> { - // Use a Servlet API that would cause a read of the Request inputStream. - List paramNames = List.of("TODO"); // TODO Collections.list(request.getParameterNames()); - // This should result in no paramNames, as nothing is read during RequestLog execution - requestLogLines.add(String.format("method:%s|uri:%s|paramNames.size:%d|status:%d", request.getMethod(), request.getHttpURI(), paramNames.size(), response1.getStatus())); + try + { + // Use API that would trigger a read of the request + Fields params = Request.getParameters(request); + + // This should result in only params from the query string, not from request body, as nothing is read during RequestLog execution + requestLogLines.add(String.format("method:%s|uri:%s|params.size:%d|status:%d", request.getMethod(), request.getHttpURI(), params.getSize(), response1.getStatus())); + } + catch (Exception e) + { + throw new RuntimeException(e); + } + }, new NormalResponse()); server.start(); @@ -178,45 +177,35 @@ public void testNormalPostFormRequest(String requestPath) throws Exception byte[] bufForm = form.toString().getBytes(UTF_8); - StringBuilder req = new StringBuilder(); - req.append("POST ").append(requestPath).append(" HTTP/1.1\r\n"); - req.append("Host: ").append(baseURI.getRawAuthority()).append("\r\n"); - req.append("Content-Type: ").append(MimeTypes.Type.FORM_ENCODED).append("\r\n"); - req.append("Content-Length: ").append(bufForm.length).append("\r\n"); - req.append("Connection: close\r\n"); - req.append("\r\n"); - - byte[] bufRequest = req.toString().getBytes(UTF_8); + String rawRequest = """ + POST %s HTTP/1.1 + Host: %s + Content-Type: application/x-www-form-urlencoded + Content-Length: %d + Connection: close + + """.formatted(requestPath, baseURI.getRawAuthority(), bufForm.length); - if (LOG.isDebugEnabled()) - LOG.debug("--Request--\n" + req); - out.write(bufRequest); + out.write(rawRequest.getBytes(UTF_8)); out.write(bufForm); out.flush(); - ByteArrayOutputStream outBuf = new ByteArrayOutputStream(); - IO.copy(in, outBuf); - String response = outBuf.toString(UTF_8); - if (LOG.isDebugEnabled()) - LOG.debug("--Response--\n" + response); - - List responseLines = response.lines() - .map(String::trim) - .collect(Collectors.toList()); + String expectedURI = "http://%s%s".formatted(baseURI.getRawAuthority(), requestPath); + HttpTester.Response response = HttpTester.parseResponse(in); // Find status code - String responseStatusLine = responseLines.get(0); - assertThat("Status Code Response", responseStatusLine, containsString("HTTP/1.1 200")); + assertThat("Status Code Response", response.getStatus(), is(200)); // Find body content (always last line) - String bodyContent = responseLines.get(responseLines.size() - 1); - assertThat("Body Content", bodyContent, containsString("Got POST to /hello")); + assertThat("Body Content", response.getContent(), containsString("Got POST to " + expectedURI)); String reqlog = requestLogLines.poll(5, TimeUnit.SECONDS); int querySize = 0; if (requestPath.contains("?")) querySize = 1; // assuming that parameterized version only has 1 query value - assertThat("RequestLog", reqlog, containsString("method:POST|uri:/hello|paramNames.size:" + querySize + "|status:200")); + assertThat("RequestLog", reqlog, containsString("method:POST|uri:%s|params.size:%d|status:200" + .formatted(expectedURI, querySize) + )); } } finally @@ -239,14 +228,21 @@ public void testBadPostFormRequest() throws Exception { BlockingArrayQueue requestLogLines = new BlockingArrayQueue<>(); - // Use a Servlet API that would cause a read of the Request inputStream. - // This should result in no paramNames, as nothing is read during RequestLog execution server = createServer((request, response1) -> { - // Use a Servlet API that would cause a read of the Request inputStream. - List paramNames = List.of("TODO"); // TODO Collections.list(request.getParameterNames()); - // This should result in no paramNames, as nothing is read during RequestLog execution - requestLogLines.add(String.format("method:%s|uri:%s|paramNames.size:%d|status:%d", request.getMethod(), request.getHttpURI(), paramNames.size(), response1.getStatus())); + try + { + // Use API that would trigger a read of the request + Fields params = Request.getParameters(request); + + // This should result in no params, as nothing is read during RequestLog execution + requestLogLines.add(String.format("method:%s|uri:%s|params.size:%d|status:%d", request.getMethod(), request.getHttpURI(), params.getSize(), response1.getStatus())); + } + catch (Exception e) + { + throw new RuntimeException(e); + } + }, new NormalResponse()); server.start(); @@ -264,45 +260,31 @@ public void testBadPostFormRequest() throws Exception byte[] bufForm = form.toString().getBytes(UTF_8); - StringBuilder req = new StringBuilder(); - req.append("POST /hello HTTP/1.1\r\n"); - req.append("Host: ").append(baseURI.getRawAuthority()).append("\r\n"); - req.append("Content-Type: ").append(MimeTypes.Type.FORM_ENCODED).append("\r\n"); - req.append("Content-Length: ").append(bufForm.length).append("\r\n"); - // add extra Transfer-Encoding: chunked header, making the POST request invalid per HTTP spec - req.append("Transfer-Encoding: chunked\r\n"); - req.append("Connection: close\r\n"); - req.append("\r\n"); - - byte[] bufRequest = req.toString().getBytes(UTF_8); - - if (LOG.isDebugEnabled()) - LOG.debug("--Request--\n" + req); - out.write(bufRequest); + String rawRequest = """ + POST /hello HTTP/1.1 + Host: %s + Content-Type: application/x-www-form-urlencoded + Content-Length: %d + Transfer-Encoding: chunked + Connection: close + + """.formatted(baseURI.getRawAuthority(), bufForm.length); + + out.write(rawRequest.getBytes(UTF_8)); out.write(bufForm); out.flush(); - ByteArrayOutputStream outBuf = new ByteArrayOutputStream(); - IO.copy(in, outBuf); - String response = outBuf.toString(UTF_8); - if (LOG.isDebugEnabled()) - LOG.debug("--Response--\n" + response); - - List responseLines = response.lines() - .map(String::trim) - .collect(Collectors.toList()); + HttpTester.Response response = HttpTester.parseResponse(in); // Find status code - String responseStatusLine = responseLines.get(0); - assertThat("Status Code Response", responseStatusLine, containsString("HTTP/1.1 400 Bad Request")); + assertThat("Status Code Response", response.getStatus(), is(400)); // Find body content (always last line) - String bodyContent = responseLines.get(responseLines.size() - 1); - assertThat("Body Content", bodyContent, containsString("reason: Transfer-Encoding and Content-Length")); + assertThat("Body Content", response.getContent(), containsString("Transfer-Encoding and Content-Length")); // We should see a requestlog entry for this 400 response String reqlog = requestLogLines.poll(3, TimeUnit.SECONDS); - assertThat("RequestLog", reqlog, containsString("method:POST|uri:/hello|paramNames.size:0|status:400")); + assertThat("RequestLog", reqlog, containsString("method:POST|uri:/hello|params.size:0|status:400")); } } finally @@ -315,7 +297,6 @@ public void testBadPostFormRequest() throws Exception * Test where the response is committed, then the dispatch changes the status code and response headers. * The RequestLog should see the committed status code and committed headers, not the changed ones. */ - @Disabled("Support for restoring Committed Response Headers coming in later PR") @Test public void testResponseThenChangeStatusAndHeaders() throws Exception { @@ -330,27 +311,36 @@ public void testResponseThenChangeStatusAndHeaders() throws Exception requestLogLines.add(String.format("method:%s|uri:%s|header[x-name]:%s|status:%d", request.getMethod(), request.getHttpURI(), xname, response.getStatus())); }; - /* TODO - Handler handler = new AbstractHandler() + Handler handler = new Handler.Abstract() { @Override - public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + public boolean handle(Request request, Response response, Callback callback) throws Exception { response.setStatus(202); response.getHeaders().put("X-Name", "actual"); - response.setCharacterEncoding("UTF-8"); - response.setContentType("text/plain"); - response.getWriter().printf("Got %s to %s%n", request.getMethod(), request.getHttpURI()); - response.flushBuffer(); - assertTrue(response.isCommitted(), "Response should be committed"); - baseRequest.setHandled(true); - response.setStatus(204); - response.getHeaders().put("X-Name", "post-commit"); + response.getHeaders().put(HttpHeader.CONTENT_TYPE, "text/plain; charset=utf-8"); + + String msg = "Got %s to %s%n".formatted(request.getMethod(), request.getHttpURI()); + Callback testCallback = Callback.from(callback, () -> + { + assertTrue(response.isCommitted(), "Response should be committed"); + // This shouldn't change the status for the RequestLog output + response.setStatus(204); + // attempting to set a response header after commit shouldn't be possible + UnsupportedOperationException unsupported = assertThrows(UnsupportedOperationException.class, () -> + { + response.getHeaders().put("X-Name", "post-commit"); + }); + assertThat(unsupported.getMessage(), is("Read Only")); + // finish response + response.write(true, null, callback); + }); + Content.Sink.write(response, false, msg, testCallback); + return true; } }; server = createServer(requestLog, handler); - */ server.start(); URI baseURI = server.getURI(); @@ -359,40 +349,118 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques OutputStream out = socket.getOutputStream(); InputStream in = socket.getInputStream()) { - StringBuilder req = new StringBuilder(); - req.append("GET /world HTTP/1.1\r\n"); - req.append("Host: ").append(baseURI.getRawAuthority()).append("\r\n"); - req.append("Connection: close\r\n"); - req.append("\r\n"); + String rawRequest = """ + GET /world HTTP/1.1 + Host: %s + Connection: close + + """.formatted(baseURI.getRawAuthority()); + + out.write(rawRequest.getBytes(UTF_8)); + out.flush(); - byte[] bufRequest = req.toString().getBytes(UTF_8); + String expectedURI = "http://%s/world".formatted(baseURI.getRawAuthority()); + HttpTester.Response response = HttpTester.parseResponse(in); - if (LOG.isDebugEnabled()) - LOG.debug("--Request--\n" + req); - out.write(bufRequest); - out.flush(); + // Find status code + assertThat("Status Code Response", response.getStatus(), is(202)); + + // Find body content (always last line) + assertThat("Body Content", response.getContent(), containsString("Got GET to " + expectedURI)); + + // We should see a requestlog entry for the original 202 response + String reqlog = requestLogLines.poll(3, TimeUnit.SECONDS); + assertThat("RequestLog", reqlog, containsString("method:GET|uri:%s|header[x-name]:actual|status:202".formatted(expectedURI))); + } + } + finally + { + LifeCycle.stop(server); + } + } - ByteArrayOutputStream outBuf = new ByteArrayOutputStream(); - IO.copy(in, outBuf); - String response = outBuf.toString(UTF_8); - if (LOG.isDebugEnabled()) - LOG.debug("--Response--\n" + response); + /** + * Test where the request local-address and remote-address is accessible during RequestLog. + * Requires that the EndPoint is closed before the RequestLog can execute. + */ + @Test + public void testLogRemoteAndLocalAddressesAfterClose() throws Exception + { + Server server = null; + try + { + BlockingArrayQueue requestLogLines = new BlockingArrayQueue<>(); + + RequestLog requestLog = (request, response) -> + { + SocketAddress remoteAddress = request.getConnectionMetaData().getRemoteSocketAddress(); + SocketAddress localAddress = request.getConnectionMetaData().getLocalSocketAddress(); + requestLogLines.add(String.format("method:%s|uri:%s|remote-addr:%s|local-addr:%s|status:%d", + request.getMethod(), request.getHttpURI(), remoteAddress, localAddress, response.getStatus())); + }; + + Handler handler = new Handler.Abstract() + { + @Override + public boolean handle(Request request, Response response, Callback callback) throws Exception + { + response.setStatus(202); + response.getHeaders().put("X-RemoteAddr", Objects.toString(request.getConnectionMetaData().getRemoteSocketAddress(), "")); + response.getHeaders().put("X-LocalAddr", Objects.toString(request.getConnectionMetaData().getLocalSocketAddress(), "")); + response.getHeaders().put(HttpHeader.CONTENT_TYPE, "text/plain; charset=utf-8"); + + String msg = "Got %s to %s%n".formatted(request.getMethod(), request.getHttpURI()); + Callback testCallback = Callback.from(callback, () -> + { + EndPoint endPoint = request.getConnectionMetaData().getConnection().getEndPoint(); + // Close connection + endPoint.close(); + // Wait for endpoint to be closed + Awaitility.await().atMost(Duration.ofSeconds(5)).until(() -> !endPoint.isOpen()); + }); + Content.Sink.write(response, true, msg, testCallback); + return true; + } + }; + + server = createServer(requestLog, handler); + server.start(); + + URI baseURI = server.getURI(); - List responseLines = response.lines() - .map(String::trim) - .collect(Collectors.toList()); + try (Socket socket = new Socket(baseURI.getHost(), baseURI.getPort()); + OutputStream out = socket.getOutputStream(); + InputStream in = socket.getInputStream()) + { + String rawRequest = """ + GET /world HTTP/1.1 + Host: %s + Connection: close + + """.formatted(baseURI.getRawAuthority()); + + out.write(rawRequest.getBytes(UTF_8)); + out.flush(); + + String expectedURI = "http://%s/world".formatted(baseURI.getRawAuthority()); + HttpTester.Response response = HttpTester.parseResponse(in); // Find status code - String responseStatusLine = responseLines.get(0); - assertThat("Status Code Response", responseStatusLine, containsString("HTTP/1.1 202 Accepted")); + assertThat("Status Code Response", response.getStatus(), is(202)); // Find body content (always last line) - String bodyContent = responseLines.get(responseLines.size() - 1); - assertThat("Body Content", bodyContent, containsString("Got GET to /world")); + assertThat("Body Content", response.getContent(), containsString("Got GET to " + expectedURI)); + + String remoteAddrStr = response.get("X-RemoteAddr"); + String localAddrStr = response.get("X-LocalAddr"); + + assertThat(remoteAddrStr, not(containsString(""))); + assertThat(localAddrStr, not(containsString(""))); // We should see a requestlog entry for this 400 response String reqlog = requestLogLines.poll(3, TimeUnit.SECONDS); - assertThat("RequestLog", reqlog, containsString("method:GET|uri:/world|header[x-name]:actual|status:202")); + assertThat("RequestLog", reqlog, containsString("method:GET|uri:%s|remote-addr:%s|local-addr:%s|status:202" + .formatted(expectedURI, remoteAddrStr, localAddrStr))); } } finally diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannel.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannel.java index 39736e7fd058..c8ac14515e02 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannel.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannel.java @@ -324,18 +324,9 @@ public EndPoint getEndPoint() */ public String getLocalName() { - HttpConfiguration httpConfiguration = getHttpConfiguration(); - if (httpConfiguration != null) - { - SocketAddress localAddress = httpConfiguration.getLocalAddress(); - if (localAddress instanceof InetSocketAddress) - return ((InetSocketAddress)localAddress).getHostName(); - } - InetSocketAddress local = getLocalAddress(); if (local != null) - return local.getHostString(); - + return Request.getHostName(local); return null; } @@ -358,40 +349,20 @@ public String getLocalName() */ public int getLocalPort() { - HttpConfiguration httpConfiguration = getHttpConfiguration(); - if (httpConfiguration != null) - { - SocketAddress localAddress = httpConfiguration.getLocalAddress(); - if (localAddress instanceof InetSocketAddress) - return ((InetSocketAddress)localAddress).getPort(); - } - InetSocketAddress local = getLocalAddress(); return local == null ? 0 : local.getPort(); } public InetSocketAddress getLocalAddress() { - HttpConfiguration httpConfiguration = getHttpConfiguration(); - if (httpConfiguration != null) - { - SocketAddress localAddress = httpConfiguration.getLocalAddress(); - if (localAddress instanceof InetSocketAddress) - return ((InetSocketAddress)localAddress); - } - - SocketAddress local = getEndPoint().getLocalSocketAddress(); - if (local instanceof InetSocketAddress) - return (InetSocketAddress)local; - return null; + return getRequest().getConnectionMetaData().getLocalSocketAddress() instanceof InetSocketAddress inetSocketAddress + ? inetSocketAddress : null; } public InetSocketAddress getRemoteAddress() { - SocketAddress remote = getEndPoint().getRemoteSocketAddress(); - if (remote instanceof InetSocketAddress) - return (InetSocketAddress)remote; - return null; + return getRequest().getConnectionMetaData().getRemoteSocketAddress() instanceof InetSocketAddress inetSocketAddress + ? inetSocketAddress : null; } /** diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/HttpChannel.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/HttpChannel.java index c3ff60f4e227..92665ca3b5c7 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/HttpChannel.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/HttpChannel.java @@ -380,19 +380,8 @@ public EndPoint getEndPoint() */ public String getLocalName() { - HttpConfiguration httpConfiguration = getHttpConfiguration(); - if (httpConfiguration != null) - { - SocketAddress localAddress = httpConfiguration.getLocalAddress(); - if (localAddress instanceof InetSocketAddress) - return ((InetSocketAddress)localAddress).getHostName(); - } - - InetSocketAddress local = getLocalAddress(); - if (local != null) - return local.getHostString(); - - return null; + return getConnectionMetaData().getLocalSocketAddress() instanceof InetSocketAddress inetSocketAddress + ? org.eclipse.jetty.server.Request.getHostName(inetSocketAddress) : null; } /** @@ -414,44 +403,20 @@ public String getLocalName() */ public int getLocalPort() { - HttpConfiguration httpConfiguration = getHttpConfiguration(); - if (httpConfiguration != null) - { - SocketAddress localAddress = httpConfiguration.getLocalAddress(); - if (localAddress instanceof InetSocketAddress) - return ((InetSocketAddress)localAddress).getPort(); - } - - InetSocketAddress local = getLocalAddress(); - return local == null ? 0 : local.getPort(); + return getConnectionMetaData().getLocalSocketAddress() instanceof InetSocketAddress inetSocketAddress + ? inetSocketAddress.getPort() : 0; } public InetSocketAddress getLocalAddress() { - HttpConfiguration httpConfiguration = getHttpConfiguration(); - if (httpConfiguration != null) - { - SocketAddress localAddress = httpConfiguration.getLocalAddress(); - if (localAddress instanceof InetSocketAddress inetSocketAddress) - return inetSocketAddress; - } - - SocketAddress local = getConnectionMetaData().getLocalSocketAddress(); - if (local == null) - local = _endPoint.getLocalSocketAddress(); - if (local instanceof InetSocketAddress inetSocketAddress) - return inetSocketAddress; - return null; + return getConnectionMetaData().getLocalSocketAddress() instanceof InetSocketAddress inetSocketAddress + ? inetSocketAddress : null; } public InetSocketAddress getRemoteAddress() { - SocketAddress remote = getConnectionMetaData().getRemoteSocketAddress(); - if (remote == null) - remote = _endPoint.getRemoteSocketAddress(); - if (remote instanceof InetSocketAddress inetSocketAddress) - return inetSocketAddress; - return null; + return getConnectionMetaData().getRemoteSocketAddress() instanceof InetSocketAddress inetSocketAddress + ? inetSocketAddress : null; } /**