Merge pull request wildfly#17466 from pferraro/WFLY-18813

WFLY-18813 Fix gzip and mod_cluster filter creation regression
khermano · Jan 12, 2024 · 6dcb885 · 6dcb885
2 parents eb950e5 + 01fc5e2
commit 6dcb885
Show file tree

Hide file tree

Showing 16 changed files with 116 additions and 54 deletions.
diff --git a/undertow/src/main/java/org/wildfly/extension/undertow/filters/BasicAuthHandler.java b/undertow/src/main/java/org/wildfly/extension/undertow/filters/BasicAuthHandler.java
@@ -9,7 +9,7 @@
 import java.util.Collections;
 
 import io.undertow.security.handlers.AuthenticationCallHandler;
-import io.undertow.server.HandlerWrapper;
+
 import org.jboss.as.controller.AttributeDefinition;
 import org.jboss.as.controller.OperationContext;
 import org.jboss.as.controller.PathElement;
@@ -41,7 +41,7 @@ public Collection<AttributeDefinition> getAttributes() {
         return Collections.singleton(SECURITY_DOMAIN);
     }
 
-    static HandlerWrapper createHandlerWrapper(OperationContext context, ModelNode model) {
-        return AuthenticationCallHandler::new;
+    static PredicateHandlerWrapper createHandlerWrapper(OperationContext context, ModelNode model) {
+        return PredicateHandlerWrapper.filter(AuthenticationCallHandler::new);
     }
 }
diff --git a/undertow/src/main/java/org/wildfly/extension/undertow/filters/CustomFilterDefinition.java b/undertow/src/main/java/org/wildfly/extension/undertow/filters/CustomFilterDefinition.java
@@ -65,19 +65,19 @@ public Collection<AttributeDefinition> getAttributes() {
         return ATTRIBUTES;
     }
 
-    static HandlerWrapper createHandlerWrapper(OperationContext context, ModelNode model) throws OperationFailedException {
+    static PredicateHandlerWrapper createHandlerWrapper(OperationContext context, ModelNode model) throws OperationFailedException {
         String className = CLASS_NAME.resolveModelAttribute(context, model).asString();
         String moduleName = MODULE.resolveModelAttribute(context, model).asString();
         Map<String, String> parameters = PARAMETERS.unwrap(context, model);
         UndertowLogger.ROOT_LOGGER.debugf("Creating http handler %s from module %s with parameters %s", className, moduleName, parameters);
         // Resolve module lazily
-        return new HandlerWrapper() {
+        return PredicateHandlerWrapper.filter(new HandlerWrapper() {
             @Override
             public HttpHandler wrap(HttpHandler handler) {
                 Class<?> handlerClass = getHandlerClass(className, moduleName);
                 return new ConfiguredHandlerWrapper(handlerClass, parameters).wrap(handler);
             }
-        };
+        });
     }
 
     private static Class<?> getHandlerClass(String className, String moduleName) {

diff --git a/undertow/src/main/java/org/wildfly/extension/undertow/filters/ErrorPageDefinition.java b/undertow/src/main/java/org/wildfly/extension/undertow/filters/ErrorPageDefinition.java
@@ -10,6 +10,7 @@
 import java.util.List;
 
 import io.undertow.server.HandlerWrapper;
+import io.undertow.server.HttpHandler;
 import io.undertow.server.handlers.error.FileErrorPageHandler;
 
 import org.jboss.as.controller.AttributeDefinition;
@@ -48,9 +49,14 @@ public Collection<AttributeDefinition> getAttributes() {
         return ATTRIBUTES;
     }
 
-    static HandlerWrapper createHandlerWrapper(OperationContext context, ModelNode model) throws OperationFailedException {
+    static PredicateHandlerWrapper createHandlerWrapper(OperationContext context, ModelNode model) throws OperationFailedException {
         int code = CODE.resolveModelAttribute(context, model).asInt();
         String path = PATH.resolveModelAttribute(context, model).asStringOrNull();
-        return next -> new FileErrorPageHandler(next, Paths.get(path), code);
+        return PredicateHandlerWrapper.filter(new HandlerWrapper() {
+            @Override
+            public HttpHandler wrap(HttpHandler next) {
+                return new FileErrorPageHandler(next, Paths.get(path), code);
+            }
+        });
     }
 }
diff --git a/...rtow/src/main/java/org/wildfly/extension/undertow/filters/ExpressionFilterDefinition.java b/...rtow/src/main/java/org/wildfly/extension/undertow/filters/ExpressionFilterDefinition.java
@@ -10,6 +10,7 @@
 
 import io.undertow.Handlers;
 import io.undertow.server.HandlerWrapper;
+import io.undertow.server.HttpHandler;
 import io.undertow.server.handlers.builder.PredicatedHandler;
 import io.undertow.server.handlers.builder.PredicatedHandlersParser;
 
@@ -54,7 +55,7 @@ public Collection<AttributeDefinition> getAttributes() {
         return ATTRIBUTES;
     }
 
-    static HandlerWrapper createHandlerWrapper(OperationContext context, ModelNode model) throws OperationFailedException {
+    static PredicateHandlerWrapper createHandlerWrapper(OperationContext context, ModelNode model) throws OperationFailedException {
         String expression = EXPRESSION.resolveModelAttribute(context, model).asString();
         String moduleName = MODULE.resolveModelAttribute(context, model).asStringOrNull();
         ClassLoader loader = ExpressionFilterDefinition.class.getClassLoader();
@@ -71,6 +72,11 @@ static HandlerWrapper createHandlerWrapper(OperationContext context, ModelNode m
         List<PredicatedHandler> handlers = PredicatedHandlersParser.parse(expression, loader);
         UndertowLogger.ROOT_LOGGER.debugf("Creating http handler %s from module %s", expression, moduleName);
 
-        return next -> Handlers.predicates(handlers, next);
+        return PredicateHandlerWrapper.filter(new HandlerWrapper() {
+            @Override
+            public HttpHandler wrap(HttpHandler next) {
+                return Handlers.predicates(handlers, next);
+            }
+        });
     }
 }
diff --git a/undertow/src/main/java/org/wildfly/extension/undertow/filters/FilterAdd.java b/undertow/src/main/java/org/wildfly/extension/undertow/filters/FilterAdd.java
@@ -19,17 +19,15 @@
 import java.util.Collection;
 import java.util.function.Consumer;
 
-import io.undertow.server.HandlerWrapper;
-
 /**
  * @author Tomaz Cerar (c) 2013 Red Hat Inc.
  * @author <a href="mailto:[email protected]">Richard Opalka</a>
  */
 class FilterAdd extends AbstractAddStepHandler {
 
-    private HandlerWrapperFactory factory;
+    private PredicateHandlerWrapperFactory factory;
 
-    FilterAdd(HandlerWrapperFactory factory, Collection<AttributeDefinition> attributes) {
+    FilterAdd(PredicateHandlerWrapperFactory factory, Collection<AttributeDefinition> attributes) {
         super(attributes);
         this.factory = factory;
     }
@@ -39,8 +37,8 @@ protected void performRuntime(OperationContext context, ModelNode operation, Mod
         final String name = context.getCurrentAddressValue();
         final ServiceTarget target = context.getServiceTarget();
         final ServiceBuilder<?> sb = target.addService(UndertowService.FILTER.append(name));
-        final Consumer<HandlerWrapper> serviceConsumer = sb.provides(UndertowService.FILTER.append(name));
-        HandlerWrapper wrapper = this.factory.createHandlerWrapper(context, model);
+        final Consumer<PredicateHandlerWrapper> serviceConsumer = sb.provides(UndertowService.FILTER.append(name));
+        PredicateHandlerWrapper wrapper = this.factory.createHandlerWrapper(context, model);
         sb.setInstance(Service.newInstance(serviceConsumer, wrapper));
         sb.setInitialMode(ServiceController.Mode.ON_DEMAND);
         sb.install();

diff --git a/undertow/src/main/java/org/wildfly/extension/undertow/filters/FilterRefDefinition.java b/undertow/src/main/java/org/wildfly/extension/undertow/filters/FilterRefDefinition.java
@@ -12,7 +12,6 @@
 
 import io.undertow.predicate.Predicate;
 import io.undertow.predicate.PredicateParser;
-import io.undertow.server.HandlerWrapper;
 
 import org.jboss.as.controller.AbstractAddStepHandler;
 import org.jboss.as.controller.AttributeDefinition;
@@ -116,7 +115,7 @@ protected void performRuntime(OperationContext context, ModelNode operation, Mod
             final ServiceName sn = UndertowService.getFilterRefServiceName(address, name);
             final ServiceBuilder<?> sb = target.addService(sn);
             final Consumer<UndertowFilter> frConsumer = sb.provides(sn);
-            final Supplier<HandlerWrapper> fSupplier = sb.requires(UndertowService.FILTER.append(name));
+            final Supplier<PredicateHandlerWrapper> fSupplier = sb.requires(UndertowService.FILTER.append(name));
             final Supplier<FilterLocation> lSupplier = sb.requires(locationSN);
             sb.setInstance(new FilterService(frConsumer, fSupplier, lSupplier, predicate, priority));
             sb.install();

diff --git a/undertow/src/main/java/org/wildfly/extension/undertow/filters/FilterService.java b/undertow/src/main/java/org/wildfly/extension/undertow/filters/FilterService.java
@@ -8,9 +8,7 @@
 import java.util.function.Consumer;
 import java.util.function.Supplier;
 
-import io.undertow.Handlers;
 import io.undertow.predicate.Predicate;
-import io.undertow.server.HandlerWrapper;
 import io.undertow.server.HttpHandler;
 import org.jboss.msc.service.Service;
 import org.jboss.msc.service.StartContext;
@@ -25,12 +23,12 @@
  */
 public class FilterService implements Service<UndertowFilter>, UndertowFilter {
     private final Consumer<UndertowFilter> serviceConsumer;
-    private final Supplier<HandlerWrapper> wrapper;
+    private final Supplier<PredicateHandlerWrapper> wrapper;
     private final Supplier<FilterLocation> location;
     private final Predicate predicate;
     private final int priority;
 
-    FilterService(final Consumer<UndertowFilter> serviceConsumer, final Supplier<HandlerWrapper> wrapper, final Supplier<FilterLocation> location, final Predicate predicate, final int priority) {
+    FilterService(final Consumer<UndertowFilter> serviceConsumer, final Supplier<PredicateHandlerWrapper> wrapper, final Supplier<FilterLocation> location, final Predicate predicate, final int priority) {
         this.serviceConsumer = serviceConsumer;
         this.wrapper = wrapper;
         this.location = location;
@@ -62,7 +60,6 @@ public UndertowFilter getValue() {
 
     @Override
     public HttpHandler wrap(HttpHandler next) {
-        HttpHandler handler = this.wrapper.get().wrap(next);
-        return (this.predicate != null) ? Handlers.predicate(this.predicate, handler, next) : handler;
+        return this.wrapper.get().wrap(this.predicate, next);
     }
 }
diff --git a/undertow/src/main/java/org/wildfly/extension/undertow/filters/GzipFilterDefinition.java b/undertow/src/main/java/org/wildfly/extension/undertow/filters/GzipFilterDefinition.java
@@ -5,7 +5,9 @@
 
 package org.wildfly.extension.undertow.filters;
 
-import io.undertow.server.HandlerWrapper;
+import io.undertow.predicate.Predicate;
+import io.undertow.predicate.Predicates;
+import io.undertow.server.HttpHandler;
 import io.undertow.server.handlers.encoding.ContentEncodingRepository;
 import io.undertow.server.handlers.encoding.EncodingHandler;
 import io.undertow.server.handlers.encoding.GzipEncodingProvider;
@@ -24,8 +26,13 @@ public class GzipFilterDefinition extends SimpleFilterDefinition {
         super(PATH_ELEMENT, GzipFilterDefinition::createHandlerWrapper);
     }
 
-    static HandlerWrapper createHandlerWrapper(OperationContext context, ModelNode model) {
-        ContentEncodingRepository repository = new ContentEncodingRepository().addEncodingHandler("gzip", new GzipEncodingProvider(), 50);
-        return next -> new EncodingHandler(next, repository);
+    static PredicateHandlerWrapper createHandlerWrapper(OperationContext context, ModelNode model) {
+        return new PredicateHandlerWrapper() {
+            @Override
+            public HttpHandler wrap(Predicate predicate, HttpHandler next) {
+                ContentEncodingRepository repository = new ContentEncodingRepository().addEncodingHandler("gzip", new GzipEncodingProvider(), 50, predicate != null ? predicate : Predicates.truePredicate());
+                return new EncodingHandler(next, repository);
+            }
+        };
     }
 }
diff --git a/undertow/src/main/java/org/wildfly/extension/undertow/filters/ModClusterDefinition.java b/undertow/src/main/java/org/wildfly/extension/undertow/filters/ModClusterDefinition.java
@@ -355,11 +355,11 @@ public void installServices(OperationContext context, ModelNode model) throws Op
 
             RuntimeCapability<Void> capability = ModClusterDefinition.Capability.MOD_CLUSTER_FILTER_CAPABILITY.getDefinition();
             CapabilityServiceBuilder<?> builder = context.getCapabilityServiceTarget().addCapability(capability);
-            Consumer<HandlerWrapper> filter = builder.provides(capability, UndertowService.FILTER.append(context.getCurrentAddressValue()));
+            Consumer<PredicateHandlerWrapper> filter = builder.provides(capability, UndertowService.FILTER.append(context.getCurrentAddressValue()));
             Supplier<ModCluster> serviceRequirement = builder.requires(configurator.getServiceName());
             Supplier<MCMPConfig> configRequirement = builder.requires(configurator.getConfigServiceName());
 
-            HandlerWrapper wrapper = new HandlerWrapper() {
+            PredicateHandlerWrapper wrapper = PredicateHandlerWrapper.filter(new HandlerWrapper() {
                 @Override
                 public HttpHandler wrap(HttpHandler next) {
                     ModCluster modCluster = serviceRequirement.get();
@@ -369,8 +369,8 @@ public HttpHandler wrap(HttpHandler next) {
                     //to specify the next handler. To get around this we invoke the mod_proxy handler
                     //and then if it has not dispatched or handled the request then we know that we can
                     //just pass it on to the next handler
-                    final HttpHandler proxyHandler = modCluster.createProxyHandler(next);
-                    final HttpHandler realNext = new HttpHandler() {
+                    HttpHandler proxyHandler = modCluster.createProxyHandler(next);
+                    HttpHandler realNext = new HttpHandler() {
                         @Override
                         public void handleRequest(HttpServerExchange exchange) throws Exception {
                             proxyHandler.handleRequest(exchange);
@@ -380,9 +380,10 @@ public void handleRequest(HttpServerExchange exchange) throws Exception {
                             }
                         }
                     };
-                    return (managementAccessPredicate != null)  ? Handlers.predicate(managementAccessPredicate, config.create(modCluster, realNext), next)  :  config.create(modCluster, realNext);
+                    HttpHandler mcmpHandler = config.create(modCluster, realNext);
+                    return (managementAccessPredicate != null) ? Handlers.predicate(managementAccessPredicate, mcmpHandler, next) : mcmpHandler;
                 }
-            };
+            });
 
             builder.setInstance(Service.newInstance(filter, wrapper)).setInitialMode(ServiceController.Mode.ON_DEMAND).install();
 

diff --git a/undertow/src/main/java/org/wildfly/extension/undertow/filters/PredicateHandlerWrapper.java b/undertow/src/main/java/org/wildfly/extension/undertow/filters/PredicateHandlerWrapper.java
@@ -0,0 +1,30 @@
+/*
+ * Copyright The WildFly Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package org.wildfly.extension.undertow.filters;
+
+import io.undertow.Handlers;
+import io.undertow.predicate.Predicate;
+import io.undertow.server.HandlerWrapper;
+import io.undertow.server.HttpHandler;
+
+/**
+ * Like {@link io.undertow.server.HandlerWrapper}, but for filters.
+ * @author Paul Ferraro
+ */
+public interface PredicateHandlerWrapper {
+
+    HttpHandler wrap(Predicate predicate, HttpHandler next);
+
+    static PredicateHandlerWrapper filter(HandlerWrapper wrapper) {
+        return new PredicateHandlerWrapper() {
+            @Override
+            public HttpHandler wrap(Predicate predicate, HttpHandler next) {
+                HttpHandler handler = wrapper.wrap(next);
+                return (predicate != null) ? Handlers.predicate(predicate, handler, next) : handler;
+            }
+        };
+    }
+}
diff --git a/...dertow/filters/HandlerWrapperFactory.java → ...lters/PredicateHandlerWrapperFactory.java b/...dertow/filters/HandlerWrapperFactory.java → ...lters/PredicateHandlerWrapperFactory.java
@@ -5,16 +5,15 @@
 
 package org.wildfly.extension.undertow.filters;
 
-import io.undertow.server.HandlerWrapper;
-
 import org.jboss.as.controller.OperationContext;
 import org.jboss.as.controller.OperationFailedException;
 import org.jboss.dmr.ModelNode;
 
 /**
- * Factory for creating a {@link HandlerWrapper}.
+ * Factory for creating a {@link PredicateHandlerWrapper}.
  * @author Paul Ferraro
  */
-public interface HandlerWrapperFactory {
-    HandlerWrapper createHandlerWrapper(OperationContext context, ModelNode model) throws OperationFailedException;
+public interface PredicateHandlerWrapperFactory {
+
+    PredicateHandlerWrapper createHandlerWrapper(OperationContext context, ModelNode model) throws OperationFailedException;
 }
diff --git a/...w/src/main/java/org/wildfly/extension/undertow/filters/RequestLimitHandlerDefinition.java b/...w/src/main/java/org/wildfly/extension/undertow/filters/RequestLimitHandlerDefinition.java
@@ -9,6 +9,7 @@
 import java.util.List;
 
 import io.undertow.server.HandlerWrapper;
+import io.undertow.server.HttpHandler;
 import io.undertow.server.handlers.RequestLimitingHandler;
 
 import org.jboss.as.controller.AttributeDefinition;
@@ -53,9 +54,14 @@ public Collection<AttributeDefinition> getAttributes() {
         return ATTRIBUTES;
     }
 
-    static HandlerWrapper createHandlerWrapper(OperationContext context, ModelNode model) throws OperationFailedException {
+    static PredicateHandlerWrapper createHandlerWrapper(OperationContext context, ModelNode model) throws OperationFailedException {
         int maxConcurrentRequests = MAX_CONCURRENT_REQUESTS.resolveModelAttribute(context, model).asInt();
         int queueSize = QUEUE_SIZE.resolveModelAttribute(context, model).asInt();
-        return next -> new RequestLimitingHandler(maxConcurrentRequests, queueSize, next);
+        return PredicateHandlerWrapper.filter(new HandlerWrapper() {
+            @Override
+            public HttpHandler wrap(HttpHandler next) {
+                return new RequestLimitingHandler(maxConcurrentRequests, queueSize, next);
+            }
+        });
     }
 }
diff --git a/.../src/main/java/org/wildfly/extension/undertow/filters/ResponseHeaderFilterDefinition.java b/.../src/main/java/org/wildfly/extension/undertow/filters/ResponseHeaderFilterDefinition.java
@@ -9,6 +9,7 @@
 import java.util.List;
 
 import io.undertow.server.HandlerWrapper;
+import io.undertow.server.HttpHandler;
 import io.undertow.server.handlers.SetHeaderHandler;
 
 import org.jboss.as.controller.AttributeDefinition;
@@ -47,9 +48,14 @@ public Collection<AttributeDefinition> getAttributes() {
         return ATTRIBUTES;
     }
 
-    static HandlerWrapper createHandlerWrapper(OperationContext context, ModelNode model) throws OperationFailedException {
+    static PredicateHandlerWrapper createHandlerWrapper(OperationContext context, ModelNode model) throws OperationFailedException {
         String name = NAME.resolveModelAttribute(context, model).asString();
         String value = VALUE.resolveModelAttribute(context, model).asString();
-        return next -> new SetHeaderHandler(next, name, value);
+        return PredicateHandlerWrapper.filter(new HandlerWrapper() {
+            @Override
+            public HttpHandler wrap(HttpHandler next) {
+                return new SetHeaderHandler(next, name, value);
+            }
+        });
     }
 }
diff --git a/undertow/src/main/java/org/wildfly/extension/undertow/filters/RewriteFilterDefinition.java b/undertow/src/main/java/org/wildfly/extension/undertow/filters/RewriteFilterDefinition.java
@@ -10,6 +10,7 @@
 
 import io.undertow.attribute.ExchangeAttributes;
 import io.undertow.server.HandlerWrapper;
+import io.undertow.server.HttpHandler;
 import io.undertow.server.handlers.RedirectHandler;
 import io.undertow.server.handlers.SetAttributeHandler;
 
@@ -53,9 +54,14 @@ public Collection<AttributeDefinition> getAttributes() {
         return ATTRIBUTES;
     }
 
-    static HandlerWrapper createHandlerWrapper(OperationContext context, ModelNode model) throws OperationFailedException {
+    static PredicateHandlerWrapper createHandlerWrapper(OperationContext context, ModelNode model) throws OperationFailedException {
         String target = TARGET.resolveModelAttribute(context, model).asString();
         boolean redirect = REDIRECT.resolveModelAttribute(context, model).asBoolean();
-        return next -> redirect ? new RedirectHandler(target) : new SetAttributeHandler(next, ExchangeAttributes.relativePath(), ExchangeAttributes.parser(RewriteFilterDefinition.class.getClassLoader()).parse(target));
+        return PredicateHandlerWrapper.filter(new HandlerWrapper() {
+            @Override
+            public HttpHandler wrap(HttpHandler next) {
+                return redirect ? new RedirectHandler(target) : new SetAttributeHandler(next, ExchangeAttributes.relativePath(), ExchangeAttributes.parser(RewriteFilterDefinition.class.getClassLoader()).parse(target));
+            }
+        });
     }
 }
diff --git a/undertow/src/main/java/org/wildfly/extension/undertow/filters/SimpleFilterDefinition.java b/undertow/src/main/java/org/wildfly/extension/undertow/filters/SimpleFilterDefinition.java
@@ -16,9 +16,9 @@
  */
 abstract class SimpleFilterDefinition extends AbstractFilterDefinition {
 
-    private final HandlerWrapperFactory factory;
+    private final PredicateHandlerWrapperFactory factory;
 
-    protected SimpleFilterDefinition(PathElement path, HandlerWrapperFactory factory) {
+    protected SimpleFilterDefinition(PathElement path, PredicateHandlerWrapperFactory factory) {
         super(path);
         this.factory = factory;
     }