diff --git a/core/src/main/java/com/opensymphony/xwork2/config/ConfigurationManager.java b/core/src/main/java/com/opensymphony/xwork2/config/ConfigurationManager.java index 60d73d12e..49517eedf 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/ConfigurationManager.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/ConfigurationManager.java @@ -210,7 +210,7 @@ public class ConfigurationManager { private boolean needReloadContainerProviders() { Optional 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; diff --git a/core/src/main/java/org/apache/struts2/dispatcher/PrepareOperations.java b/core/src/main/java/org/apache/struts2/dispatcher/PrepareOperations.java index d8e4bf570..a643de310 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/PrepareOperations.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/PrepareOperations.java @@ -229,13 +229,14 @@ public class PrepareOperations { * * @return true if the request URI matches one of the given patterns */ - public boolean isUrlExcluded( HttpServletRequest request, List 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 excludedPatterns) { + if (excludedPatterns == null) { + return false; + } + String uri = RequestUtils.getUri(request); + for (Pattern pattern : excludedPatterns) { + if (pattern.matcher(uri).matches()) { + return true; } } return false; diff --git a/core/src/main/java/org/apache/struts2/dispatcher/filter/StrutsExecuteFilter.java b/core/src/main/java/org/apache/struts2/dispatcher/filter/StrutsExecuteFilter.java index eeaf3abb9..5328ac3a9 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/filter/StrutsExecuteFilter.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/filter/StrutsExecuteFilter.java @@ -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 { diff --git a/core/src/main/java/org/apache/struts2/dispatcher/filter/StrutsPrepareAndExecuteFilter.java b/core/src/main/java/org/apache/struts2/dispatcher/filter/StrutsPrepareAndExecuteFilter.java index 194ae33c4..e91a1b9a8 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/filter/StrutsPrepareAndExecuteFilter.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/filter/StrutsPrepareAndExecuteFilter.java @@ -50,7 +50,7 @@ public class StrutsPrepareAndExecuteFilter implements StrutsStatics, Filter { protected PrepareOperations prepare; protected ExecuteOperations execute; - protected List excludedPatterns = null; + protected List 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(); } diff --git a/core/src/main/java/org/apache/struts2/dispatcher/filter/StrutsPrepareFilter.java b/core/src/main/java/org/apache/struts2/dispatcher/filter/StrutsPrepareFilter.java index 9c3f434a7..81dffb6a7 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/filter/StrutsPrepareFilter.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/filter/StrutsPrepareFilter.java @@ -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 excludedPatterns = null; + protected List 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); diff --git a/core/src/test/java/org/apache/struts2/dispatcher/PrepareOperationsTest.java b/core/src/test/java/org/apache/struts2/dispatcher/PrepareOperationsTest.java index 000f97013..76fee5579 100644 --- a/core/src/test/java/org/apache/struts2/dispatcher/PrepareOperationsTest.java +++ b/core/src/test/java/org/apache/struts2/dispatcher/PrepareOperationsTest.java @@ -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()); } } diff --git a/core/src/test/java/org/apache/struts2/dispatcher/StrutsPrepareAndExecuteFilterIntegrationTest.java b/core/src/test/java/org/apache/struts2/dispatcher/StrutsPrepareAndExecuteFilterIntegrationTest.java index a5419cd8b..eb87235e1 100644 --- a/core/src/test/java/org/apache/struts2/dispatcher/StrutsPrepareAndExecuteFilterIntegrationTest.java +++ b/core/src/test/java/org/apache/struts2/dispatcher/StrutsPrepareAndExecuteFilterIntegrationTest.java @@ -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(); + 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) { diff --git a/core/src/test/java/org/apache/struts2/dispatcher/TwoFilterIntegrationTest.java b/core/src/test/java/org/apache/struts2/dispatcher/TwoFilterIntegrationTest.java index edc88ba17..65d2dfaea 100644 --- a/core/src/test/java/org/apache/struts2/dispatcher/TwoFilterIntegrationTest.java +++ b/core/src/test/java/org/apache/struts2/dispatcher/TwoFilterIntegrationTest.java @@ -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 filterInitParams, final Filter... filters) throws ServletException, IOException { final LinkedList 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; + } }