WW-4437 Fixes problem with accepted params
Project: http://git-wip-us.apache.org/repos/asf/struts/repo Commit: http://git-wip-us.apache.org/repos/asf/struts/commit/40822d67 Tree: http://git-wip-us.apache.org/repos/asf/struts/tree/40822d67 Diff: http://git-wip-us.apache.org/repos/asf/struts/diff/40822d67 Branch: refs/heads/master Commit: 40822d67f5b6b667bb2760986cb78efc9e2e3ac4 Parents: 4964b74 Author: Lukasz Lenart <lukaszlen...@apache.org> Authored: Tue Dec 23 13:29:17 2014 +0100 Committer: Lukasz Lenart <lukaszlen...@apache.org> Committed: Tue Dec 23 13:29:17 2014 +0100 ---------------------------------------------------------------------- .../struts2/interceptor/CookieInterceptor.java | 37 ++++++++++--------- .../interceptor/CookieInterceptorTest.java | 38 ++++++++++++-------- 2 files changed, 45 insertions(+), 30 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/struts/blob/40822d67/core/src/main/java/org/apache/struts2/interceptor/CookieInterceptor.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/struts2/interceptor/CookieInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/CookieInterceptor.java index ca195fa..06c4c30 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/CookieInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/CookieInterceptor.java @@ -25,6 +25,7 @@ import com.opensymphony.xwork2.ActionContext; import com.opensymphony.xwork2.ActionInvocation; import com.opensymphony.xwork2.inject.Inject; import com.opensymphony.xwork2.interceptor.AbstractInterceptor; +import com.opensymphony.xwork2.security.AcceptedPatternsChecker; import com.opensymphony.xwork2.security.ExcludedPatternsChecker; import com.opensymphony.xwork2.util.TextParseUtil; import com.opensymphony.xwork2.util.ValueStack; @@ -37,7 +38,6 @@ import java.util.Collections; import java.util.LinkedHashMap; import java.util.Map; import java.util.Set; -import java.util.regex.Pattern; /** * <!-- START SNIPPET: description --> @@ -174,16 +174,20 @@ public class CookieInterceptor extends AbstractInterceptor { private Set<String> cookiesNameSet = Collections.emptySet(); private Set<String> cookiesValueSet = Collections.emptySet(); - // Allowed names of cookies - private Pattern acceptedPattern = Pattern.compile(ACCEPTED_PATTERN, Pattern.CASE_INSENSITIVE); - private ExcludedPatternsChecker excludedPatternsChecker; + private AcceptedPatternsChecker acceptedPatternsChecker; @Inject public void setExcludedPatternsChecker(ExcludedPatternsChecker excludedPatternsChecker) { this.excludedPatternsChecker = excludedPatternsChecker; } + @Inject + public void setAcceptedPatternsChecker(AcceptedPatternsChecker acceptedPatternsChecker) { + this.acceptedPatternsChecker = acceptedPatternsChecker; + this.acceptedPatternsChecker.setAcceptedPatterns(ACCEPTED_PATTERN); + } + /** * Set the <code>cookiesName</code> which if matched will allow the cookie * to be injected into action, could be comma-separated string. @@ -208,12 +212,13 @@ public class CookieInterceptor extends AbstractInterceptor { } /** - * Set the <code>acceptCookieNames</code> pattern of allowed names of cookies to protect against remote command execution vulnerability + * Set the <code>acceptCookieNames</code> pattern of allowed names of cookies + * to protect against remote command execution vulnerability. * - * @param pattern used to check cookie name against + * @param commaDelimitedPattern is used to check cookie name against, can set of comma delimited patterns */ - public void setAcceptCookieNames(String pattern) { - acceptedPattern = Pattern.compile(pattern); + public void setAcceptCookieNames(String commaDelimitedPattern) { + acceptedPatternsChecker.setAcceptedPatterns(commaDelimitedPattern); } public String intercept(ActionInvocation invocation) throws Exception { @@ -280,17 +285,17 @@ public class CookieInterceptor extends AbstractInterceptor { * @return true|false */ protected boolean isAccepted(String name) { - boolean matches = acceptedPattern.matcher(name).matches(); - if (matches) { - if (LOG.isTraceEnabled()) { - LOG.trace("Cookie [#0] matches acceptedPattern [#1]", name, ACCEPTED_PATTERN); - } - } else { + AcceptedPatternsChecker.IsAccepted accepted = acceptedPatternsChecker.isAccepted(name); + if (accepted.isAccepted()) { if (LOG.isTraceEnabled()) { - LOG.trace("Cookie [#0] doesn't match acceptedPattern [#1]", name, ACCEPTED_PATTERN); + LOG.trace("Cookie [#0] matches acceptedPattern [#1]", name, accepted.getAcceptedPattern()); } + return true; } - return matches; + if (LOG.isTraceEnabled()) { + LOG.trace("Cookie [#0] doesn't match acceptedPattern [#1]", name, accepted.getAcceptedPattern()); + } + return false; } /** http://git-wip-us.apache.org/repos/asf/struts/blob/40822d67/core/src/test/java/org/apache/struts2/interceptor/CookieInterceptorTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/struts2/interceptor/CookieInterceptorTest.java b/core/src/test/java/org/apache/struts2/interceptor/CookieInterceptorTest.java index a531a69..c730382 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/CookieInterceptorTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/CookieInterceptorTest.java @@ -27,6 +27,7 @@ import java.util.Map; import javax.servlet.http.Cookie; +import com.opensymphony.xwork2.security.DefaultAcceptedPatternsChecker; import com.opensymphony.xwork2.security.DefaultExcludedPatternsChecker; import com.opensymphony.xwork2.mock.MockActionInvocation; import org.easymock.MockControl; @@ -44,11 +45,11 @@ public class CookieInterceptorTest extends StrutsInternalTestCase { public void testIntercepDefault() throws Exception { MockHttpServletRequest request = new MockHttpServletRequest(); - request.setCookies(new Cookie[] { + request.setCookies( new Cookie("cookie1", "cookie1value"), new Cookie("cookie2", "cookie2value"), new Cookie("cookie3", "cookie3value") - }); + ); ServletActionContext.setRequest(request); MockActionWithCookieAware action = new MockActionWithCookieAware(); @@ -67,6 +68,7 @@ public class CookieInterceptorTest extends StrutsInternalTestCase { // by default the interceptor doesn't accept any cookies CookieInterceptor interceptor = new CookieInterceptor(); interceptor.setExcludedPatternsChecker(new DefaultExcludedPatternsChecker()); + interceptor.setAcceptedPatternsChecker(new DefaultAcceptedPatternsChecker()); interceptor.intercept(invocation); @@ -81,11 +83,11 @@ public class CookieInterceptorTest extends StrutsInternalTestCase { public void testInterceptAll1() throws Exception { MockHttpServletRequest request = new MockHttpServletRequest(); - request.setCookies(new Cookie[] { + request.setCookies( new Cookie("cookie1", "cookie1value"), new Cookie("cookie2", "cookie2value"), new Cookie("cookie3", "cookie3value") - }); + ); ServletActionContext.setRequest(request); MockActionWithCookieAware action = new MockActionWithCookieAware(); @@ -103,6 +105,7 @@ public class CookieInterceptorTest extends StrutsInternalTestCase { CookieInterceptor interceptor = new CookieInterceptor(); interceptor.setExcludedPatternsChecker(new DefaultExcludedPatternsChecker()); + interceptor.setAcceptedPatternsChecker(new DefaultAcceptedPatternsChecker()); interceptor.setCookiesName("*"); interceptor.setCookiesValue("*"); interceptor.intercept(invocation); @@ -123,11 +126,11 @@ public class CookieInterceptorTest extends StrutsInternalTestCase { public void testInterceptAll2() throws Exception { MockHttpServletRequest request = new MockHttpServletRequest(); - request.setCookies(new Cookie[] { + request.setCookies( new Cookie("cookie1", "cookie1value"), new Cookie("cookie2", "cookie2value"), new Cookie("cookie3", "cookie3value") - }); + ); ServletActionContext.setRequest(request); MockActionWithCookieAware action = new MockActionWithCookieAware(); @@ -145,6 +148,7 @@ public class CookieInterceptorTest extends StrutsInternalTestCase { CookieInterceptor interceptor = new CookieInterceptor(); interceptor.setExcludedPatternsChecker(new DefaultExcludedPatternsChecker()); + interceptor.setAcceptedPatternsChecker(new DefaultAcceptedPatternsChecker()); interceptor.setCookiesName("cookie1, cookie2, cookie3"); interceptor.setCookiesValue("cookie1value, cookie2value, cookie3value"); interceptor.intercept(invocation); @@ -164,11 +168,11 @@ public class CookieInterceptorTest extends StrutsInternalTestCase { public void testInterceptSelectedCookiesNameOnly1() throws Exception { MockHttpServletRequest request = new MockHttpServletRequest(); - request.setCookies(new Cookie[] { + request.setCookies( new Cookie("cookie1", "cookie1value"), new Cookie("cookie2", "cookie2value"), new Cookie("cookie3", "cookie3value") - }); + ); ServletActionContext.setRequest(request); MockActionWithCookieAware action = new MockActionWithCookieAware(); @@ -186,6 +190,7 @@ public class CookieInterceptorTest extends StrutsInternalTestCase { CookieInterceptor interceptor = new CookieInterceptor(); interceptor.setExcludedPatternsChecker(new DefaultExcludedPatternsChecker()); + interceptor.setAcceptedPatternsChecker(new DefaultAcceptedPatternsChecker()); interceptor.setCookiesName("cookie1, cookie3"); interceptor.setCookiesValue("cookie1value, cookie2value, cookie3value"); interceptor.intercept(invocation); @@ -205,11 +210,11 @@ public class CookieInterceptorTest extends StrutsInternalTestCase { public void testInterceptSelectedCookiesNameOnly2() throws Exception { MockHttpServletRequest request = new MockHttpServletRequest(); - request.setCookies(new Cookie[] { + request.setCookies( new Cookie("cookie1", "cookie1value"), new Cookie("cookie2", "cookie2value"), new Cookie("cookie3", "cookie3value") - }); + ); ServletActionContext.setRequest(request); MockActionWithCookieAware action = new MockActionWithCookieAware(); @@ -227,6 +232,7 @@ public class CookieInterceptorTest extends StrutsInternalTestCase { CookieInterceptor interceptor = new CookieInterceptor(); interceptor.setExcludedPatternsChecker(new DefaultExcludedPatternsChecker()); + interceptor.setAcceptedPatternsChecker(new DefaultAcceptedPatternsChecker()); interceptor.setCookiesName("cookie1, cookie3"); interceptor.setCookiesValue("*"); interceptor.intercept(invocation); @@ -246,11 +252,11 @@ public class CookieInterceptorTest extends StrutsInternalTestCase { public void testInterceptSelectedCookiesNameOnly3() throws Exception { MockHttpServletRequest request = new MockHttpServletRequest(); - request.setCookies(new Cookie[] { + request.setCookies( new Cookie("cookie1", "cookie1value"), new Cookie("cookie2", "cookie2value"), new Cookie("cookie3", "cookie3value") - }); + ); ServletActionContext.setRequest(request); MockActionWithCookieAware action = new MockActionWithCookieAware(); @@ -268,6 +274,7 @@ public class CookieInterceptorTest extends StrutsInternalTestCase { CookieInterceptor interceptor = new CookieInterceptor(); interceptor.setExcludedPatternsChecker(new DefaultExcludedPatternsChecker()); + interceptor.setAcceptedPatternsChecker(new DefaultAcceptedPatternsChecker()); interceptor.setCookiesName("cookie1, cookie3"); interceptor.setCookiesValue(""); interceptor.intercept(invocation); @@ -288,11 +295,11 @@ public class CookieInterceptorTest extends StrutsInternalTestCase { public void testInterceptSelectedCookiesNameAndValue() throws Exception { MockHttpServletRequest request = new MockHttpServletRequest(); - request.setCookies(new Cookie[] { + request.setCookies( new Cookie("cookie1", "cookie1value"), new Cookie("cookie2", "cookie2value"), new Cookie("cookie3", "cookie3value") - }); + ); ServletActionContext.setRequest(request); MockActionWithCookieAware action = new MockActionWithCookieAware(); @@ -310,6 +317,7 @@ public class CookieInterceptorTest extends StrutsInternalTestCase { CookieInterceptor interceptor = new CookieInterceptor(); interceptor.setExcludedPatternsChecker(new DefaultExcludedPatternsChecker()); + interceptor.setAcceptedPatternsChecker(new DefaultAcceptedPatternsChecker()); interceptor.setCookiesName("cookie1, cookie3"); interceptor.setCookiesValue("cookie1value"); interceptor.intercept(invocation); @@ -371,6 +379,7 @@ public class CookieInterceptorTest extends StrutsInternalTestCase { } }; interceptor.setExcludedPatternsChecker(new DefaultExcludedPatternsChecker()); + interceptor.setAcceptedPatternsChecker(new DefaultAcceptedPatternsChecker()); interceptor.setCookiesName("*"); MockActionInvocation invocation = new MockActionInvocation(); @@ -431,6 +440,7 @@ public class CookieInterceptorTest extends StrutsInternalTestCase { } }; interceptor.setExcludedPatternsChecker(new DefaultExcludedPatternsChecker()); + interceptor.setAcceptedPatternsChecker(new DefaultAcceptedPatternsChecker()); interceptor.setCookiesName("*"); MockActionInvocation invocation = new MockActionInvocation();