Merge pull request #910 from apache/WW-5406-excluded-patterns

WW-5406 Ensure Action excluded patterns are reinjected

core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java

@@ -78,6 +78,7 @@
 import javax.servlet.http.HttpServletResponse;
 import java.io.File;
 import java.io.IOException;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.HashSet;
 import java.util.List;
@@ -88,6 +89,10 @@
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.regex.Pattern;
 
+import static java.util.Collections.emptyList;
+import static java.util.Collections.unmodifiableList;
+import static java.util.stream.Collectors.toList;
+
 /**
  * A utility class the actual dispatcher delegates most of its tasks to. Each instance
  * of the primary dispatcher holds an instance of this dispatcher to be shared for
@@ -162,6 +167,9 @@ public class Dispatcher {
      */
     private Pattern multipartValidationPattern = Pattern.compile(MULTIPART_FORM_DATA_REGEX);
 
+    private String actionExcludedPatternsSeparator = ",";
+    private List<Pattern> actionExcludedPatterns = emptyList();
+
     /**
      * Provide list of default configuration files.
      */
@@ -340,6 +348,27 @@ public void setMultipartValidationRegex(String multipartValidationRegex) {
         this.multipartValidationPattern = Pattern.compile(multipartValidationRegex);
     }
 
+    @Inject(value = StrutsConstants.STRUTS_ACTION_EXCLUDE_PATTERN_SEPARATOR, required = false)
+    public void setActionExcludedPatternsSeparator(String separator) {
+        this.actionExcludedPatternsSeparator = separator;
+    }
+
+    @Inject(value = StrutsConstants.STRUTS_ACTION_EXCLUDE_PATTERN, required = false)
+    public void setActionExcludedPatterns(String excludedPatterns) {
+        this.actionExcludedPatterns = buildExcludedPatternsList(excludedPatterns, actionExcludedPatternsSeparator);
+    }
+
+    private static List<Pattern> buildExcludedPatternsList(String patterns, String separator) {
+        if (patterns == null || patterns.trim().isEmpty()) {
+            return emptyList();
+        }
+        return unmodifiableList(Arrays.stream(patterns.split(separator)).map(String::trim).map(Pattern::compile).collect(toList()));
+    }
+
+    public List<Pattern> getActionExcludedPatterns() {
+        return actionExcludedPatterns;
+    }
+
     @Inject
     public void setValueStackFactory(ValueStackFactory valueStackFactory) {
         this.valueStackFactory = valueStackFactory;

core/src/main/java/org/apache/struts2/dispatcher/InitOperations.java

@@ -19,10 +19,7 @@
 package org.apache.struts2.dispatcher;
 
 import com.opensymphony.xwork2.ActionContext;
-import org.apache.struts2.StrutsConstants;
 
-import java.util.ArrayList;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
@@ -100,27 +97,11 @@ public void cleanup() {
      * @param dispatcher The dispatcher to check for exclude pattern configuration
      * @return a List of Patterns for request to exclude if apply, or <tt>null</tt>
      * @see org.apache.struts2.StrutsConstants#STRUTS_ACTION_EXCLUDE_PATTERN
+     * @deprecated since 6.4.0, use {@link Dispatcher#getActionExcludedPatterns()} instead.
      */
+    @Deprecated
     public List<Pattern> buildExcludedPatternsList(Dispatcher dispatcher) {
-        String excludePatterns = dispatcher.getContainer().getInstance(String.class, StrutsConstants.STRUTS_ACTION_EXCLUDE_PATTERN);
-        String separator = dispatcher.getContainer().getInstance(String.class, StrutsConstants.STRUTS_ACTION_EXCLUDE_PATTERN_SEPARATOR);
-        if (separator == null) {
-            separator = ",";
-        }
-        return buildExcludedPatternsList(excludePatterns, separator);
-    }
-
-    private List<Pattern> buildExcludedPatternsList(String patterns, String separator) {
-        if (null != patterns && patterns.trim().length() != 0) {
-            List<Pattern> list = new ArrayList<>();
-            String[] tokens = patterns.split(separator);
-            for (String token : tokens) {
-                list.add(Pattern.compile(token.trim()));
-            }
-            return Collections.unmodifiableList(list);
-        } else {
-            return null;
-        }
+        return dispatcher.getActionExcludedPatterns();
     }
 
 }

core/src/main/java/org/apache/struts2/dispatcher/PrepareOperations.java

@@ -223,21 +223,23 @@ public void cleanupDispatcher() {
      * Check whether the request matches a list of exclude patterns.
      *
      * @param request          The request to check patterns against
-     * @param excludedPatterns list of patterns for exclusion
-     *
      * @return <tt>true</tt> if the request URI matches one of the given patterns
      */
+    public boolean isUrlExcluded(HttpServletRequest request) {
+        String uri = RequestUtils.getUri(request);
+        return dispatcher.getActionExcludedPatterns().stream().anyMatch(pattern -> pattern.matcher(uri).matches());
+    }
+
+    /**
+     * @deprecated since 6.4.0, use {@link #isUrlExcluded(HttpServletRequest)} instead.
+     */
+    @Deprecated
     public boolean isUrlExcluded(HttpServletRequest request, List<Pattern> excludedPatterns) {
         if (excludedPatterns == null) {
             return false;
         }
         String uri = RequestUtils.getUri(request);
-        for (Pattern pattern : excludedPatterns) {
-            if (pattern.matcher(uri).matches()) {
-                return true;
-            }
-        }
-        return false;
+        return excludedPatterns.stream().anyMatch(pattern -> pattern.matcher(uri).matches());
     }
 
     /**

core/src/main/java/org/apache/struts2/dispatcher/filter/StrutsPrepareAndExecuteFilter.java

@@ -50,6 +50,12 @@ public class StrutsPrepareAndExecuteFilter implements StrutsStatics, Filter {
 
     protected PrepareOperations prepare;
     protected ExecuteOperations execute;
+
+    /**
+     * @deprecated since 6.4.0, use {@link Dispatcher#getActionExcludedPatterns} or
+     * {@link PrepareOperations#isUrlExcluded(HttpServletRequest)} instead.
+     */
+    @Deprecated
     protected List<Pattern> excludedPatterns;
 
     public void init(FilterConfig filterConfig) throws ServletException {
@@ -62,7 +68,7 @@ public void init(FilterConfig filterConfig) throws ServletException {
 
             prepare = createPrepareOperations(dispatcher);
             execute = createExecuteOperations(dispatcher);
-            // Note: Currently, excluded patterns are not refreshed following an XWork config reload
+
             this.excludedPatterns = init.buildExcludedPatternsList(dispatcher);
 
             postInit(dispatcher, filterConfig);
@@ -121,7 +127,7 @@ public void doFilter(ServletRequest req, ServletResponse res, FilterChain chain)
         try {
             prepare.trackRecursion(request);
             String uri = RequestUtils.getUri(request);
-            if (prepare.isUrlExcluded(request, excludedPatterns)) {
+            if (prepare.isUrlExcluded(request)) {
                 LOG.trace("Request: {} is excluded from handling by Struts, passing request to other filters", uri);
                 chain.doFilter(request, response);
             } else {

core/src/main/java/org/apache/struts2/dispatcher/filter/StrutsPrepareFilter.java

@@ -43,6 +43,12 @@ public class StrutsPrepareFilter implements StrutsStatics, Filter {
     protected static final String REQUEST_EXCLUDED_FROM_ACTION_MAPPING = StrutsPrepareFilter.class.getName() + ".REQUEST_EXCLUDED_FROM_ACTION_MAPPING";
 
     protected PrepareOperations prepare;
+
+    /**
+     * @deprecated since 6.4.0, use {@link Dispatcher#getActionExcludedPatterns} or
+     * {@link PrepareOperations#isUrlExcluded(HttpServletRequest)} instead.
+     */
+    @Deprecated
     protected List<Pattern> excludedPatterns;
 
     public void init(FilterConfig filterConfig) throws ServletException {
@@ -53,7 +59,7 @@ public void init(FilterConfig filterConfig) throws ServletException {
             dispatcher = init.initDispatcher(config);
 
             prepare = createPrepareOperations(dispatcher);
-            // Note: Currently, excluded patterns are not refreshed following an XWork config reload
+
             this.excludedPatterns = init.buildExcludedPatternsList(dispatcher);
 
             postInit(dispatcher, filterConfig);
@@ -102,7 +108,7 @@ public void doFilter(ServletRequest req, ServletResponse res, FilterChain chain)
         boolean didWrap = false;
         try {
             prepare.trackRecursion(request);
-            if (prepare.isUrlExcluded(request, excludedPatterns)) {
+            if (prepare.isUrlExcluded(request)) {
                 request.setAttribute(REQUEST_EXCLUDED_FROM_ACTION_MAPPING, true);
             } else {
                 request.setAttribute(REQUEST_EXCLUDED_FROM_ACTION_MAPPING, false);

core/src/test/java/org/apache/struts2/dispatcher/DispatcherTest.java

@@ -54,10 +54,12 @@
 import java.util.Locale;
 import java.util.Map;
 import java.util.Set;
+import java.util.regex.Pattern;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static java.util.Collections.emptyMap;
 import static java.util.Collections.singletonMap;
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
@@ -588,6 +590,30 @@ public void register(ContainerBuilder builder,
         assertEquals(Locale.CANADA_FRENCH, dispatcher.getLocale(request));
     }
 
+    @Test
+    public void testExcludePatterns() {
+        initDispatcher(singletonMap(StrutsConstants.STRUTS_ACTION_EXCLUDE_PATTERN, "/ns1/.*\\.json,/ns2/.*\\.json"));
+
+        assertThat(dispatcher.getActionExcludedPatterns()).extracting(Pattern::toString).containsOnly(
+                "/ns1/.*\\.json",
+                "/ns2/.*\\.json"
+        );
+    }
+
+    @Test
+    public void testExcludePatternsUsingCustomSeparator() {
+        Map<String, String> props = new HashMap<>();
+        props.put(StrutsConstants.STRUTS_ACTION_EXCLUDE_PATTERN, "/ns1/[a-z]{1,10}.json///ns2/[a-z]{1,10}.json");
+        props.put(StrutsConstants.STRUTS_ACTION_EXCLUDE_PATTERN_SEPARATOR, "//");
+
+        initDispatcher(props);
+
+        assertThat(dispatcher.getActionExcludedPatterns()).extracting(Pattern::toString).containsOnly(
+                "/ns1/[a-z]{1,10}.json",
+                "/ns2/[a-z]{1,10}.json"
+        );
+    }
+
     public static Dispatcher spyDispatcherWithConfigurationManager(Dispatcher dispatcher, ConfigurationManager configurationManager) {
         Dispatcher spiedDispatcher = spy(dispatcher);
         doReturn(configurationManager).when(spiedDispatcher).createConfigurationManager(any());

core/src/test/java/org/apache/struts2/dispatcher/StrutsPrepareAndExecuteFilterIntegrationTest.java

@@ -26,13 +26,10 @@
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.mock.web.MockHttpServletResponse;
 
-import javax.servlet.FilterConfig;
 import javax.servlet.ServletException;
 import javax.servlet.ServletRequest;
 import javax.servlet.ServletResponse;
 import java.io.IOException;
-import java.util.ArrayList;
-import java.util.regex.Pattern;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
@@ -127,6 +124,7 @@ public void testUriPatternExclusion() throws ServletException, IOException {
         MockHttpServletRequest request = new MockHttpServletRequest();
         MockHttpServletResponse response = new MockHttpServletResponse();
         MockFilterConfig filterConfig = new MockFilterConfig();
+        filterConfig.addInitParameter("struts.action.excludePattern", ".*hello.*");
         MockFilterChain filterChain = new MockFilterChain() {
             @Override
             public void doFilter(ServletRequest req, ServletResponse res) {
@@ -135,14 +133,7 @@ public void doFilter(ServletRequest req, ServletResponse res) {
         };
 
         request.setRequestURI("/hello.action");
-        StrutsPrepareAndExecuteFilter filter = new StrutsPrepareAndExecuteFilter() {
-            @Override
-            public void init( FilterConfig filterConfig ) throws ServletException {
-                super.init(filterConfig);
-                excludedPatterns = new ArrayList<>();
-                excludedPatterns.add(Pattern.compile(".*hello.*"));
-            }
-        };
+        StrutsPrepareAndExecuteFilter filter = new StrutsPrepareAndExecuteFilter();
         filter.init(filterConfig);
         filter.doFilter(request, response, filterChain);
         assertEquals(200, response.getStatus());