Only query the remote address for incoming HTTP connections at the beginning.

Fixes possible errors when attempting to query the address on an already closed connection and avoids some computational overhead.

Author: s-ludwig

source/vibe/http/internal/http1/server.d

@@ -36,7 +36,7 @@ import std.format : format, formattedWrite;
 		context = Information about the incoming listener and available
 			virtual hosts
 */
-void handleHTTP1Connection(TLSStreamType)(TCPConnection connection, TLSStreamType tls_stream, StreamProxy http_stream, HTTPServerContext context)
+void handleHTTP1Connection(TLSStreamType)(TCPConnection connection, TLSStreamType tls_stream, StreamProxy http_stream, HTTPServerContext context, ref NetworkAddress remote_address)
 @safe {
 
 	while (!connection.empty) {
@@ -52,7 +52,7 @@ void handleHTTP1Connection(TLSStreamType)(TCPConnection connection, TLSStreamTyp
 			scope request_allocator = createRequestAllocator();
 			scope (exit) freeRequestAllocator(request_allocator);
 
-			handleRequest!TLSStreamType(http_stream, connection, context, settings, keep_alive, request_allocator);
+			handleRequest!TLSStreamType(http_stream, connection, context, settings, keep_alive, request_allocator, remote_address);
 		} ();
 		if (!keep_alive) { logTrace("No keep-alive - disconnecting client."); break; }
 
@@ -67,7 +67,7 @@ void handleHTTP1Connection(TLSStreamType)(TCPConnection connection, TLSStreamTyp
 }
 
 
-private bool handleRequest(TLSStreamType, Allocator)(StreamProxy http_stream, TCPConnection tcp_connection, HTTPServerContext listen_info, ref HTTPServerSettings settings, ref bool keep_alive, scope Allocator request_allocator)
+private bool handleRequest(TLSStreamType, Allocator)(StreamProxy http_stream, TCPConnection tcp_connection, HTTPServerContext listen_info, ref HTTPServerSettings settings, ref bool keep_alive, scope Allocator request_allocator, ref NetworkAddress remote_address)
 @safe {
 	import vibe.container.internal.utilallocator : make, dispose;
 	import vibe.http.internal.utils : formatRFC822DateAlloc;
@@ -85,7 +85,7 @@ private bool handleRequest(TLSStreamType, Allocator)(StreamProxy http_stream, TC
 	FreeListRef!ChunkedInputStream chunked_input_stream;
 
 	// store the IP address
-	req.clientAddress = tcp_connection.remoteAddress;
+	req.clientAddress = remote_address;
 
 	if (!listen_info.hasVirtualHosts) {
 		logWarn("Didn't find a HTTP listening context for incoming connection. Dropping.");
@@ -164,7 +164,7 @@ private bool handleRequest(TLSStreamType, Allocator)(StreamProxy http_stream, TC
 
 			http_stream.read(dummy); // finish reading connection preface
 			auto h2settings = HTTP2Settings();
-			auto h2context = new HTTP2ServerContext(listen_info, h2settings);
+			auto h2context = new HTTP2ServerContext(listen_info, h2settings, remote_address);
 			handleHTTP2Connection(tcp_connection, tcp_connection, h2context, true);
 			return true;
 		}

source/vibe/http/internal/http2/exchange.d

@@ -124,7 +124,8 @@ unittest {
 	import std.experimental.allocator.mallocator;
 	HTTP2Settings settings;
 	HTTPServerContext ctx;
-	auto context = new HTTP2ServerContext(ctx, settings);
+	NetworkAddress raddr;
+	auto context = new HTTP2ServerContext(ctx, settings, raddr);
 	auto table = IndexingTable(settings.headerTableSize);
 	scope alloc = new RegionListAllocator!(shared(Mallocator), false)(1024, Mallocator.instance);
 
@@ -167,7 +168,7 @@ bool handleHTTP2Request(UStream)(ref HTTP2ConnectionStream!UStream stream,
 	auto req = () @trusted { return alloc.make!HTTPServerRequest(reqtime, listen_info.bindPort); } ();
 	scope (exit) () @trusted { alloc.dispose(req); } ();
 	// store the IP address
-	req.clientAddress = tcp_connection.remoteAddress;
+	req.clientAddress = h2context.remoteAddress;
 
 	if (!listen_info.hasVirtualHosts) {
 		logWarn("Didn't find a HTTP listening context for incoming connection. Dropping.");

source/vibe/http/internal/http2/settings.d

@@ -313,25 +313,28 @@ final class HTTP2ServerContext
 		FreeListRef!HTTP2Multiplexer m_multiplexer;
 		bool m_initializedT = false;
 		bool m_initializedM = false;
-
+		NetworkAddress m_remoteAddress;
 	}
 
 	alias m_context this;
 
 	// used to mantain the first request in case of `h2c` protocol switching
 	ubyte[] resFrame = void;
 
-	this(HTTPServerContext ctx, HTTP2Settings settings) @safe
-	{
+	this(HTTPServerContext ctx, HTTP2Settings settings, ref NetworkAddress remote_address)
+	@safe {
 		m_context = ctx;
 		m_settings = settings;
+		m_remoteAddress = remote_address;
 	}
 
 	this(HTTPServerContext ctx) @safe
 	{
 		m_context = ctx;
 	}
 
+	@property ref const(NetworkAddress) remoteAddress() const @safe { return m_remoteAddress; }
+
 	@property auto ref table() @safe { return m_table.get; }
 
 	@property bool hasTable() @safe { return m_initializedT; }

source/vibe/http/server.d

@@ -193,6 +193,12 @@ unittest
 			virtual hosts
 */
 void handleHTTPConnection(TCPConnection connection, HTTPServerContext context)
+@safe {
+	auto raddr = connection.remoteAddress;
+	handleHTTPConnection(connection, context, raddr);
+}
+/// ditto
+void handleHTTPConnection(TCPConnection connection, HTTPServerContext context, ref NetworkAddress remote_address)
 @safe {
 	import vibe.http.internal.http1.server : handleHTTP1Connection;
 	import vibe.http.internal.http2.server : handleHTTP2Connection;
@@ -212,7 +218,7 @@ void handleHTTPConnection(TCPConnection connection, HTTPServerContext context)
 	// check wether the client's address is banned
 	foreach (ref virtual_host; context.m_virtualHosts)
 		if ((virtual_host.settings.rejectConnectionPredicate !is null) &&
-			virtual_host.settings.rejectConnectionPredicate(connection.remoteAddress()))
+			virtual_host.settings.rejectConnectionPredicate(remote_address))
 			return;
 
 	// Set NODELAY to true, to avoid delays caused by sending the response
@@ -235,13 +241,13 @@ void handleHTTPConnection(TCPConnection connection, HTTPServerContext context)
 		else {
 			logDebug("Accept TLS connection: %s", context.tlsContext.kind);
 			// TODO: reverse DNS lookup for peer_name of the incoming connection for TLS client certificate verification purposes
-			tls_stream = createTLSStreamFL(http_stream, context.tlsContext, TLSStreamState.accepting, null, connection.remoteAddress);
+			tls_stream = createTLSStreamFL(http_stream, context.tlsContext, TLSStreamState.accepting, null, remote_address);
 
 			Nullable!string proto = tls_stream.alpn;
 			if(!proto.isNull && proto == "h2" && (context.m_virtualHosts[0].settings.options & HTTPServerOption.enableHTTP2)) {
 				logTrace("Using HTTP/2 as requested per ALPN");
 				HTTP2Settings settings;
-				auto h2context = new HTTP2ServerContext(context, settings);
+				auto h2context = new HTTP2ServerContext(context, settings, remote_address);
 				handleHTTP2Connection(tls_stream, connection, h2context);
 				return;
 			}
@@ -250,7 +256,7 @@ void handleHTTPConnection(TCPConnection connection, HTTPServerContext context)
 		}
 	}
 
-	handleHTTP1Connection(connection, tls_stream, http_stream, context);
+	handleHTTP1Connection(connection, tls_stream, http_stream, context, remote_address);
 
 	logTrace("Done handling connection.");
 }
@@ -1900,9 +1906,10 @@ private HTTPListener listenHTTPPlain(HTTPServerSettings settings, HTTPServerRequ
 			if(reuseAddress) options |= TCPListenOptions.reuseAddress; else options &= ~TCPListenOptions.reuseAddress;
 			if(reusePort) options |= TCPListenOptions.reusePort; else options &= ~TCPListenOptions.reusePort;
 			auto ret = listenTCP(listen_info.bindPort, (TCPConnection conn) nothrow @safe {
-					try handleHTTPConnection(conn, listen_info);
+					auto raddr = conn.remoteAddress;
+					try handleHTTPConnection(conn, listen_info, raddr);
 					catch (Exception e) {
-						logError("HTTP connection handler has thrown at the peer %s: %s", conn.peerAddress, e.msg);
+						logError("HTTP connection handler has thrown at the peer %s: %s", raddr, e.msg);
 						debug logDebug("Full error: %s", () @trusted { return e.toString().sanitize(); } ());
 						try conn.close();
 						catch (Exception e) logError("Failed to close connection: %s", e.msg);