Merge pull request #667 from atlassian/WW-5292-operations

WW-5292 Allow overriding of Operations classes in two filter setup and assorted clean up
This commit is contained in:
Lukasz Lenart
2023-03-11 10:41:06 +01:00
committed by GitHub
8 changed files with 201 additions and 69 deletions
@@ -210,7 +210,7 @@ public class ConfigurationManager {
private boolean needReloadContainerProviders() {
Optional<ContainerProvider> provider = containerProviders.stream().filter(ContainerProvider::needsReload).findAny();
if (provider.isPresent()) {
LOG.info("Detected container provider [{}] needs to be reloaded.", provider);
LOG.info("Detected container provider [{}] needs to be reloaded.", provider.get());
return true;
}
return false;
@@ -229,13 +229,14 @@ public class PrepareOperations {
*
* @return <tt>true</tt> if the request URI matches one of the given patterns
*/
public boolean isUrlExcluded( HttpServletRequest request, List<Pattern> excludedPatterns ) {
if (excludedPatterns != null) {
String uri = RequestUtils.getUri(request);
for ( Pattern pattern : excludedPatterns ) {
if (pattern.matcher(uri).matches()) {
return true;
}
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;
@@ -20,12 +20,17 @@ package org.apache.struts2.dispatcher.filter;
import org.apache.struts2.StrutsStatics;
import org.apache.struts2.dispatcher.Dispatcher;
import org.apache.struts2.dispatcher.mapper.ActionMapping;
import org.apache.struts2.dispatcher.ExecuteOperations;
import org.apache.struts2.dispatcher.InitOperations;
import org.apache.struts2.dispatcher.PrepareOperations;
import org.apache.struts2.dispatcher.mapper.ActionMapping;
import javax.servlet.*;
import javax.servlet.Filter;
import javax.servlet.FilterChain;
import javax.servlet.FilterConfig;
import javax.servlet.ServletException;
import javax.servlet.ServletRequest;
import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import java.io.IOException;
@@ -46,14 +51,43 @@ public class StrutsExecuteFilter implements StrutsStatics, Filter {
protected synchronized void lazyInit() {
if (execute == null) {
InitOperations init = new InitOperations();
InitOperations init = createInitOperations();
Dispatcher dispatcher = init.findDispatcherOnThread();
init.initStaticContentLoader(new FilterHostConfig(filterConfig), dispatcher);
prepare = new PrepareOperations(dispatcher);
execute = new ExecuteOperations(dispatcher);
prepare = createPrepareOperations(dispatcher);
execute = createExecuteOperations(dispatcher);
}
}
/**
* Creates a new instance of {@link InitOperations} to be used during
* initialising {@link Dispatcher}
*
* @return instance of {@link InitOperations}
*/
protected InitOperations createInitOperations() {
return new InitOperations();
}
/**
* Creates a new instance of {@link PrepareOperations} to be used during
* initialising {@link Dispatcher}
*
* @return instance of {@link PrepareOperations}
*/
protected PrepareOperations createPrepareOperations(Dispatcher dispatcher) {
return new PrepareOperations(dispatcher);
}
/**
* Creates a new instance of {@link ExecuteOperations} to be used during
* initialising {@link Dispatcher}
*
* @return instance of {@link ExecuteOperations}
*/
protected ExecuteOperations createExecuteOperations(Dispatcher dispatcher) {
return new ExecuteOperations(dispatcher);
}
public void doFilter(ServletRequest req, ServletResponse res, FilterChain chain) throws IOException, ServletException {
@@ -50,7 +50,7 @@ public class StrutsPrepareAndExecuteFilter implements StrutsStatics, Filter {
protected PrepareOperations prepare;
protected ExecuteOperations execute;
protected List<Pattern> excludedPatterns = null;
protected List<Pattern> excludedPatterns;
public void init(FilterConfig filterConfig) throws ServletException {
InitOperations init = createInitOperations();
@@ -62,6 +62,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);
@@ -120,7 +121,7 @@ public class StrutsPrepareAndExecuteFilter implements StrutsStatics, Filter {
try {
prepare.trackRecursion(request);
String uri = RequestUtils.getUri(request);
if (isRequestExcluded(request)) {
if (prepare.isUrlExcluded(request, excludedPatterns)) {
LOG.trace("Request: {} is excluded from handling by Struts, passing request to other filters", uri);
chain.doFilter(request, response);
} else {
@@ -160,10 +161,6 @@ public class StrutsPrepareAndExecuteFilter implements StrutsStatics, Filter {
}
}
private boolean isRequestExcluded(HttpServletRequest request) {
return excludedPatterns != null && prepare.isUrlExcluded(request, excludedPatterns);
}
public void destroy() {
prepare.cleanupDispatcher();
}
@@ -43,16 +43,17 @@ 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;
protected List<Pattern> excludedPatterns = null;
protected List<Pattern> excludedPatterns;
public void init(FilterConfig filterConfig) throws ServletException {
InitOperations init = new InitOperations();
InitOperations init = createInitOperations();
Dispatcher dispatcher = null;
try {
FilterHostConfig config = new FilterHostConfig(filterConfig);
dispatcher = init.initDispatcher(config);
prepare = new PrepareOperations(dispatcher);
prepare = createPrepareOperations(dispatcher);
// Note: Currently, excluded patterns are not refreshed following an XWork config reload
this.excludedPatterns = init.buildExcludedPatternsList(dispatcher);
postInit(dispatcher, filterConfig);
@@ -64,6 +65,26 @@ public class StrutsPrepareFilter implements StrutsStatics, Filter {
}
}
/**
* Creates a new instance of {@link InitOperations} to be used during
* initialising {@link Dispatcher}
*
* @return instance of {@link InitOperations}
*/
protected InitOperations createInitOperations() {
return new InitOperations();
}
/**
* Creates a new instance of {@link PrepareOperations} to be used during
* initialising {@link Dispatcher}
*
* @return instance of {@link PrepareOperations}
*/
protected PrepareOperations createPrepareOperations(Dispatcher dispatcher) {
return new PrepareOperations(dispatcher);
}
/**
* Callback for post initialization
*
@@ -81,7 +102,7 @@ public class StrutsPrepareFilter implements StrutsStatics, Filter {
boolean didWrap = false;
try {
prepare.trackRecursion(request);
if (excludedPatterns != null && prepare.isUrlExcluded(request, excludedPatterns)) {
if (prepare.isUrlExcluded(request, excludedPatterns)) {
request.setAttribute(REQUEST_EXCLUDED_FROM_ACTION_MAPPING, true);
} else {
request.setAttribute(REQUEST_EXCLUDED_FROM_ACTION_MAPPING, false);
@@ -73,10 +73,12 @@ public class PrepareOperationsTest extends StrutsInternalTestCase {
});
IntStream.range(0, mockedRecursions - 1).forEach(i -> prepare.cleanupWrappedRequest(req));
// Assert org.apache.struts2.dispatcher.Dispatcher#cleanUpRequest has not yet run
assertNotNull(ContainerHolder.get());
prepare.cleanupWrappedRequest(req);
// Assert org.apache.struts2.dispatcher.Dispatcher#cleanUpRequest has run after final #cleanupWrappedRequest
assertNull(ContainerHolder.get());
}
}
@@ -19,28 +19,33 @@
package org.apache.struts2.dispatcher;
import com.opensymphony.xwork2.ActionContext;
import junit.framework.TestCase;
import org.apache.struts2.dispatcher.Dispatcher;
import org.apache.struts2.dispatcher.filter.StrutsPrepareAndExecuteFilter;
import org.junit.Test;
import org.springframework.mock.web.MockFilterChain;
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 javax.servlet.FilterConfig;
import javax.servlet.http.HttpServletRequest;
import java.io.IOException;
import java.util.regex.Pattern;
import java.util.ArrayList;
import java.util.regex.Pattern;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
/**
* Integration tests for the filter
*/
public class StrutsPrepareAndExecuteFilterIntegrationTest extends TestCase {
public class StrutsPrepareAndExecuteFilterIntegrationTest {
@Test
public void test404() throws ServletException, IOException {
MockHttpServletRequest request = new MockHttpServletRequest();
MockHttpServletResponse response = new MockHttpServletResponse();
@@ -61,6 +66,7 @@ public class StrutsPrepareAndExecuteFilterIntegrationTest extends TestCase {
assertNull(Dispatcher.getInstance());
}
@Test
public void test200() throws ServletException, IOException {
MockHttpServletRequest request = new MockHttpServletRequest();
MockHttpServletResponse response = new MockHttpServletResponse();
@@ -81,6 +87,7 @@ public class StrutsPrepareAndExecuteFilterIntegrationTest extends TestCase {
assertNull(Dispatcher.getInstance());
}
@Test
public void testActionMappingLookup() throws ServletException, IOException {
MockHttpServletRequest request = new MockHttpServletRequest();
MockHttpServletResponse response = new MockHttpServletResponse();
@@ -115,6 +122,7 @@ public class StrutsPrepareAndExecuteFilterIntegrationTest extends TestCase {
assertTrue((Boolean) request.getAttribute("__invoked"));
}
@Test
public void testUriPatternExclusion() throws ServletException, IOException {
MockHttpServletRequest request = new MockHttpServletRequest();
MockHttpServletResponse response = new MockHttpServletResponse();
@@ -131,7 +139,7 @@ public class StrutsPrepareAndExecuteFilterIntegrationTest extends TestCase {
@Override
public void init( FilterConfig filterConfig ) throws ServletException {
super.init(filterConfig);
excludedPatterns = new ArrayList<Pattern>();
excludedPatterns = new ArrayList<>();
excludedPatterns.add(Pattern.compile(".*hello.*"));
}
};
@@ -141,6 +149,7 @@ public class StrutsPrepareAndExecuteFilterIntegrationTest extends TestCase {
assertEquals("invoked", request.getAttribute("i_was"));
}
@Test
public void testStaticFallthrough() throws ServletException, IOException {
MockHttpServletRequest request = new MockHttpServletRequest();
MockHttpServletResponse response = new MockHttpServletResponse();
@@ -168,6 +177,7 @@ public class StrutsPrepareAndExecuteFilterIntegrationTest extends TestCase {
assertNull(Dispatcher.getInstance());
}
@Test
public void testStaticExecute() throws ServletException, IOException {
MockHttpServletRequest request = new MockHttpServletRequest();
MockHttpServletResponse response = new MockHttpServletResponse();
@@ -189,6 +199,7 @@ public class StrutsPrepareAndExecuteFilterIntegrationTest extends TestCase {
assertNull(Dispatcher.getInstance());
}
@Test
public void testDestroy() throws ServletException {
MockFilterConfig filterConfig = new MockFilterConfig();
final MockPrepareOperations[] prepareOperations = {null};
@@ -208,7 +219,7 @@ public class StrutsPrepareAndExecuteFilterIntegrationTest extends TestCase {
assertTrue(prepareOperations[0].isCleaned());
}
private class MockPrepareOperations extends PrepareOperations {
private static class MockPrepareOperations extends PrepareOperations {
private boolean cleaned;
public MockPrepareOperations(Dispatcher dispatcher) {
@@ -19,58 +19,68 @@
package org.apache.struts2.dispatcher;
import com.opensymphony.xwork2.ActionContext;
import junit.framework.TestCase;
import org.apache.struts2.dispatcher.Dispatcher;
import org.apache.struts2.dispatcher.PrepareOperations;
import org.apache.struts2.dispatcher.filter.StrutsExecuteFilter;
import org.apache.struts2.dispatcher.filter.StrutsPrepareFilter;
import org.springframework.mock.web.*;
import org.junit.Before;
import org.junit.Test;
import org.springframework.mock.web.MockFilterChain;
import org.springframework.mock.web.MockFilterConfig;
import org.springframework.mock.web.MockHttpServletRequest;
import org.springframework.mock.web.MockHttpServletResponse;
import javax.servlet.*;
import javax.servlet.Filter;
import javax.servlet.FilterChain;
import javax.servlet.FilterConfig;
import javax.servlet.ServletException;
import javax.servlet.ServletRequest;
import javax.servlet.ServletResponse;
import java.io.IOException;
import java.util.LinkedList;
import java.util.Arrays;
import java.util.LinkedList;
import java.util.Map;
import static java.util.Collections.emptyMap;
import static java.util.Collections.singletonMap;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
/**
* Integration tests for the filter
*/
public class TwoFilterIntegrationTest extends TestCase {
StrutsExecuteFilter filterExecute;
StrutsPrepareFilter filterPrepare;
Filter failFilter;
public class TwoFilterIntegrationTest {
private StrutsExecuteFilter filterExecute;
private StrutsPrepareFilter filterPrepare;
private Filter failFilter;
private Filter stringFilter;
@Before
public void setUp() {
filterPrepare = new StrutsPrepareFilter();
filterExecute = new StrutsExecuteFilter();
failFilter = new Filter() {
public void init(FilterConfig filterConfig) throws ServletException {}
public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException {
fail("Should never get here");
}
public void destroy() {}
};
stringFilter = new Filter() {
public void init(FilterConfig filterConfig) throws ServletException {}
public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException {
response.getWriter().write("content");
assertNotNull(ActionContext.getContext());
assertNotNull(Dispatcher.getInstance());
}
public void destroy() {}
};
failFilter = newFilter((req, res, chain) -> fail("Should never get here"));
stringFilter = newFilter((req, res, chain) -> {
res.getWriter().write("content");
assertNotNull(ActionContext.getContext());
assertNotNull(Dispatcher.getInstance());
});
}
@Test
public void test404() throws ServletException, IOException {
MockHttpServletResponse response = run("/foo.action", filterPrepare, filterExecute, failFilter);
assertEquals(404, response.getStatus());
}
@Test
public void test200() throws ServletException, IOException {
MockHttpServletResponse response = run("/hello.action", filterPrepare, filterExecute, failFilter);
assertEquals(200, response.getStatus());
}
@Test
public void testStaticFallthrough() throws ServletException, IOException {
MockHttpServletResponse response = run("/foo.txt", filterPrepare, filterExecute, stringFilter);
assertEquals(200, response.getStatus());
@@ -78,29 +88,68 @@ public class TwoFilterIntegrationTest extends TestCase {
}
@Test
public void testStaticExecute() throws ServletException, IOException {
MockHttpServletResponse response = run("/static/utils.js", filterPrepare, filterExecute, failFilter);
assertEquals(200, response.getStatus());
assertTrue(response.getContentAsString().contains("StrutsUtils"));
}
@Test
public void testFilterInMiddle() throws ServletException, IOException {
Filter middle = new Filter() {
public void init(FilterConfig filterConfig) throws ServletException {}
public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException {
assertNotNull(ActionContext.getContext());
assertNotNull(Dispatcher.getInstance());
assertNull(ActionContext.getContext().getActionInvocation());
chain.doFilter(request, response);
assertEquals("hello", ActionContext.getContext().getActionInvocation().getProxy().getActionName());
}
public void destroy() {}
};
Filter middle = newFilter((req, res, chain) -> {
assertNotNull(ActionContext.getContext());
assertNotNull(Dispatcher.getInstance());
assertNull(ActionContext.getContext().getActionInvocation());
chain.doFilter(req, res);
assertEquals("hello", ActionContext.getContext().getActionInvocation().getProxy().getActionName());
});
MockHttpServletResponse response = run("/hello.action", filterPrepare, middle, filterExecute, failFilter);
assertEquals(200, response.getStatus());
}
/**
* It is possible for a Struts excluded URL to be forwarded to a Struts URL. If this happens, the ActionContext
* should not be cleared until the very first execution of the StrutsPrepareFilter, otherwise SiteMesh will malfunction.
*/
@Test
public void testActionContextNotClearedUntilEndWhenForwardedFromExcludedUrl() throws ServletException, IOException {
Filter firstFilter = newFilter((req, res, chain) -> {
chain.doFilter(req, res);
// Assert ActionContext cleared at end of request lifecycle
assertNull(ActionContext.getContext());
});
Filter dummySiteMesh = newFilter((req, res, chain) -> {
// Assert ActionContext not created initially, as URL is Struts excluded
assertNull(ActionContext.getContext());
chain.doFilter(req, res);
// Assert ActionContext not cleared by second StrutsPrepareFilter even though it created it
assertNotNull(ActionContext.getContext());
});
Filter dummyForward = newFilter((req, res, chain) -> {
MockHttpServletRequest castReq = (MockHttpServletRequest) req;
String oldUri = castReq.getRequestURI();
castReq.setRequestURI("/hello.action");
chain.doFilter(castReq, res);
castReq.setRequestURI(oldUri);
});
MockHttpServletResponse response = run(
"/excluded/hello.action",
singletonMap("struts.action.excludePattern", "^/excluded/hello.action"),
firstFilter,
filterPrepare,
dummySiteMesh,
filterExecute,
dummyForward,
filterPrepare);
assertEquals(200, response.getStatus());
}
private MockHttpServletResponse run(String uri, final Filter... filters) throws ServletException, IOException {
return run(uri, emptyMap(), filters);
}
private MockHttpServletResponse run(String uri, Map<String, String> filterInitParams, final Filter... filters) throws ServletException, IOException {
final LinkedList<Filter> filterList = new LinkedList<>(Arrays.asList(filters));
MockHttpServletRequest request = new MockHttpServletRequest();
MockHttpServletResponse response = new MockHttpServletResponse();
@@ -118,7 +167,7 @@ public class TwoFilterIntegrationTest extends TestCase {
}
}
};
filterInitParams.forEach(filterConfig::addInitParameter);
request.setRequestURI(uri);
for (Filter filter : filters) {
filter.init(filterConfig);
@@ -130,5 +179,22 @@ public class TwoFilterIntegrationTest extends TestCase {
return response;
}
private Filter newFilter(DoFilterConsumer doFilterConsumer) {
return new Filter() {
public void init(FilterConfig filterConfig) {
}
public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException {
doFilterConsumer.accept(request, response, chain);
}
public void destroy() {
}
};
}
@FunctionalInterface
public interface DoFilterConsumer {
void accept(ServletRequest req, ServletResponse res, FilterChain chain) throws IOException, ServletException;
}
}