From cb945245f2d9c69f012b0828d8a708ebcb8d3d12 Mon Sep 17 00:00:00 2001 From: yawkat Date: Fri, 1 Apr 2022 18:03:26 +0200 Subject: [PATCH 1/7] Properly proceed in filter processing when request is replaced When a filter calls chain.proceed with a new request, that request should be used for further processing. Replacing the request is still not fully working even with this change, so #5491 is not fully fixed. RoutingInBoundHandler still does not consider the new request, because most of the request processing happens before the filter is applied. In the test case of this patch, this manifests as the response text still being "initial" even though the request has been replaced. --- .../filters/FilterReplaceRequestSpec.groovy | 87 +++++++++++++++++++ .../micronaut/http/server/RouteExecutor.java | 4 +- 2 files changed, 89 insertions(+), 2 deletions(-) create mode 100644 http-server-netty/src/test/groovy/io/micronaut/http/server/netty/filters/FilterReplaceRequestSpec.groovy diff --git a/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/filters/FilterReplaceRequestSpec.groovy b/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/filters/FilterReplaceRequestSpec.groovy new file mode 100644 index 00000000000..d4eb2be4d91 --- /dev/null +++ b/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/filters/FilterReplaceRequestSpec.groovy @@ -0,0 +1,87 @@ +package io.micronaut.http.server.netty.filters + +import io.micronaut.context.ApplicationContext +import io.micronaut.context.annotation.Requires +import io.micronaut.http.HttpRequest +import io.micronaut.http.MutableHttpResponse +import io.micronaut.http.annotation.Controller +import io.micronaut.http.annotation.Filter +import io.micronaut.http.annotation.Get +import io.micronaut.http.client.HttpClient +import io.micronaut.http.filter.HttpServerFilter +import io.micronaut.http.filter.ServerFilterChain +import io.micronaut.runtime.server.EmbeddedServer +import org.reactivestreams.Publisher +import spock.lang.Specification + +class FilterReplaceRequestSpec extends Specification { + def 'test replaced http request is handled by next filter'() { + given: + def ctx = ApplicationContext.run(['spec.name': 'FilterReplaceRequestSpec']) + def server = ctx.getBean(EmbeddedServer) + server.start() + def client = ctx.createBean(HttpClient, server.URI) + def filter1 = ctx.getBean(Filter1) + def filter2 = ctx.getBean(Filter2) + + when: + def resp = client.toBlocking().exchange("/initial", String) + then: + resp.body() == "initial" + filter1.filteredRequest.path == "/initial" + filter2.filteredRequest.path == "/filter1" + + cleanup: + server.stop() + client.stop() + } + + + @Filter(Filter.MATCH_ALL_PATTERN) + @Requires(property = 'spec.name', value = 'FilterReplaceRequestSpec') + static class Filter1 implements HttpServerFilter { + HttpRequest filteredRequest = null + + @Override + int getOrder() { + 1 + } + + @Override + Publisher> doFilter(HttpRequest request, ServerFilterChain chain) { + filteredRequest = request + return chain.proceed(HttpRequest.GET("/filter1")) + } + } + + @Filter(Filter.MATCH_ALL_PATTERN) + @Requires(property = 'spec.name', value = 'FilterReplaceRequestSpec') + static class Filter2 implements HttpServerFilter { + HttpRequest filteredRequest = null + + @Override + int getOrder() { + 2 + } + + @Override + Publisher> doFilter(HttpRequest request, ServerFilterChain chain) { + filteredRequest = request + return chain.proceed(HttpRequest.GET("/filter2")) + } + } + + @Controller + @Requires(property = 'spec.name', value = 'FilterReplaceRequestSpec') + static class Ctrl { + @Get("/filter2") + def filter2() { + return "filter2" + } + + @Get("/initial") + def initial() { + return "initial" + } + } +} diff --git a/http-server/src/main/java/io/micronaut/http/server/RouteExecutor.java b/http-server/src/main/java/io/micronaut/http/server/RouteExecutor.java index 0af01c28ddb..4ece878cd99 100644 --- a/http-server/src/main/java/io/micronaut/http/server/RouteExecutor.java +++ b/http-server/src/main/java/io/micronaut/http/server/RouteExecutor.java @@ -393,9 +393,9 @@ public Publisher> proceed(io.micronaut.http.HttpRequest requestForFilter = requestReference.getAndSet(request); + requestReference.set(request); try { - return Flux.from((Publisher>) httpFilter.doFilter(requestForFilter, this)) + return Flux.from((Publisher>) httpFilter.doFilter(request, this)) .flatMap(handleStatusException) .onErrorResume(onError); } catch (Throwable t) { From 5217a0c5760f5285a99c1d4071002dc682146dcc Mon Sep 17 00:00:00 2001 From: Sergio del Amo Date: Tue, 5 Apr 2022 10:44:00 +0200 Subject: [PATCH 2/7] use type --- .../filters/FilterReplaceRequestSpec.groovy | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/filters/FilterReplaceRequestSpec.groovy b/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/filters/FilterReplaceRequestSpec.groovy index d4eb2be4d91..dd555a3799a 100644 --- a/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/filters/FilterReplaceRequestSpec.groovy +++ b/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/filters/FilterReplaceRequestSpec.groovy @@ -3,6 +3,7 @@ package io.micronaut.http.server.netty.filters import io.micronaut.context.ApplicationContext import io.micronaut.context.annotation.Requires 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.Filter @@ -17,15 +18,15 @@ import spock.lang.Specification class FilterReplaceRequestSpec extends Specification { def 'test replaced http request is handled by next filter'() { given: - def ctx = ApplicationContext.run(['spec.name': 'FilterReplaceRequestSpec']) - def server = ctx.getBean(EmbeddedServer) + ApplicationContext ctx = ApplicationContext.run(['spec.name': 'FilterReplaceRequestSpec']) + EmbeddedServer server = ctx.getBean(EmbeddedServer) server.start() - def client = ctx.createBean(HttpClient, server.URI) - def filter1 = ctx.getBean(Filter1) - def filter2 = ctx.getBean(Filter2) + HttpClient client = ctx.createBean(HttpClient, server.URI) + Filter1 filter1 = ctx.getBean(Filter1) + Filter2 filter2 = ctx.getBean(Filter2) when: - def resp = client.toBlocking().exchange("/initial", String) + HttpResponse resp = client.toBlocking().exchange("/initial", String) then: resp.body() == "initial" filter1.filteredRequest.path == "/initial" @@ -75,12 +76,12 @@ class FilterReplaceRequestSpec extends Specification { @Requires(property = 'spec.name', value = 'FilterReplaceRequestSpec') static class Ctrl { @Get("/filter2") - def filter2() { + String filter2() { return "filter2" } @Get("/initial") - def initial() { + String initial() { return "initial" } } From 97ec2f75a31452ff5463d309debc294c09dad43a Mon Sep 17 00:00:00 2001 From: Sergio del Amo Date: Tue, 5 Apr 2022 10:45:34 +0200 Subject: [PATCH 3/7] remove implicit return keyword --- .../server/netty/filters/FilterReplaceRequestSpec.groovy | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/filters/FilterReplaceRequestSpec.groovy b/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/filters/FilterReplaceRequestSpec.groovy index dd555a3799a..1fb2566b89f 100644 --- a/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/filters/FilterReplaceRequestSpec.groovy +++ b/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/filters/FilterReplaceRequestSpec.groovy @@ -51,7 +51,7 @@ class FilterReplaceRequestSpec extends Specification { @Override Publisher> doFilter(HttpRequest request, ServerFilterChain chain) { filteredRequest = request - return chain.proceed(HttpRequest.GET("/filter1")) + chain.proceed(HttpRequest.GET("/filter1")) } } @@ -68,7 +68,7 @@ class FilterReplaceRequestSpec extends Specification { @Override Publisher> doFilter(HttpRequest request, ServerFilterChain chain) { filteredRequest = request - return chain.proceed(HttpRequest.GET("/filter2")) + chain.proceed(HttpRequest.GET("/filter2")) } } @@ -77,12 +77,12 @@ class FilterReplaceRequestSpec extends Specification { static class Ctrl { @Get("/filter2") String filter2() { - return "filter2" + "filter2" } @Get("/initial") String initial() { - return "initial" + "initial" } } } From b613fa932a14d24380bfd33c0ae2f81f5ed81d77 Mon Sep 17 00:00:00 2001 From: Sergio del Amo Date: Tue, 5 Apr 2022 10:47:08 +0200 Subject: [PATCH 4/7] use EmbeddedServer server = ApplicationContext.run(EmbeddedServer, --- .../netty/filters/FilterReplaceRequestSpec.groovy | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/filters/FilterReplaceRequestSpec.groovy b/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/filters/FilterReplaceRequestSpec.groovy index 1fb2566b89f..797043fe262 100644 --- a/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/filters/FilterReplaceRequestSpec.groovy +++ b/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/filters/FilterReplaceRequestSpec.groovy @@ -18,12 +18,10 @@ import spock.lang.Specification class FilterReplaceRequestSpec extends Specification { def 'test replaced http request is handled by next filter'() { given: - ApplicationContext ctx = ApplicationContext.run(['spec.name': 'FilterReplaceRequestSpec']) - EmbeddedServer server = ctx.getBean(EmbeddedServer) - server.start() - HttpClient client = ctx.createBean(HttpClient, server.URI) - Filter1 filter1 = ctx.getBean(Filter1) - Filter2 filter2 = ctx.getBean(Filter2) + EmbeddedServer server = ApplicationContext.run(EmbeddedServer, ['spec.name': 'FilterReplaceRequestSpec']) + HttpClient client = server.applicationContext.createBean(HttpClient, server.URI) + Filter1 filter1 = server.applicationContext.getBean(Filter1) + Filter2 filter2 = server.applicationContext.getBean(Filter2) when: HttpResponse resp = client.toBlocking().exchange("/initial", String) From a4da25dd78fef76ebd59c5fdc034d54fa780a6f1 Mon Sep 17 00:00:00 2001 From: Sergio del Amo Date: Tue, 5 Apr 2022 10:47:15 +0200 Subject: [PATCH 5/7] remove extra blank line --- .../http/server/netty/filters/FilterReplaceRequestSpec.groovy | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/filters/FilterReplaceRequestSpec.groovy b/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/filters/FilterReplaceRequestSpec.groovy index 797043fe262..e444e70977b 100644 --- a/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/filters/FilterReplaceRequestSpec.groovy +++ b/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/filters/FilterReplaceRequestSpec.groovy @@ -34,8 +34,7 @@ class FilterReplaceRequestSpec extends Specification { server.stop() client.stop() } - - + @Filter(Filter.MATCH_ALL_PATTERN) @Requires(property = 'spec.name', value = 'FilterReplaceRequestSpec') static class Filter1 implements HttpServerFilter { From 27e8cfdc8ac0337568fff17a5e134f9dc8a704c2 Mon Sep 17 00:00:00 2001 From: Sergio del Amo Date: Tue, 5 Apr 2022 10:57:27 +0200 Subject: [PATCH 6/7] test: use @Shared --- .../filters/FilterReplaceRequestSpec.groovy | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/filters/FilterReplaceRequestSpec.groovy b/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/filters/FilterReplaceRequestSpec.groovy index e444e70977b..1ec40986abf 100644 --- a/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/filters/FilterReplaceRequestSpec.groovy +++ b/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/filters/FilterReplaceRequestSpec.groovy @@ -13,26 +13,30 @@ import io.micronaut.http.filter.HttpServerFilter import io.micronaut.http.filter.ServerFilterChain import io.micronaut.runtime.server.EmbeddedServer import org.reactivestreams.Publisher +import spock.lang.AutoCleanup +import spock.lang.Shared import spock.lang.Specification class FilterReplaceRequestSpec extends Specification { + + @Shared + @AutoCleanup + EmbeddedServer server = ApplicationContext.run(EmbeddedServer, ['spec.name': 'FilterReplaceRequestSpec']) + + @Shared + @AutoCleanup + HttpClient client = server.applicationContext.createBean(HttpClient, server.URI) + def 'test replaced http request is handled by next filter'() { - given: - EmbeddedServer server = ApplicationContext.run(EmbeddedServer, ['spec.name': 'FilterReplaceRequestSpec']) - HttpClient client = server.applicationContext.createBean(HttpClient, server.URI) + when: + HttpResponse resp = client.toBlocking().exchange("/initial", String) Filter1 filter1 = server.applicationContext.getBean(Filter1) Filter2 filter2 = server.applicationContext.getBean(Filter2) - when: - HttpResponse resp = client.toBlocking().exchange("/initial", String) then: resp.body() == "initial" filter1.filteredRequest.path == "/initial" filter2.filteredRequest.path == "/filter1" - - cleanup: - server.stop() - client.stop() } @Filter(Filter.MATCH_ALL_PATTERN) From 3e72e9f22cb75488a5a762ac938b9342b3e88d6d Mon Sep 17 00:00:00 2001 From: Sergio del Amo Date: Tue, 5 Apr 2022 10:59:13 +0200 Subject: [PATCH 7/7] extract test and annotate with @PendingFeature --- .../netty/filters/FilterReplaceRequestSpec.groovy | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/filters/FilterReplaceRequestSpec.groovy b/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/filters/FilterReplaceRequestSpec.groovy index 1ec40986abf..682827d4eea 100644 --- a/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/filters/FilterReplaceRequestSpec.groovy +++ b/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/filters/FilterReplaceRequestSpec.groovy @@ -14,6 +14,7 @@ import io.micronaut.http.filter.ServerFilterChain import io.micronaut.runtime.server.EmbeddedServer import org.reactivestreams.Publisher import spock.lang.AutoCleanup +import spock.lang.PendingFeature import spock.lang.Shared import spock.lang.Specification @@ -26,18 +27,26 @@ class FilterReplaceRequestSpec extends Specification { @Shared @AutoCleanup HttpClient client = server.applicationContext.createBean(HttpClient, server.URI) - + def 'test replaced http request is handled by next filter'() { when: - HttpResponse resp = client.toBlocking().exchange("/initial", String) + client.toBlocking().exchange("/initial", String) Filter1 filter1 = server.applicationContext.getBean(Filter1) Filter2 filter2 = server.applicationContext.getBean(Filter2) then: - resp.body() == "initial" filter1.filteredRequest.path == "/initial" filter2.filteredRequest.path == "/filter1" } + + @PendingFeature + def 'last filter http request is used to match route'() { + when: + HttpResponse resp = client.toBlocking().retrieve("/initial", String) + + then: + resp.body() == "filter2" + } @Filter(Filter.MATCH_ALL_PATTERN) @Requires(property = 'spec.name', value = 'FilterReplaceRequestSpec')