Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not apply filters when RouteExecutor decides not to handle an error #6745

Merged
merged 1 commit into from Jan 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions gradle/libs.versions.toml
Expand Up @@ -11,6 +11,7 @@ hibernate = "5.5.3.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"
Expand Down Expand Up @@ -524,6 +525,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" }

Expand Down
3 changes: 2 additions & 1 deletion http-server-netty/build.gradle
Expand Up @@ -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
Expand Down
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
Expand Down
@@ -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())
}
}
}
Expand Up @@ -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);

Expand Down