From a30aa849754ee32e357804149ba8162bb829d1c2 Mon Sep 17 00:00:00 2001 From: Violeta Georgieva Date: Fri, 26 Jul 2024 11:15:16 +0300 Subject: [PATCH] HttpDecoderSpec#validateHeaders() specifies whether request/response headers are validated (#3370) This is related only to HTTP/1.1, for HTTP/2 and HTTP/3 headers are always validated. By default the validation is enabled. --- .../Http2StreamBridgeServerHandler.java | 5 +-- .../http/server/Http3ServerOperations.java | 2 +- .../Http3StreamBridgeServerHandler.java | 2 +- .../netty/http/server/HttpServerConfig.java | 4 +-- .../http/server/HttpServerOperations.java | 35 ++++++++++++------- .../netty/http/server/HttpTrafficHandler.java | 33 +++++++++-------- .../netty/http/server/HttpServerTests.java | 6 ++-- 7 files changed, 53 insertions(+), 34 deletions(-) diff --git a/reactor-netty-http/src/main/java/reactor/netty/http/server/Http2StreamBridgeServerHandler.java b/reactor-netty-http/src/main/java/reactor/netty/http/server/Http2StreamBridgeServerHandler.java index 91bc96a341..3c7c72cccc 100644 --- a/reactor-netty-http/src/main/java/reactor/netty/http/server/Http2StreamBridgeServerHandler.java +++ b/reactor-netty-http/src/main/java/reactor/netty/http/server/Http2StreamBridgeServerHandler.java @@ -146,12 +146,13 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) { readTimeout, requestTimeout, secured, - timestamp); + timestamp, + true); } catch (RuntimeException e) { pendingResponse = false; request.setDecoderResult(DecoderResult.failure(e.getCause() != null ? e.getCause() : e)); - HttpServerOperations.sendDecodingFailures(ctx, listener, secured, e, msg, httpMessageLogFactory, true, timestamp, connectionInfo, remoteAddress); + HttpServerOperations.sendDecodingFailures(ctx, listener, secured, e, msg, httpMessageLogFactory, true, timestamp, connectionInfo, remoteAddress, true); return; } ops.bind(); diff --git a/reactor-netty-http/src/main/java/reactor/netty/http/server/Http3ServerOperations.java b/reactor-netty-http/src/main/java/reactor/netty/http/server/Http3ServerOperations.java index 00fd98cae5..b731e575c3 100644 --- a/reactor-netty-http/src/main/java/reactor/netty/http/server/Http3ServerOperations.java +++ b/reactor-netty-http/src/main/java/reactor/netty/http/server/Http3ServerOperations.java @@ -54,7 +54,7 @@ final class Http3ServerOperations extends HttpServerOperations { boolean secured, ZonedDateTime timestamp) { super(c, listener, nettyRequest, compressionPredicate, connectionInfo, decoder, encoder, formDecoderProvider, - httpMessageLogFactory, isHttp2, mapHandle, readTimeout, requestTimeout, secured, timestamp); + httpMessageLogFactory, isHttp2, mapHandle, readTimeout, requestTimeout, secured, timestamp, true); } @Override diff --git a/reactor-netty-http/src/main/java/reactor/netty/http/server/Http3StreamBridgeServerHandler.java b/reactor-netty-http/src/main/java/reactor/netty/http/server/Http3StreamBridgeServerHandler.java index cad7446244..caa8388c77 100644 --- a/reactor-netty-http/src/main/java/reactor/netty/http/server/Http3StreamBridgeServerHandler.java +++ b/reactor-netty-http/src/main/java/reactor/netty/http/server/Http3StreamBridgeServerHandler.java @@ -142,7 +142,7 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) { catch (RuntimeException e) { pendingResponse = false; request.setDecoderResult(DecoderResult.failure(e.getCause() != null ? e.getCause() : e)); - HttpServerOperations.sendDecodingFailures(ctx, listener, true, e, msg, httpMessageLogFactory, true, timestamp, connectionInfo, remoteAddress); + HttpServerOperations.sendDecodingFailures(ctx, listener, true, e, msg, httpMessageLogFactory, true, timestamp, connectionInfo, remoteAddress, true); return; } ops.bind(); diff --git a/reactor-netty-http/src/main/java/reactor/netty/http/server/HttpServerConfig.java b/reactor-netty-http/src/main/java/reactor/netty/http/server/HttpServerConfig.java index dcfa3ee897..f6a2fb3e6b 100644 --- a/reactor-netty-http/src/main/java/reactor/netty/http/server/HttpServerConfig.java +++ b/reactor-netty-http/src/main/java/reactor/netty/http/server/HttpServerConfig.java @@ -743,7 +743,7 @@ static void configureHttp11OrH2CleartextPipeline(ChannelPipeline p, NettyPipeline.HttpTrafficHandler, new HttpTrafficHandler(compressPredicate, cookieDecoder, cookieEncoder, formDecoderProvider, forwardedHeaderHandler, httpMessageLogFactory, idleTimeout, listener, mapHandle, maxKeepAliveRequests, - readTimeout, requestTimeout)); + readTimeout, requestTimeout, decoder.validateHeaders())); if (accessLogEnabled) { p.addAfter(NettyPipeline.HttpTrafficHandler, NettyPipeline.AccessLogHandler, AccessLogHandlerFactory.H1.create(accessLog)); @@ -813,7 +813,7 @@ static void configureHttp11Pipeline(ChannelPipeline p, NettyPipeline.HttpTrafficHandler, new HttpTrafficHandler(compressPredicate, cookieDecoder, cookieEncoder, formDecoderProvider, forwardedHeaderHandler, httpMessageLogFactory, idleTimeout, listener, mapHandle, maxKeepAliveRequests, - readTimeout, requestTimeout)); + readTimeout, requestTimeout, decoder.validateHeaders())); if (accessLogEnabled) { p.addAfter(NettyPipeline.HttpTrafficHandler, NettyPipeline.AccessLogHandler, AccessLogHandlerFactory.H1.create(accessLog)); diff --git a/reactor-netty-http/src/main/java/reactor/netty/http/server/HttpServerOperations.java b/reactor-netty-http/src/main/java/reactor/netty/http/server/HttpServerOperations.java index 340a9ec6b2..e0b0c81004 100644 --- a/reactor-netty-http/src/main/java/reactor/netty/http/server/HttpServerOperations.java +++ b/reactor-netty-http/src/main/java/reactor/netty/http/server/HttpServerOperations.java @@ -99,6 +99,7 @@ import reactor.util.context.Context; import static io.netty.buffer.Unpooled.EMPTY_BUFFER; +import static io.netty.handler.codec.http.DefaultHttpHeadersFactory.headersFactory; import static io.netty.handler.codec.http.DefaultHttpHeadersFactory.trailersFactory; import static io.netty.handler.codec.http.HttpUtil.isTransferEncodingChunked; import static reactor.netty.ReactorNetty.format; @@ -129,6 +130,7 @@ class HttpServerOperations extends HttpOperations compressionPredicate; boolean isWebsocket; @@ -165,6 +167,7 @@ class HttpServerOperations extends HttpOperations withConn @Override protected HttpMessage newFullBodyMessage(ByteBuf body) { HttpResponse res = - new DefaultFullHttpResponse(version(), status(), body); + new DefaultFullHttpResponse(version(), status(), body, + headersFactory().withValidation(validateHeaders), trailersFactory().withValidation(validateHeaders)); if (!HttpMethod.HEAD.equals(method())) { responseHeaders.remove(HttpHeaderNames.TRANSFER_ENCODING); @@ -415,7 +421,8 @@ public Flux receiveObject() { return FutureMono.deferFuture(() -> { if (!hasSentHeaders()) { return channel().writeAndFlush( - new DefaultFullHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.CONTINUE, EMPTY_BUFFER)); + new DefaultFullHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.CONTINUE, EMPTY_BUFFER, + headersFactory().withValidation(validateHeaders), trailersFactory().withValidation(validateHeaders))); } return channel().newSucceededFuture(); }) @@ -966,7 +973,7 @@ else if (contentLength != -1) { responseHeaders.remove(HttpHeaderNames.TRANSFER_ENCODING); } - return new DefaultFullHttpResponse(version(), status(), body, responseHeaders, trailersFactory().newHeaders()); + return new DefaultFullHttpResponse(version(), status(), body, responseHeaders, trailersFactory().withValidation(validateHeaders).newHeaders()); } static long requestsCounter(Channel channel) { @@ -988,8 +995,9 @@ static void sendDecodingFailures( HttpMessageLogFactory httpMessageLogFactory, @Nullable ZonedDateTime timestamp, @Nullable ConnectionInfo connectionInfo, - SocketAddress remoteAddress) { - sendDecodingFailures(ctx, listener, secure, t, msg, httpMessageLogFactory, false, timestamp, connectionInfo, remoteAddress); + SocketAddress remoteAddress, + boolean validateHeaders) { + sendDecodingFailures(ctx, listener, secure, t, msg, httpMessageLogFactory, false, timestamp, connectionInfo, remoteAddress, validateHeaders); } @SuppressWarnings("FutureReturnValueIgnored") @@ -1003,7 +1011,8 @@ static void sendDecodingFailures( boolean isHttp2, @Nullable ZonedDateTime timestamp, @Nullable ConnectionInfo connectionInfo, - SocketAddress remoteAddress) { + SocketAddress remoteAddress, + boolean validateHeaders) { Throwable cause = t.getCause() != null ? t.getCause() : t; @@ -1025,7 +1034,8 @@ else if (cause instanceof TooLongHttpHeaderException) { else { status = HttpResponseStatus.BAD_REQUEST; } - FullHttpResponse response = new DefaultFullHttpResponse(HttpVersion.HTTP_1_1, status); + FullHttpResponse response = new DefaultFullHttpResponse(HttpVersion.HTTP_1_1, status, Unpooled.buffer(0), + headersFactory().withValidation(validateHeaders), trailersFactory().withValidation(validateHeaders)); response.headers() .setInt(HttpHeaderNames.CONTENT_LENGTH, 0) .set(HttpHeaderNames.CONNECTION, HttpHeaderValues.CLOSE); @@ -1036,7 +1046,7 @@ else if (cause instanceof TooLongHttpHeaderException) { if (msg instanceof HttpRequest) { ops = new FailedHttpServerRequest(conn, listener, (HttpRequest) msg, response, httpMessageLogFactory, isHttp2, secure, timestamp == null ? ZonedDateTime.now(ReactorNetty.ZONE_ID_SYSTEM) : timestamp, - connectionInfo == null ? new ConnectionInfo(ctx.channel().localAddress(), remoteAddress, secure) : connectionInfo); + connectionInfo == null ? new ConnectionInfo(ctx.channel().localAddress(), remoteAddress, secure) : connectionInfo, validateHeaders); ops.bind(); } else { @@ -1213,10 +1223,11 @@ static final class FailedHttpServerRequest extends HttpServerOperations { boolean isHttp2, boolean secure, ZonedDateTime timestamp, - ConnectionInfo connectionInfo) { + ConnectionInfo connectionInfo, + boolean validateHeaders) { super(c, listener, nettyRequest, null, connectionInfo, ServerCookieDecoder.STRICT, ServerCookieEncoder.STRICT, DEFAULT_FORM_DECODER_SPEC, httpMessageLogFactory, isHttp2, - null, null, null, secure, timestamp); + null, null, null, secure, timestamp, validateHeaders); this.customResponse = nettyResponse; } diff --git a/reactor-netty-http/src/main/java/reactor/netty/http/server/HttpTrafficHandler.java b/reactor-netty-http/src/main/java/reactor/netty/http/server/HttpTrafficHandler.java index 4eee4376b1..b095d52285 100644 --- a/reactor-netty-http/src/main/java/reactor/netty/http/server/HttpTrafficHandler.java +++ b/reactor-netty-http/src/main/java/reactor/netty/http/server/HttpTrafficHandler.java @@ -84,6 +84,7 @@ final class HttpTrafficHandler extends ChannelDuplexHandler implements Runnable final int maxKeepAliveRequests; final Duration readTimeout; final Duration requestTimeout; + final boolean validateHeaders; ChannelHandlerContext ctx; @@ -116,7 +117,8 @@ final class HttpTrafficHandler extends ChannelDuplexHandler implements Runnable @Nullable BiFunction, ? super Connection, ? extends Mono> mapHandle, int maxKeepAliveRequests, @Nullable Duration readTimeout, - @Nullable Duration requestTimeout) { + @Nullable Duration requestTimeout, + boolean validateHeaders) { this.listener = listener; this.formDecoderProvider = formDecoderProvider; this.forwardedHeaderHandler = forwardedHeaderHandler; @@ -129,6 +131,7 @@ final class HttpTrafficHandler extends ChannelDuplexHandler implements Runnable this.maxKeepAliveRequests = maxKeepAliveRequests; this.readTimeout = readTimeout; this.requestTimeout = requestTimeout; + this.validateHeaders = validateHeaders; } @Override @@ -173,7 +176,7 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) { IllegalStateException e = new IllegalStateException( "Unexpected request [" + request.method() + " " + request.uri() + " HTTP/2.0]"); request.setDecoderResult(DecoderResult.failure(e.getCause() != null ? e.getCause() : e)); - sendDecodingFailures(e, msg); + sendDecodingFailures(e, msg, validateHeaders); return; } @@ -219,7 +222,7 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) { DecoderResult decoderResult = request.decoderResult(); if (decoderResult.isFailure()) { - sendDecodingFailures(decoderResult.cause(), msg); + sendDecodingFailures(decoderResult.cause(), msg, validateHeaders); return; } @@ -247,11 +250,12 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) { readTimeout, requestTimeout, secure, - timestamp); + timestamp, + validateHeaders); } catch (RuntimeException e) { request.setDecoderResult(DecoderResult.failure(e.getCause() != null ? e.getCause() : e)); - sendDecodingFailures(e, msg, timestamp, connectionInfo); + sendDecodingFailures(e, msg, timestamp, connectionInfo, validateHeaders); return; } ops.bind(); @@ -266,7 +270,7 @@ else if (persistentConnection && pendingResponses == 0) { if (msg instanceof LastHttpContent) { DecoderResult decoderResult = ((LastHttpContent) msg).decoderResult(); if (decoderResult.isFailure()) { - sendDecodingFailures(decoderResult.cause(), msg); + sendDecodingFailures(decoderResult.cause(), msg, validateHeaders); return; } @@ -298,7 +302,7 @@ else if (overflow) { if (msg instanceof DecoderResultProvider) { DecoderResult decoderResult = ((DecoderResultProvider) msg).decoderResult(); if (decoderResult.isFailure()) { - sendDecodingFailures(decoderResult.cause(), msg); + sendDecodingFailures(decoderResult.cause(), msg, validateHeaders); return; } } @@ -338,14 +342,14 @@ public void flush(ChannelHandlerContext ctx) { } } - void sendDecodingFailures(Throwable t, Object msg) { - sendDecodingFailures(t, msg, null, null); + void sendDecodingFailures(Throwable t, Object msg, boolean validateHeaders) { + sendDecodingFailures(t, msg, null, null, validateHeaders); } - void sendDecodingFailures(Throwable t, Object msg, @Nullable ZonedDateTime timestamp, @Nullable ConnectionInfo connectionInfo) { + void sendDecodingFailures(Throwable t, Object msg, @Nullable ZonedDateTime timestamp, @Nullable ConnectionInfo connectionInfo, boolean validateHeaders) { persistentConnection = false; HttpServerOperations.sendDecodingFailures(ctx, listener, secure, t, msg, httpMessageLogFactory, timestamp, connectionInfo, - remoteAddress); + remoteAddress, validateHeaders); } void doPipeline(ChannelHandlerContext ctx, Object msg) { @@ -478,7 +482,7 @@ public void run() { DecoderResult decoderResult = nextRequest.decoderResult(); if (decoderResult.isFailure()) { - sendDecodingFailures(decoderResult.cause(), nextRequest, holder.timestamp, null); + sendDecodingFailures(decoderResult.cause(), nextRequest, holder.timestamp, null, validateHeaders); discard(); return; } @@ -506,11 +510,12 @@ public void run() { readTimeout, requestTimeout, secure, - holder.timestamp); + holder.timestamp, + validateHeaders); } catch (RuntimeException e) { holder.request.setDecoderResult(DecoderResult.failure(e.getCause() != null ? e.getCause() : e)); - sendDecodingFailures(e, holder.request, holder.timestamp, connectionInfo); + sendDecodingFailures(e, holder.request, holder.timestamp, connectionInfo, validateHeaders); return; } ops.bind(); diff --git a/reactor-netty-http/src/test/java/reactor/netty/http/server/HttpServerTests.java b/reactor-netty-http/src/test/java/reactor/netty/http/server/HttpServerTests.java index 7b423253af..cd008286f5 100644 --- a/reactor-netty-http/src/test/java/reactor/netty/http/server/HttpServerTests.java +++ b/reactor-netty-http/src/test/java/reactor/netty/http/server/HttpServerTests.java @@ -2133,7 +2133,8 @@ private void doTestStatus(HttpResponseStatus status) { null, null, false, - ZonedDateTime.now(ReactorNetty.ZONE_ID_SYSTEM)); + ZonedDateTime.now(ReactorNetty.ZONE_ID_SYSTEM), + true); ops.status(status); HttpMessage response = ops.newFullBodyMessage(Unpooled.EMPTY_BUFFER); assertThat(((FullHttpResponse) response).status().reasonPhrase()).isEqualTo(status.reasonPhrase()); @@ -3136,7 +3137,8 @@ private void doTestIsFormUrlencoded(String headerValue, boolean expectation) { null, null, false, - ZonedDateTime.now(ReactorNetty.ZONE_ID_SYSTEM)); + ZonedDateTime.now(ReactorNetty.ZONE_ID_SYSTEM), + true); assertThat(ops.isFormUrlencoded()).isEqualTo(expectation); // "FutureReturnValueIgnored" is suppressed deliberately channel.close();