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");


Reply via email to