From a29aa82853a07cf5bb98efead7514ff14db2a476 Mon Sep 17 00:00:00 2001 From: James Yuzawa Date: Fri, 7 Oct 2022 06:51:45 -0400 Subject: [PATCH] Support HTTP errors 414 and 431 (#2534) 414 is "uri too long" https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/414 431 is "header too long" https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/431 Old behaviour was using 413 "request payload too large" which is specific to the content, not the first line or headers. --- .../ITTracingHttpServerDecoratorTest.java | 2 +- .../http/server/HttpServerOperations.java | 17 +++++++++---- .../netty/http/HttpMetricsHandlerTests.java | 6 ++--- .../netty/http/server/HttpServerTests.java | 24 ++++++++++++++++++- 4 files changed, 40 insertions(+), 9 deletions(-) diff --git a/reactor-netty-http-brave/src/test/java/reactor/netty/http/brave/ITTracingHttpServerDecoratorTest.java b/reactor-netty-http-brave/src/test/java/reactor/netty/http/brave/ITTracingHttpServerDecoratorTest.java index 5dde784024..57860119b1 100644 --- a/reactor-netty-http-brave/src/test/java/reactor/netty/http/brave/ITTracingHttpServerDecoratorTest.java +++ b/reactor-netty-http-brave/src/test/java/reactor/netty/http/brave/ITTracingHttpServerDecoratorTest.java @@ -151,6 +151,6 @@ public void testBadRequest() throws IOException { this.get("/request_line_too_long"); - assertThat(testSpanHandler.takeRemoteSpanWithErrorTag(SERVER, "413").tags()).containsEntry("error", "413"); + assertThat(testSpanHandler.takeRemoteSpanWithErrorTag(SERVER, "414").tags()).containsEntry("error", "414"); } } \ No newline at end of file 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 34172f5ce3..2f55268cd8 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 @@ -38,7 +38,6 @@ import io.netty.channel.ChannelFutureListener; import io.netty.channel.ChannelHandlerContext; import io.netty.handler.codec.DefaultHeaders; -import io.netty.handler.codec.TooLongFrameException; import io.netty.handler.codec.http.DefaultFullHttpResponse; import io.netty.handler.codec.http.DefaultHttpHeaders; import io.netty.handler.codec.http.DefaultHttpResponse; @@ -58,6 +57,8 @@ import io.netty.handler.codec.http.HttpUtil; import io.netty.handler.codec.http.HttpVersion; import io.netty.handler.codec.http.LastHttpContent; +import io.netty.handler.codec.http.TooLongHttpHeaderException; +import io.netty.handler.codec.http.TooLongHttpLineException; import io.netty.handler.codec.http.cookie.Cookie; import io.netty.handler.codec.http.cookie.ServerCookieDecoder; import io.netty.handler.codec.http.cookie.ServerCookieEncoder; @@ -750,9 +751,17 @@ static void sendDecodingFailures( ReferenceCountUtil.release(msg); - HttpResponse response = new DefaultFullHttpResponse(HttpVersion.HTTP_1_1, - cause instanceof TooLongFrameException ? HttpResponseStatus.REQUEST_ENTITY_TOO_LARGE : - HttpResponseStatus.BAD_REQUEST); + final HttpResponseStatus status; + if (cause instanceof TooLongHttpLineException) { + status = HttpResponseStatus.REQUEST_URI_TOO_LONG; + } + else if (cause instanceof TooLongHttpHeaderException) { + status = HttpResponseStatus.REQUEST_HEADER_FIELDS_TOO_LARGE; + } + else { + status = HttpResponseStatus.BAD_REQUEST; + } + HttpResponse response = new DefaultFullHttpResponse(HttpVersion.HTTP_1_1, status); response.headers() .setInt(HttpHeaderNames.CONTENT_LENGTH, 0) .set(HttpHeaderNames.CONNECTION, HttpHeaderValues.CLOSE); diff --git a/reactor-netty-http/src/test/java/reactor/netty/http/HttpMetricsHandlerTests.java b/reactor-netty-http/src/test/java/reactor/netty/http/HttpMetricsHandlerTests.java index a8f65d63b5..9ee9f5283f 100644 --- a/reactor-netty-http/src/test/java/reactor/netty/http/HttpMetricsHandlerTests.java +++ b/reactor-netty-http/src/test/java/reactor/netty/http/HttpMetricsHandlerTests.java @@ -764,7 +764,7 @@ void testBadRequest(HttpProtocol[] serverProtocols, HttpProtocol[] clientProtoco .uri("/max_header_size") .responseSingle((res, byteBufMono) -> Mono.just(res.status().code())) .as(StepVerifier::create) - .expectNext(413) + .expectNext(431) .expectComplete() .verify(Duration.ofSeconds(30)); @@ -879,7 +879,7 @@ private void checkExpectationsNonExisting(String serverAddress, int connIndex, i private void checkExpectationsBadRequest(String serverAddress, boolean checkTls) { String uri = "/max_header_size"; - String[] timerTags1 = new String[] {URI, uri, METHOD, "GET", STATUS, "413"}; + String[] timerTags1 = new String[] {URI, uri, METHOD, "GET", STATUS, "431"}; String[] summaryTags1 = new String[] {URI, uri}; checkTimer(SERVER_RESPONSE_TIME, timerTags1, 1); @@ -887,7 +887,7 @@ private void checkExpectationsBadRequest(String serverAddress, boolean checkTls) checkDistributionSummary(SERVER_DATA_SENT, summaryTags1, 1, 0); checkCounter(SERVER_ERRORS, summaryTags1, false, 0); - timerTags1 = new String[] {REMOTE_ADDRESS, serverAddress, URI, uri, METHOD, "GET", STATUS, "413"}; + timerTags1 = new String[] {REMOTE_ADDRESS, serverAddress, URI, uri, METHOD, "GET", STATUS, "431"}; String[] timerTags2 = new String[] {REMOTE_ADDRESS, serverAddress, URI, uri, METHOD, "GET"}; String[] timerTags3 = new String[] {REMOTE_ADDRESS, serverAddress, STATUS, "SUCCESS"}; summaryTags1 = new String[] {REMOTE_ADDRESS, serverAddress, URI, uri}; 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 e8e47339db..69da6f4dda 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 @@ -923,7 +923,29 @@ private void doTestIssue309(String path, HttpServer httpServer) { .responseSingle((res, byteBufMono) -> Mono.just(res.status())); StepVerifier.create(status) - .expectNextMatches(HttpResponseStatus.REQUEST_ENTITY_TOO_LARGE::equals) + .expectNextMatches(HttpResponseStatus.REQUEST_URI_TOO_LONG::equals) + .expectComplete() + .verify(); + } + + @Test + void httpServerRequestHeadersTooLong() { + HttpServer httpServer = HttpServer.create() + .port(0) + .httpRequestDecoder(c -> c.maxHeaderSize(20)); + disposableServer = + httpServer.handle((req, res) -> res.sendString(Mono.just("Should not be reached"))) + .bindNow(); + + Mono status = + createClient(disposableServer.port()) + .headers(h -> h.set("content-type", "somethingtooolooong")) + .get() + .uri("/path") + .responseSingle((res, byteBufMono) -> Mono.just(res.status())); + + StepVerifier.create(status) + .expectNextMatches(HttpResponseStatus.REQUEST_HEADER_FIELDS_TOO_LARGE::equals) .expectComplete() .verify(); }