Don't run error routes when initial processing of malformed request f…

…ails (#11161)

When netty reports a decode failure, we run the status routes and error response processor. If *that* fails, we would run onError. onError will run filters which can occlude the error because the filters will not see the full headers (decoding failed after all) and may e.g. fail authentication.

This change removes the onError fallback from the decode failure branch.

http-server-netty/src/main/java/io/micronaut/http/server/netty/NettyRequestLifecycle.java

@@ -74,31 +74,37 @@ void handleNormal(NettyHttpRequest<?> request) {
 
         ExecutionFlow<HttpResponse<?>> result;
 
-        try {
-            // handle decoding failure
-            DecoderResult decoderResult = request.getNativeRequest().decoderResult();
-            if (decoderResult.isFailure()) {
-                Throwable cause = decoderResult.cause();
-                HttpStatus status = cause instanceof TooLongFrameException ? HttpStatus.REQUEST_ENTITY_TOO_LARGE : HttpStatus.BAD_REQUEST;
+        // handle decoding failure
+        DecoderResult decoderResult = request.getNativeRequest().decoderResult();
+        if (decoderResult.isFailure()) {
+            Throwable cause = decoderResult.cause();
+            HttpStatus status = cause instanceof TooLongFrameException ? HttpStatus.REQUEST_ENTITY_TOO_LARGE : HttpStatus.BAD_REQUEST;
+            try {
                 result = onStatusError(
                     request,
                     HttpResponse.status(status),
                     status.getReason()
                 );
-            } else {
-                result = normalFlow(request);
+            } catch (Exception e) {
+                result = ExecutionFlow.error(e);
             }
-            ImperativeExecutionFlow<HttpResponse<?>> imperativeFlow = result.tryComplete();
-            if (imperativeFlow != null) {
-                Object value = ((ImperativeExecutionFlow<?>) imperativeFlow).getValue();
-                // usually this is a MutableHttpResponse, avoid scalability issues here
-                HttpResponse<?> response = value instanceof NettyMutableHttpResponse<?> mut ? mut : (HttpResponse<?>) value;
-                rib.writeResponse(outboundAccess, request, response, imperativeFlow.getError());
-            } else {
-                result.onComplete((response, throwable) -> rib.writeResponse(outboundAccess, request, response, throwable));
+        } else {
+            try {
+                result = normalFlow(request);
+            } catch (Exception e) {
+                handleException(request, e);
+                return;
             }
-        } catch (Exception e) {
-            handleException(request, e);
+        }
+
+        ImperativeExecutionFlow<HttpResponse<?>> imperativeFlow = result.tryComplete();
+        if (imperativeFlow != null) {
+            Object value = ((ImperativeExecutionFlow<?>) imperativeFlow).getValue();
+            // usually this is a MutableHttpResponse, avoid scalability issues here
+            HttpResponse<?> response = value instanceof NettyMutableHttpResponse<?> mut ? mut : (HttpResponse<?>) value;
+            rib.writeResponse(outboundAccess, request, response, imperativeFlow.getError());
+        } else {
+            result.onComplete((response, throwable) -> rib.writeResponse(outboundAccess, request, response, throwable));
         }
     }
 

http-server-netty/src/test/groovy/io/micronaut/http/server/netty/errors/HeaderTooLongSpec.groovy

@@ -0,0 +1,60 @@
+package io.micronaut.http.server.netty.errors
+
+import io.micronaut.context.ApplicationContext
+import io.micronaut.context.annotation.Requires
+import io.micronaut.core.annotation.NonNull
+import io.micronaut.http.HttpRequest
+import io.micronaut.http.MutableHttpResponse
+import io.micronaut.http.annotation.RequestFilter
+import io.micronaut.http.annotation.ServerFilter
+import io.micronaut.http.server.exceptions.response.ErrorContext
+import io.micronaut.http.server.exceptions.response.ErrorResponseProcessor
+import io.micronaut.runtime.server.EmbeddedServer
+import jakarta.inject.Singleton
+import spock.lang.AutoCleanup
+import spock.lang.Shared
+import spock.lang.Specification
+
+class HeaderTooLongSpec extends Specification {
+
+    @Shared @AutoCleanup EmbeddedServer embeddedServer = ApplicationContext.run(EmbeddedServer, [
+            'spec.name': 'HeaderTooLongSpec',
+            'micronaut.server.netty.log-level': 'info'
+    ])
+
+    def 'header too long'() {
+        given:
+        def connection = new URL("$embeddedServer.URL/malformed-proxy/xyz").openConnection()
+        connection.setRequestProperty("foo", "b".repeat(9000))
+
+        def myFilter = embeddedServer.applicationContext.getBean(MyFilter)
+
+        when:
+        connection.inputStream
+        then:
+        thrown IOException
+        ((HttpURLConnection) connection).errorStream == null
+        myFilter.filteredRequest == null
+    }
+
+    @Singleton
+    @Requires(property = "spec.name", value = "HeaderTooLongSpec")
+    static class BrokenProcessor implements ErrorResponseProcessor<String> {
+        @Override
+        MutableHttpResponse<String> processResponse(@NonNull ErrorContext errorContext, @NonNull MutableHttpResponse<?> baseResponse) {
+            throw new Exception("This processor is intentionally broken")
+        }
+    }
+
+    @Singleton
+    @Requires(property = "spec.name", value = "HeaderTooLongSpec")
+    @ServerFilter("/**")
+    static class MyFilter {
+        HttpRequest<?> filteredRequest
+
+        @RequestFilter
+        void requestFilter(HttpRequest<?> request) {
+            filteredRequest = request
+        }
+    }
+}

http-server-netty/src/test/groovy/io/micronaut/http/server/netty/errors/MalformedUriSpec.groovy

@@ -15,6 +15,7 @@ import io.micronaut.http.annotation.ServerFilter
 import io.micronaut.http.client.HttpClient
 import io.micronaut.http.filter.HttpServerFilter
 import io.micronaut.http.filter.ServerFilterChain
+import io.micronaut.http.server.annotation.PreMatching
 import io.micronaut.runtime.server.EmbeddedServer
 import jakarta.inject.Singleton
 import org.reactivestreams.Publisher
@@ -64,6 +65,31 @@ class MalformedUriSpec extends Specification {
         result == 'Exception: Illegal character in path at index 17: /malformed-proxy/[]'
     }
 
+    void "header too long"() {
+        given:
+        OncePerFilter filter = embeddedServer.applicationContext.getBean(OncePerFilter)
+        filter.filterCalled = false
+
+        MalformedUriFilter newFilter = embeddedServer.applicationContext.getBean(MalformedUriFilter)
+        newFilter.preMatchingCalled = false
+
+        def connection = new URL("$embeddedServer.URL/malformed-proxy/xyz").openConnection()
+        connection.setRequestProperty("foo", "b".repeat(9000))
+
+        when:
+        connection.inputStream
+        then:
+        thrown IOException
+
+        when:
+        def result = ((HttpURLConnection) connection).errorStream.text
+
+        then:
+        result == '{"message":"Request Entity Too Large","_links":{"self":{"href":"/malformed-proxy/xyz","templated":false}},"_embedded":{"errors":[{"message":"Request Entity Too Large"}]}}'
+        !filter.filterCalled
+        !newFilter.preMatchingCalled
+    }
+
     @Requires(property = "spec.name", value = "MalformedUriSpec")
     @Controller('/malformed')
     static class SomeController {
@@ -97,10 +123,17 @@ class MalformedUriSpec extends Specification {
     @Singleton
     @ServerFilter("/malformed-proxy/**")
     static class MalformedUriFilter {
+        boolean preMatchingCalled
 
         @RequestFilter
         HttpResponse<?> filter(HttpRequest<?> request) {
             return HttpResponse.ok("ok: " + request.path)
         }
+
+        @RequestFilter
+        @PreMatching
+        void preMatching(HttpRequest<?> request) {
+            preMatchingCalled = false
+        }
     }
 }