From fe0bcce83ca4577a067f2fec3e5896055fb5d512 Mon Sep 17 00:00:00 2001
From: Jonas Konrad <jonas.konrad@oracle.com>
Date: Tue, 5 Apr 2022 13:21:06 +0200
Subject: [PATCH] Properly proceed in filter processing when request is
 replaced (#7174)

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   | 98 +++++++++++++++++++
 .../micronaut/http/server/RouteExecutor.java  |  4 +-
 2 files changed, 100 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..682827d4eea
--- /dev/null
+++ b/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/filters/FilterReplaceRequestSpec.groovy
@@ -0,0 +1,98 @@
+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
+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.AutoCleanup
+import spock.lang.PendingFeature
+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'() {
+        when:
+        client.toBlocking().exchange("/initial", String)
+        Filter1 filter1 = server.applicationContext.getBean(Filter1)
+        Filter2 filter2 = server.applicationContext.getBean(Filter2)
+
+        then:
+        filter1.filteredRequest.path == "/initial"
+        filter2.filteredRequest.path == "/filter1"
+    }
+
+    @PendingFeature
+    def 'last filter http request is used to match route'() {
+        when:
+        HttpResponse<String> resp = client.toBlocking().retrieve("/initial", String)
+
+        then:
+        resp.body() == "filter2"
+    }
+    
+    @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<MutableHttpResponse<?>> doFilter(HttpRequest<?> request, ServerFilterChain chain) {
+            filteredRequest = request
+            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<MutableHttpResponse<?>> doFilter(HttpRequest<?> request, ServerFilterChain chain) {
+            filteredRequest = request
+            chain.proceed(HttpRequest.GET("/filter2"))
+        }
+    }
+
+    @Controller
+    @Requires(property = 'spec.name', value = 'FilterReplaceRequestSpec')
+    static class Ctrl {
+        @Get("/filter2")
+        String filter2() {
+            "filter2"
+        }
+
+        @Get("/initial")
+        String initial() {
+            "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<MutableHttpResponse<?>> proceed(io.micronaut.http.HttpRequest<?
                 }
                 HttpFilter httpFilter = filters.get(pos);
 
-                HttpRequest<?> requestForFilter = requestReference.getAndSet(request);
+                requestReference.set(request);
                 try {
-                    return Flux.from((Publisher<MutableHttpResponse<?>>) httpFilter.doFilter(requestForFilter, this))
+                    return Flux.from((Publisher<MutableHttpResponse<?>>) httpFilter.doFilter(request, this))
                             .flatMap(handleStatusException)
                             .onErrorResume(onError);
                 } catch (Throwable t) {