Author: pbenedict Date: Sun Dec 21 10:55:19 2008 New Revision: 728473 URL: http://svn.apache.org/viewvc?rev=728473&view=rev Log: STR-3169: Do not match when recursive substitutions are detected
Modified: struts/struts1/branches/STRUTS_1_3_BRANCH/core/src/main/java/org/apache/struts/config/ActionConfigMatcher.java struts/struts1/branches/STRUTS_1_3_BRANCH/core/src/test/java/org/apache/struts/config/TestActionConfigMatcher.java Modified: struts/struts1/branches/STRUTS_1_3_BRANCH/core/src/main/java/org/apache/struts/config/ActionConfigMatcher.java URL: http://svn.apache.org/viewvc/struts/struts1/branches/STRUTS_1_3_BRANCH/core/src/main/java/org/apache/struts/config/ActionConfigMatcher.java?rev=728473&r1=728472&r2=728473&view=diff ============================================================================== --- struts/struts1/branches/STRUTS_1_3_BRANCH/core/src/main/java/org/apache/struts/config/ActionConfigMatcher.java (original) +++ struts/struts1/branches/STRUTS_1_3_BRANCH/core/src/main/java/org/apache/struts/config/ActionConfigMatcher.java Sun Dec 21 10:55:19 2008 @@ -24,6 +24,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.struts.action.ActionForward; +import org.apache.struts.config.ActionConfig; import org.apache.struts.util.WildcardHelper; import java.io.Serializable; @@ -123,9 +124,18 @@ + m.getActionConfig().getPath() + "'"); } - config = - convertActionConfig(path, - (ActionConfig) m.getActionConfig(), vars); + try { + config = + convertActionConfig(path, + (ActionConfig) m.getActionConfig(), vars); + } catch (IllegalStateException e) { + log.warn("Path matches pattern '" + + m.getActionConfig().getPath() + "' but is " + + "incompatible with the matching config due " + + "to recursive substitution: " + + path); + config = null; + } } } } Modified: struts/struts1/branches/STRUTS_1_3_BRANCH/core/src/test/java/org/apache/struts/config/TestActionConfigMatcher.java URL: http://svn.apache.org/viewvc/struts/struts1/branches/STRUTS_1_3_BRANCH/core/src/test/java/org/apache/struts/config/TestActionConfigMatcher.java?rev=728473&r1=728472&r2=728473&view=diff ============================================================================== --- struts/struts1/branches/STRUTS_1_3_BRANCH/core/src/test/java/org/apache/struts/config/TestActionConfigMatcher.java (original) +++ struts/struts1/branches/STRUTS_1_3_BRANCH/core/src/test/java/org/apache/struts/config/TestActionConfigMatcher.java Sun Dec 21 10:55:19 2008 @@ -25,6 +25,8 @@ import org.apache.struts.action.ActionForward; import org.apache.struts.action.ActionMapping; +import org.apache.struts.config.ActionConfig; +import org.apache.struts.config.ActionConfigMatcher; import org.apache.struts.mock.TestMockBase; /** @@ -72,6 +74,88 @@ assertNull("ActionConfig shouldn't be matched", matcher.match("/test")); } + /** + * Verifies that a match succeeds when the substituted value contains a + * placeholder key. + * <p> + * See STR-3169. + * + * @since Struts 1.3.11 + */ + public void testMatchWithKeyInPath() { + ActionMapping[] mapping = new ActionMapping[1]; + mapping[0] = new ActionMapping(); + mapping[0].setPath("/page-*"); + + ActionConfigMatcher matcher = new ActionConfigMatcher(mapping); + ActionConfig matched = matcher.match("/page-{0}"); + + assertNotNull("ActionConfig should be matched", matched); + assertEquals("Path hasn't been replaced", "/page-{0}", matched.getPath()); + } + + /** + * Verifies that an infinite loop is prevented and substitution is skipped when + * the substituted value equals the placeholder key. + * <p> + * See STR-3169. + * + * @since Struts 1.3.11 + * @see #testMatchWithSubstitutionEqualPlaceholderKey1() + */ + public void testMatchWithSubstitutionEqualPlaceholderKey0() { + ActionMapping[] mapping = new ActionMapping[1]; + mapping[0] = new ActionMapping(); + mapping[0].setPath("/page-*"); + mapping[0].setParameter("{0}"); + + ActionConfigMatcher matcher = new ActionConfigMatcher(mapping); + ActionConfig matched = matcher.match("/page-{1}"); + + assertNull("ActionConfig should not be matched", matched); + } + + /** + * Verifies that an infinite loop is prevented and substitution is skipped when + * the substituted value equals the placeholder key. + * <p> + * See STR-3169. + * + * @since Struts 1.3.11 + * @see #testMatchWithSubstitutionEqualPlaceholderKey0() + */ + public void testMatchWithSubstitutionEqualPlaceholderKey1() { + ActionMapping[] mapping = new ActionMapping[1]; + mapping[0] = new ActionMapping(); + mapping[0].setPath("/page-*"); + mapping[0].setParameter("{1}"); + + ActionConfigMatcher matcher = new ActionConfigMatcher(mapping); + ActionConfig matched = matcher.match("/page-{1}"); + + assertNull("ActionConfig should not be matched", matched); + } + + /** + * Verifies that an infinite loop is prevented and substitution is skipped when + * the the placeholder key is contained within the substituted value. + * <p> + * See STR-3169. + * + * @since Struts 1.3.11 + */ + public void testMatchWhenSubstitutionContainsPlaceholderKey() { + ActionMapping[] mapping = new ActionMapping[1]; + mapping[0] = new ActionMapping(); + mapping[0].setPath("/page-*"); + mapping[0].setParameter("{1}"); + + ActionConfigMatcher matcher = new ActionConfigMatcher(mapping); + ActionConfig matched = matcher.match("/page-{0}"); + + assertNull("ActionConfig should not be matched", matched); + } + public void testNoWildcardMatch() { ActionConfig[] configs = new ActionConfig[1]; ActionConfig mapping = buildActionConfig("/fooBar");