Do not apply filters when RouteExecutor decides not to handle an error (

#6745)

For certain errors (`isIgnorable`), the `RouteExecutor` decides not to write a response for the error, and returns an empty flux. However, without this patch, the `RoutingInBoundHandler` would still apply filters. For example, the micronaut-security filter will check access permissions for the error request, even though those routes would never be executed anyway (`RouteExecutor` decided not to). That filter may then return a response.

This patch adds a short-circuit check for these 'closed connection' errors to `RoutingInBoundHandler`, and avoids running downstream processing altogether.

Fixes #6723

gradle/libs.versions.toml

@@ -11,6 +11,7 @@ hibernate = "5.5.9.Final"
 hibernate-validator = "6.1.6.Final"
 htmlSanityCheck = "1.1.6"
 htmlunit = "2.47.1"
+httpcomponents-client = "4.5.13"
 jakarta-inject-api = "2.0.1"
 jakarta-inject-tck = "2.0.1"
 javax-inject = "1"
@@ -526,6 +527,7 @@ testcontainers-spock = { module = "org.testcontainers:spock", version.ref = "man
 
 vertx = { module = "io.vertx:vertx-core", version.ref = "vertx" }
 vertx-webclient = { module = "io.vertx:vertx-web-client", version.ref = "vertx" }
+httpcomponents-client = { module = "org.apache.httpcomponents:httpclient", version.ref = "httpcomponents-client" }
 
 wiremock = { module = "com.github.tomakehurst:wiremock-jre8", version.ref = "wiremock" }
 

http-server-netty/build.gradle

@@ -42,9 +42,10 @@ dependencies {
     }
     testImplementation libs.managed.jackson.databind
 
-    // following 3 dependencies needed for Http2PostTest
+    // http impls for tests
     testImplementation libs.vertx
     testImplementation libs.vertx.webclient
+    testImplementation libs.httpcomponents.client
     testImplementation libs.jetty.alpn.openjdk8.client
 
     testImplementation libs.managed.groovy.json

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

@@ -144,6 +144,9 @@
 class RoutingInBoundHandler extends SimpleChannelInboundHandler<io.micronaut.http.HttpRequest<?>> {
 
     private static final Logger LOG = LoggerFactory.getLogger(RoutingInBoundHandler.class);
+    /**
+     * Also present in {@link RouteExecutor}
+     */
     private static final Pattern IGNORABLE_ERROR_MESSAGE = Pattern.compile(
             "^.*(?:connection.*(?:reset|closed|abort|broken)|broken.*pipe).*$", Pattern.CASE_INSENSITIVE);
     private static final Argument ARGUMENT_PART_DATA = Argument.of(PartData.class);
@@ -245,9 +248,18 @@ public void userEventTriggered(ChannelHandlerContext ctx, Object evt) throws Exc
 
     @Override
     public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) {
+        // short-circuit ignorable exceptions: This is also handled by RouteExecutor, but handling this early avoids
+        // running any filters
+        if (isIgnorable(cause)) {
+            if (LOG.isDebugEnabled()) {
+                LOG.debug("Swallowed an IOException caused by client connectivity: " + cause.getMessage(), cause);
+            }
+            return;
+        }
+
         NettyHttpRequest<?> nettyHttpRequest = NettyHttpRequest.remove(ctx);
         if (nettyHttpRequest == null) {
-            if (cause instanceof SSLException || cause.getCause() instanceof SSLException || isIgnorable(cause)) {
+            if (cause instanceof SSLException || cause.getCause() instanceof SSLException) {
                 if (LOG.isDebugEnabled()) {
                     LOG.debug("Micronaut Server Error - No request state present. Cause: " + cause.getMessage(), cause);
                 }

http-server-netty/src/test/groovy/io/micronaut/http/server/netty/ImmediateCloseSpec.groovy

@@ -0,0 +1,77 @@
+package io.micronaut.http.server.netty
+
+import io.micronaut.context.ApplicationContext
+import io.micronaut.context.annotation.Requires
+import io.micronaut.core.async.publisher.Publishers
+import io.micronaut.http.HttpRequest
+import io.micronaut.http.HttpResponse
+import io.micronaut.http.MutableHttpResponse
+import io.micronaut.http.annotation.Controller
+import io.micronaut.http.annotation.Get
+import io.micronaut.http.filter.HttpServerFilter
+import io.micronaut.http.filter.ServerFilterChain
+import io.micronaut.runtime.server.EmbeddedServer
+import jakarta.inject.Inject
+import jakarta.inject.Singleton
+import org.apache.http.client.methods.HttpGet
+import org.apache.http.impl.client.CloseableHttpClient
+import org.apache.http.impl.client.HttpClients
+import org.reactivestreams.Publisher
+import spock.lang.Issue
+import spock.lang.Specification
+
+class ImmediateCloseSpec extends Specification {
+    @Issue('https://github.com/micronaut-projects/micronaut-core/issues/6723')
+    def 'immediate close of client connection should not lead to log message'() {
+        given:
+        def ctx = ApplicationContext.run([
+                'spec.name': 'ImmediateCloseSpec',
+                //'micronaut.server.port': 8080
+        ])
+        def embeddedServer = ctx.getBean(EmbeddedServer)
+        embeddedServer.start()
+
+        when:
+        HttpGet get = new HttpGet(embeddedServer.URI.toString() + '/empty')
+        CloseableHttpClient httpClient = HttpClients.createDefault()
+        def response = httpClient.execute(get)
+        httpClient.close()
+        embeddedServer.close() // wait for connection handling to finish
+
+        then:
+        response.getStatusLine().statusCode == 401
+        ctx.getBean(Holder).handleCount == 1
+        // can't check for log messages :(
+
+        cleanup:
+        embeddedServer.close()
+        httpClient.close()
+    }
+
+    @Requires(property = 'spec.name', value = 'ImmediateCloseSpec')
+    @Controller('/empty')
+    static class EmptyController {
+        @Get
+        HttpResponse<?> get() {
+            return HttpResponse.ok()
+        }
+    }
+
+    @Requires(property = 'spec.name', value = 'ImmediateCloseSpec')
+    @Singleton
+    static class Holder {
+        int handleCount = 0
+    }
+
+    @Requires(property = 'spec.name', value = 'ImmediateCloseSpec')
+    @io.micronaut.http.annotation.Filter('/**')
+    static class Filter implements HttpServerFilter {
+        @Inject Holder holder
+
+        @Override
+        Publisher<MutableHttpResponse<?>> doFilter(HttpRequest<?> request, ServerFilterChain chain) {
+            holder.handleCount++
+            return Publishers.just(HttpResponse.unauthorized())
+        }
+    }
+}

http-server/src/main/java/io/micronaut/http/server/RouteExecutor.java

@@ -92,6 +92,9 @@
 public final class RouteExecutor {
 
     private static final Logger LOG = LoggerFactory.getLogger(RouteExecutor.class);
+    /**
+     * Also present in netty RoutingInBoundHandler
+     */
     private static final Pattern IGNORABLE_ERROR_MESSAGE = Pattern.compile(
             "^.*(?:connection.*(?:reset|closed|abort|broken)|broken.*pipe).*$", Pattern.CASE_INSENSITIVE);