Merge pull request #136 from georchestra/bug/applicationerror_filter_…

…removes_response_headers

Preserve response headers when redirecting application error to gateway error pages

gateway/src/main/java/org/georchestra/gateway/filter/global/ApplicationErrorGatewayFilterFactory.java

@@ -18,19 +18,19 @@
  */
 package org.georchestra.gateway.filter.global;
 
-import java.net.URI;
+import java.util.function.Supplier;
 
 import org.springframework.cloud.gateway.filter.GatewayFilter;
 import org.springframework.cloud.gateway.filter.GatewayFilterChain;
 import org.springframework.cloud.gateway.filter.factory.AbstractGatewayFilterFactory;
 import org.springframework.cloud.gateway.filter.factory.GatewayFilterFactory;
-import org.springframework.cloud.gateway.support.HttpStatusHolder;
-import org.springframework.cloud.gateway.support.ServerWebExchangeUtils;
 import org.springframework.core.Ordered;
+import org.springframework.http.HttpMethod;
 import org.springframework.http.HttpStatus;
+import org.springframework.http.MediaType;
+import org.springframework.http.server.reactive.ServerHttpRequest;
 import org.springframework.http.server.reactive.ServerHttpResponse;
 import org.springframework.http.server.reactive.ServerHttpResponseDecorator;
-import org.springframework.lang.Nullable;
 import org.springframework.web.server.ResponseStatusException;
 import org.springframework.web.server.ServerWebExchange;
 
@@ -39,7 +39,8 @@
 
 /**
  * Filter to allow custom error pages to be used when an application behind the
- * gateways returns an error.
+ * gateways returns an error, only for idempotent HTTP response status codes
+ * (i.e. GET, HEAD, OPTIONS).
  * <p>
  * {@link GatewayFilterFactory} providing a {@link GatewayFilter} that throws a
  * {@link ResponseStatusException} with the proxied response status code if the
@@ -80,29 +81,59 @@ public GatewayFilter apply(final Object config) {
         return new ServiceErrorGatewayFilter();
     }
 
-    private static class ServiceErrorGatewayFilter implements GatewayFilter, Ordered {
-
-        public @Override Mono<Void> filter(ServerWebExchange exchange, GatewayFilterChain chain) {
-
-            ApplicationErrorConveyorHttpResponse response;
-            response = new ApplicationErrorConveyorHttpResponse(exchange.getResponse());
-
-            exchange = exchange.mutate().response(response).build();
-            return chain.filter(exchange);
+    private class ServiceErrorGatewayFilter implements GatewayFilter, Ordered {
+        /**
+         * @return {@link Ordered#HIGHEST_PRECEDENCE} or
+         *         {@link ApplicationErrorConveyorHttpResponse#beforeCommit(Supplier)}
+         *         won't be called
+         */
+        @Override
+        public int getOrder() {
+            return Ordered.HIGHEST_PRECEDENCE;
         }
 
+        /**
+         * If the request method is idempotent and accepts {@literal text/html}, applies
+         * a filter that when the routed response receives an error status code, will
+         * throw a {@link ResponseStatusException} with the same status, for the gateway
+         * to apply the customized error template, also when the status code comes from
+         * a proxied service response
+         */
         @Override
-        public int getOrder() {
-            return ResolveTargetGlobalFilter.ORDER + 1;
+        public Mono<Void> filter(ServerWebExchange exchange, GatewayFilterChain chain) {
+            if (canFilter(exchange.getRequest())) {
+                exchange = decorate(exchange);
+            }
+            return chain.filter(exchange);
         }
+    }
+
+    ServerWebExchange decorate(ServerWebExchange exchange) {
+        var response = new ApplicationErrorConveyorHttpResponse(exchange.getResponse());
+        exchange = exchange.mutate().response(response).build();
+        return exchange;
+    }
+
+    boolean canFilter(ServerHttpRequest request) {
+        return methodIsIdempotent(request.getMethod()) && acceptsHtml(request);
+    }
+
+    boolean methodIsIdempotent(HttpMethod method) {
+        return switch (method) {
+        case GET, HEAD, OPTIONS, TRACE -> true;
+        default -> false;
+        };
+    }
 
+    boolean acceptsHtml(ServerHttpRequest request) {
+        return request.getHeaders().getAccept().stream().anyMatch(MediaType.TEXT_HTML::isCompatibleWith);
     }
 
     /**
      * A response decorator that throws a {@link ResponseStatusException} at
-     * {@link #setStatusCode(HttpStatus)} if the status code is an error code, thus
-     * letting the gateway render the appropriate custom error page instead of the
-     * original application response body.
+     * {@link #beforeCommit} if the status code is an error code, thus letting the
+     * gateway render the appropriate custom error page instead of the original
+     * application response body.
      */
     private static class ApplicationErrorConveyorHttpResponse extends ServerHttpResponseDecorator {
 
@@ -111,12 +142,14 @@ public ApplicationErrorConveyorHttpResponse(ServerHttpResponse delegate) {
         }
 
         @Override
-        public boolean setStatusCode(@Nullable HttpStatus status) {
-            checkStatusCode(status);
-            return super.setStatusCode(status);
+        public void beforeCommit(Supplier<? extends Mono<Void>> action) {
+            Mono<Void> checkStatus = Mono.fromRunnable(this::checkStatusCode);
+            Mono<Void> checkedAction = checkStatus.then(Mono.fromRunnable(action::get));
+            super.beforeCommit(() -> checkedAction);
         }
 
-        private void checkStatusCode(HttpStatus statusCode) {
+        private void checkStatusCode() {
+            HttpStatus statusCode = getStatusCode();
             log.debug("native status code: {}", statusCode);
             if (statusCode.is4xxClientError() || statusCode.is5xxServerError()) {
                 log.debug("Conveying {} response status", statusCode);