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

WW-5406 Ensure Action excluded patterns are reinjected
This commit is contained in:
Kusal Kithul-Godage
2024-04-11 14:27:31 +10:00
committed by GitHub
8 changed files with 86 additions and 131 deletions
@@ -78,6 +78,7 @@ import javax.servlet.http.HttpServletRequest;
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.Set;
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 class Dispatcher {
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;
@@ -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 class InitOperations {
* @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();
}
}
@@ -223,21 +223,23 @@ public class PrepareOperations {
* 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());
}
/**
@@ -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 class StrutsPrepareAndExecuteFilter implements StrutsStatics, Filter {
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 class StrutsPrepareAndExecuteFilter implements StrutsStatics, Filter {
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 {
@@ -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 class StrutsPrepareFilter implements StrutsStatics, Filter {
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 class StrutsPrepareFilter implements StrutsStatics, Filter {
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);
@@ -54,10 +54,12 @@ import java.util.HashSet;
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 class DispatcherTest extends StrutsJUnit4InternalTestCase {
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());
@@ -1,86 +0,0 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.struts2.dispatcher;
import com.opensymphony.xwork2.config.ConfigurationException;
import com.opensymphony.xwork2.inject.ContainerBuilder;
import com.opensymphony.xwork2.util.location.LocatableProperties;
import org.apache.struts2.StrutsConstants;
import org.apache.struts2.StrutsInternalTestCase;
import org.apache.struts2.config.PropertiesConfigurationProvider;
import java.util.List;
import java.util.regex.Pattern;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
public class InitOperationsTest extends StrutsInternalTestCase {
public void testExcludePatterns() {
// given
loadConfigurationProviders(new PropertiesConfigurationProvider() {
@Override
public void register(ContainerBuilder builder, LocatableProperties props) throws ConfigurationException {
props.setProperty(StrutsConstants.STRUTS_ACTION_EXCLUDE_PATTERN, "/ns1/.*\\.json,/ns2/.*\\.json");
}
});
Dispatcher mockDispatcher = mock(Dispatcher.class);
when(mockDispatcher.getContainer()).thenReturn(container);
// when
InitOperations init = new InitOperations();
List<Pattern> patterns = init.buildExcludedPatternsList(mockDispatcher);
// then
assertThat(patterns).extracting(Pattern::toString).containsOnly(
"/ns1/.*\\.json",
"/ns2/.*\\.json"
);
}
public void testExcludePatternsUsingCustomSeparator() {
// given
loadConfigurationProviders(new PropertiesConfigurationProvider() {
@Override
public void register(ContainerBuilder builder, LocatableProperties props) throws ConfigurationException {
props.setProperty(StrutsConstants.STRUTS_ACTION_EXCLUDE_PATTERN, "/ns1/[a-z]{1,10}.json///ns2/[a-z]{1,10}.json");
props.setProperty(StrutsConstants.STRUTS_ACTION_EXCLUDE_PATTERN_SEPARATOR, "//");
}
});
Dispatcher mockDispatcher = mock(Dispatcher.class);
when(mockDispatcher.getContainer()).thenReturn(container);
// when
InitOperations init = new InitOperations();
String separator = container.getInstance(String.class, StrutsConstants.STRUTS_ACTION_EXCLUDE_PATTERN_SEPARATOR);
List<Pattern> patterns = init.buildExcludedPatternsList(mockDispatcher);
// then
assertThat(separator).isNotBlank().isEqualTo("//");
assertThat(patterns).extracting(Pattern::toString).containsOnly(
"/ns1/[a-z]{1,10}.json",
"/ns2/[a-z]{1,10}.json"
);
}
}
@@ -26,13 +26,10 @@ import org.springframework.mock.web.MockFilterConfig;
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 class StrutsPrepareAndExecuteFilterIntegrationTest {
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 class StrutsPrepareAndExecuteFilterIntegrationTest {
};
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());