CVE-2011-4343

@@ -137,7 +137,8 @@ public URL getBookmarkableURL(FacesContext context) throws MalformedURLException
         return new URL(externalContext.getRequestScheme(),
                 externalContext.getRequestServerName(),
                 externalContext.getRequestServerPort(),
-                context.getApplication().getViewHandler().getBookmarkableURL(context, getToViewId(context), getParameters(), isIncludeViewParams()));  
+                context.getApplication().getViewHandler().getBookmarkableURL(context, getToViewId(context), 
+                        _NavigationUtils.getEvaluatedNavigationParameters(getParameters()), isIncludeViewParams()));  
     }
 
     public URL getResourceURL(FacesContext context) throws MalformedURLException
@@ -153,7 +154,8 @@ public URL getRedirectURL(FacesContext context) throws MalformedURLException
         return new URL(externalContext.getRequestScheme(),
                 externalContext.getRequestServerName(),
                 externalContext.getRequestServerPort(),
-                context.getApplication().getViewHandler().getRedirectURL(context, getToViewId(context), getParameters(), isIncludeViewParams()));
+                context.getApplication().getViewHandler().getRedirectURL(context, getToViewId(context), 
+                        _NavigationUtils.getEvaluatedNavigationParameters(getParameters()), isIncludeViewParams()));
     }
     
     public Map<String,List<String>> getParameters()

@@ -0,0 +1,107 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package javax.faces.application;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import javax.faces.context.FacesContext;
+
+/**
+ * 
+ * @author Leonardo Uribe
+ *
+ */
+class _NavigationUtils
+{
+    /**
+     * Evaluate all EL expressions found as parameters and return a map that can be used for 
+     * redirect or render bookmark links
+     * 
+     * @param parameters parameter map retrieved from NavigationCase.getParameters()
+     * @return
+     */
+    public static Map<String, List<String> > getEvaluatedNavigationParameters(Map<String, List<String> > parameters)
+    {
+        Map<String,List<String>> evaluatedParameters = null;
+        if (parameters != null && parameters.size() > 0)
+        {
+            evaluatedParameters = new HashMap<String, List<String>>();
+            for (Map.Entry<String, List<String>> pair : parameters.entrySet())
+            {
+                boolean containsEL = false;
+                for (String value : pair.getValue())
+                {
+                    if (_isExpression(value))
+                    {
+                        containsEL = true;
+                        break;
+                    }
+                }
+                if (containsEL)
+                {
+                    evaluatedParameters.put(pair.getKey(), _evaluateValueExpressions(pair.getValue()));
+                }
+                else
+                {
+                    evaluatedParameters.put(pair.getKey(), pair.getValue());
+                }
+            }
+        }
+        else
+        {
+            evaluatedParameters = parameters;
+        }
+        return evaluatedParameters;
+    }
+    
+    /**
+     * Checks the Strings in the List for EL expressions and evaluates them.
+     * Note that the returned List will be a copy of the given List, because
+     * otherwise it will have unwanted side-effects.
+     * @param values
+     * @return
+     */
+    private static List<String> _evaluateValueExpressions(List<String> values)
+    {
+        // note that we have to create a new List here, because if we
+        // change any value on the given List, it will be changed in the
+        // NavigationCase too and the EL expression won't be evaluated again
+        List<String> target = new ArrayList<String>(values.size());
+        FacesContext context = FacesContext.getCurrentInstance();
+        for (String value : values)
+        {
+            if (_isExpression(value))
+            {
+                // evaluate the ValueExpression
+                value = context.getApplication().evaluateExpressionGet(context, value, String.class);
+            }
+            target.add(value);
+        }
+        return target;
+    }
+    
+    private static boolean _isExpression(String text)
+    {
+        return text.indexOf("#{") != -1;
+    }
+
+}

@@ -50,6 +50,7 @@
 
 import org.apache.myfaces.config.RuntimeConfig;
 import org.apache.myfaces.config.element.NavigationRule;
+import org.apache.myfaces.shared.application.NavigationUtils;
 import org.apache.myfaces.shared.util.HashMapUtils;
 import org.apache.myfaces.shared.util.StringUtils;
 import org.apache.myfaces.view.facelets.tag.jsf.PreDisposeViewEvent;
@@ -101,7 +102,12 @@ public void handleNavigation(FacesContext facesContext, String fromAction, Strin
                 ExternalContext externalContext = facesContext.getExternalContext();
                 ViewHandler viewHandler = facesContext.getApplication().getViewHandler();
                 String toViewId = navigationCase.getToViewId(facesContext);
-                String redirectPath = viewHandler.getRedirectURL(facesContext, toViewId, navigationCase.getParameters(), navigationCase.isIncludeViewParams());
+                
+
+                String redirectPath = viewHandler.getRedirectURL(
+                        facesContext, toViewId, 
+                        NavigationUtils.getEvaluatedNavigationParameters(navigationCase.getParameters()) ,
+                        navigationCase.isIncludeViewParams());
                 
                 //Clear ViewMap if we are redirecting to other resource
                 UIViewRoot viewRoot = facesContext.getViewRoot(); 

@@ -765,7 +765,7 @@ private String encodeURL(String baseUrl, Map<String, List<String>> parameters)
             {
                 if (pair.getKey() != null && pair.getKey().trim().length() != 0)
                 {
-                    paramMap.put(pair.getKey(), _evaluateValueExpressions(pair.getValue()));
+                    paramMap.put(pair.getKey(), pair.getValue());
                 }
             }
         }
@@ -815,37 +815,6 @@ private String encodeURL(String baseUrl, Map<String, List<String>> parameters)
         return newUrl.toString();
     }
     
-    /**
-     * Checks the Strings in the List for EL expressions and evaluates them.
-     * Note that the returned List will be a copy of the given List, because
-     * otherwise it will have unwanted side-effects.
-     * @param values
-     * @return
-     */
-    private List<String> _evaluateValueExpressions(List<String> values)
-    {
-        // note that we have to create a new List here, because if we
-        // change any value on the given List, it will be changed in the
-        // NavigationCase too and the EL expression won't be evaluated again
-        List<String> target = new ArrayList<String>(values.size());
-        FacesContext context = FacesContext.getCurrentInstance();
-        for (String value : values)
-        {
-            if (_isExpression(value))
-            {
-                // evaluate the ValueExpression
-                value = context.getApplication().evaluateExpressionGet(context, value, String.class);
-            }
-            target.add(value);
-        }
-        return target;
-    }
-    
-    private boolean _isExpression(String text)
-    {
-        return text.indexOf("#{") != -1;
-    }
-    
     /**
      * @since 2.0
      */

@@ -65,6 +65,7 @@ public void tearDown() throws Exception
      * Tests if encodeRedirectURL() and encodeBookmarkableURL() correctly
      * evaluate ValueExpressions as parameters values.
      */
+    /* TODO: Invalid test, because EL evaluation should be done before call these methods.
     @Test
     @SuppressWarnings("unchecked")
     public void testEncodeURLHandlesValueExpressionParameters()
@@ -94,7 +95,7 @@ public void testEncodeURLHandlesValueExpressionParameters()
         Assert.assertTrue(bookmarkableUrl.contains("param1=literalvalue"));
         Assert.assertTrue(bookmarkableUrl.contains("param1=myvalue1"));
         Assert.assertTrue(bookmarkableUrl.contains("param2=myvalue2"));
-    }
+    }*/
 
     @Test
     public void testEncodeRedirectUrlWithEmptyParamInBaseUrl()

@@ -0,0 +1,107 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.myfaces.shared.application;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import javax.faces.context.FacesContext;
+
+/**
+ * 
+ * @author Leonardo Uribe
+ *
+ */
+public class NavigationUtils
+{
+    /**
+     * Evaluate all EL expressions found as parameters and return a map that can be used for 
+     * redirect or render bookmark links
+     * 
+     * @param parameters parameter map retrieved from NavigationCase.getParameters()
+     * @return
+     */
+    public static Map<String, List<String> > getEvaluatedNavigationParameters(Map<String, List<String> > parameters)
+    {
+        Map<String,List<String>> evaluatedParameters = null;
+        if (parameters != null && parameters.size() > 0)
+        {
+            evaluatedParameters = new HashMap<String, List<String>>();
+            for (Map.Entry<String, List<String>> pair : parameters.entrySet())
+            {
+                boolean containsEL = false;
+                for (String value : pair.getValue())
+                {
+                    if (_isExpression(value))
+                    {
+                        containsEL = true;
+                        break;
+                    }
+                }
+                if (containsEL)
+                {
+                    evaluatedParameters.put(pair.getKey(), _evaluateValueExpressions(pair.getValue()));
+                }
+                else
+                {
+                    evaluatedParameters.put(pair.getKey(), pair.getValue());
+                }
+            }
+        }
+        else
+        {
+            evaluatedParameters = parameters;
+        }
+        return evaluatedParameters;
+    }
+    
+    /**
+     * Checks the Strings in the List for EL expressions and evaluates them.
+     * Note that the returned List will be a copy of the given List, because
+     * otherwise it will have unwanted side-effects.
+     * @param values
+     * @return
+     */
+    private static List<String> _evaluateValueExpressions(List<String> values)
+    {
+        // note that we have to create a new List here, because if we
+        // change any value on the given List, it will be changed in the
+        // NavigationCase too and the EL expression won't be evaluated again
+        List<String> target = new ArrayList<String>(values.size());
+        FacesContext context = FacesContext.getCurrentInstance();
+        for (String value : values)
+        {
+            if (_isExpression(value))
+            {
+                // evaluate the ValueExpression
+                value = context.getApplication().evaluateExpressionGet(context, value, String.class);
+            }
+            target.add(value);
+        }
+        return target;
+    }
+    
+    private static boolean _isExpression(String text)
+    {
+        return text.indexOf("#{") != -1;
+    }
+
+}

@@ -63,6 +63,7 @@
 import javax.faces.model.SelectItem;
 import javax.faces.model.SelectItemGroup;
 
+import org.apache.myfaces.shared.application.NavigationUtils;
 import org.apache.myfaces.shared.component.DisplayValueOnlyCapable;
 import org.apache.myfaces.shared.component.EscapeCapable;
 import org.apache.myfaces.shared.config.MyfacesConfig;
@@ -1589,8 +1590,8 @@ public static String getOutcomeTargetHref(FacesContext facesContext,
             }
         }
         // handle NavigationCase parameters
-        Map<String, List<String>> navigationCaseParams = navigationCase
-                .getParameters();
+        Map<String, List<String>> navigationCaseParams = 
+            NavigationUtils.getEvaluatedNavigationParameters(navigationCase.getParameters());
         if (navigationCaseParams != null)
         {
             if (parameters == null)

@@ -138,8 +138,8 @@ public URL getBookmarkableURL(FacesContext context) throws MalformedURLException
         return new URL(externalContext.getRequestScheme(),
                 externalContext.getRequestServerName(),
                 externalContext.getRequestServerPort(),
-                context.getApplication().getViewHandler().getBookmarkableURL(context, getToViewId(context),
-                                                                             getParameters(), isIncludeViewParams()));
+                context.getApplication().getViewHandler().getBookmarkableURL(context, getToViewId(context), 
+                        _NavigationUtils.getEvaluatedNavigationParameters(getParameters()), isIncludeViewParams()));
     }
 
     public URL getResourceURL(FacesContext context) throws MalformedURLException
@@ -156,8 +156,8 @@ public URL getRedirectURL(FacesContext context) throws MalformedURLException
         return new URL(externalContext.getRequestScheme(),
                 externalContext.getRequestServerName(),
                 externalContext.getRequestServerPort(),
-                context.getApplication().getViewHandler().getRedirectURL(context, getToViewId(context),
-                                                                         getParameters(), isIncludeViewParams()));
+                context.getApplication().getViewHandler().getRedirectURL(context, getToViewId(context), 
+                        _NavigationUtils.getEvaluatedNavigationParameters(getParameters()), isIncludeViewParams()));
     }
     
     public Map<String,List<String>> getParameters()

@@ -0,0 +1,107 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package javax.faces.application;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import javax.faces.context.FacesContext;
+
+/**
+ * 
+ * @author Leonardo Uribe
+ *
+ */
+class _NavigationUtils
+{
+    /**
+     * Evaluate all EL expressions found as parameters and return a map that can be used for 
+     * redirect or render bookmark links
+     * 
+     * @param parameters parameter map retrieved from NavigationCase.getParameters()
+     * @return
+     */
+    public static Map<String, List<String> > getEvaluatedNavigationParameters(Map<String, List<String> > parameters)
+    {
+        Map<String,List<String>> evaluatedParameters = null;
+        if (parameters != null && parameters.size() > 0)
+        {
+            evaluatedParameters = new HashMap<String, List<String>>();
+            for (Map.Entry<String, List<String>> pair : parameters.entrySet())
+            {
+                boolean containsEL = false;
+                for (String value : pair.getValue())
+                {
+                    if (_isExpression(value))
+                    {
+                        containsEL = true;
+                        break;
+                    }
+                }
+                if (containsEL)
+                {
+                    evaluatedParameters.put(pair.getKey(), _evaluateValueExpressions(pair.getValue()));
+                }
+                else
+                {
+                    evaluatedParameters.put(pair.getKey(), pair.getValue());
+                }
+            }
+        }
+        else
+        {
+            evaluatedParameters = parameters;
+        }
+        return evaluatedParameters;
+    }
+    
+    /**
+     * Checks the Strings in the List for EL expressions and evaluates them.
+     * Note that the returned List will be a copy of the given List, because
+     * otherwise it will have unwanted side-effects.
+     * @param values
+     * @return
+     */
+    private static List<String> _evaluateValueExpressions(List<String> values)
+    {
+        // note that we have to create a new List here, because if we
+        // change any value on the given List, it will be changed in the
+        // NavigationCase too and the EL expression won't be evaluated again
+        List<String> target = new ArrayList<String>(values.size());
+        FacesContext context = FacesContext.getCurrentInstance();
+        for (String value : values)
+        {
+            if (_isExpression(value))
+            {
+                // evaluate the ValueExpression
+                value = context.getApplication().evaluateExpressionGet(context, value, String.class);
+            }
+            target.add(value);
+        }
+        return target;
+    }
+    
+    private static boolean _isExpression(String text)
+    {
+        return text.indexOf("#{") != -1;
+    }
+
+}

@@ -50,6 +50,7 @@
 
 import org.apache.myfaces.config.RuntimeConfig;
 import org.apache.myfaces.config.element.NavigationRule;
+import org.apache.myfaces.shared.application.NavigationUtils;
 import org.apache.myfaces.shared.util.HashMapUtils;
 import org.apache.myfaces.shared.util.StringUtils;
 import org.apache.myfaces.view.facelets.tag.jsf.PreDisposeViewEvent;
@@ -104,8 +105,12 @@ public void handleNavigation(FacesContext facesContext, String fromAction, Strin
                 ExternalContext externalContext = facesContext.getExternalContext();
                 ViewHandler viewHandler = facesContext.getApplication().getViewHandler();
                 String toViewId = navigationCase.getToViewId(facesContext);
-                String redirectPath = viewHandler.getRedirectURL(facesContext, toViewId, navigationCase.getParameters(),
-                                                                 navigationCase.isIncludeViewParams());
+                
+
+                String redirectPath = viewHandler.getRedirectURL(
+                        facesContext, toViewId, 
+                        NavigationUtils.getEvaluatedNavigationParameters(navigationCase.getParameters()) ,
+                        navigationCase.isIncludeViewParams());
                 
                 //Clear ViewMap if we are redirecting to other resource
                 UIViewRoot viewRoot = facesContext.getViewRoot(); 

@@ -768,7 +768,7 @@ private String encodeURL(String baseUrl, Map<String, List<String>> parameters)
             {
                 if (pair.getKey() != null && pair.getKey().trim().length() != 0)
                 {
-                    paramMap.put(pair.getKey(), _evaluateValueExpressions(pair.getValue()));
+                    paramMap.put(pair.getKey(), pair.getValue());
                 }
             }
         }
@@ -819,37 +819,6 @@ private String encodeURL(String baseUrl, Map<String, List<String>> parameters)
         return newUrl.toString();
     }
     
-    /**
-     * Checks the Strings in the List for EL expressions and evaluates them.
-     * Note that the returned List will be a copy of the given List, because
-     * otherwise it will have unwanted side-effects.
-     * @param values
-     * @return
-     */
-    private List<String> _evaluateValueExpressions(List<String> values)
-    {
-        // note that we have to create a new List here, because if we
-        // change any value on the given List, it will be changed in the
-        // NavigationCase too and the EL expression won't be evaluated again
-        List<String> target = new ArrayList<String>(values.size());
-        FacesContext context = FacesContext.getCurrentInstance();
-        for (String value : values)
-        {
-            if (_isExpression(value))
-            {
-                // evaluate the ValueExpression
-                value = context.getApplication().evaluateExpressionGet(context, value, String.class);
-            }
-            target.add(value);
-        }
-        return target;
-    }
-    
-    private boolean _isExpression(String text)
-    {
-        return text.indexOf("#{") != -1;
-    }
-    
     /**
      * @since 2.0
      */

@@ -65,6 +65,7 @@ public void tearDown() throws Exception
      * Tests if encodeRedirectURL() and encodeBookmarkableURL() correctly
      * evaluate ValueExpressions as parameters values.
      */
+    /* TODO: Invalid test, because EL evaluation should be done before call these methods.
     @Test
     @SuppressWarnings("unchecked")
     public void testEncodeURLHandlesValueExpressionParameters()
@@ -94,7 +95,7 @@ public void testEncodeURLHandlesValueExpressionParameters()
         Assert.assertTrue(bookmarkableUrl.contains("param1=literalvalue"));
         Assert.assertTrue(bookmarkableUrl.contains("param1=myvalue1"));
         Assert.assertTrue(bookmarkableUrl.contains("param2=myvalue2"));
-    }
+    }*/
 
     @Test
     public void testEncodeRedirectUrlWithEmptyParamInBaseUrl()

@@ -0,0 +1,107 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.myfaces.shared.application;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import javax.faces.context.FacesContext;
+
+/**
+ * 
+ * @author Leonardo Uribe
+ *
+ */
+public class NavigationUtils
+{
+    /**
+     * Evaluate all EL expressions found as parameters and return a map that can be used for 
+     * redirect or render bookmark links
+     * 
+     * @param parameters parameter map retrieved from NavigationCase.getParameters()
+     * @return
+     */
+    public static Map<String, List<String> > getEvaluatedNavigationParameters(Map<String, List<String> > parameters)
+    {
+        Map<String,List<String>> evaluatedParameters = null;
+        if (parameters != null && parameters.size() > 0)
+        {
+            evaluatedParameters = new HashMap<String, List<String>>();
+            for (Map.Entry<String, List<String>> pair : parameters.entrySet())
+            {
+                boolean containsEL = false;
+                for (String value : pair.getValue())
+                {
+                    if (_isExpression(value))
+                    {
+                        containsEL = true;
+                        break;
+                    }
+                }
+                if (containsEL)
+                {
+                    evaluatedParameters.put(pair.getKey(), _evaluateValueExpressions(pair.getValue()));
+                }
+                else
+                {
+                    evaluatedParameters.put(pair.getKey(), pair.getValue());
+                }
+            }
+        }
+        else
+        {
+            evaluatedParameters = parameters;
+        }
+        return evaluatedParameters;
+    }
+    
+    /**
+     * Checks the Strings in the List for EL expressions and evaluates them.
+     * Note that the returned List will be a copy of the given List, because
+     * otherwise it will have unwanted side-effects.
+     * @param values
+     * @return
+     */
+    private static List<String> _evaluateValueExpressions(List<String> values)
+    {
+        // note that we have to create a new List here, because if we
+        // change any value on the given List, it will be changed in the
+        // NavigationCase too and the EL expression won't be evaluated again
+        List<String> target = new ArrayList<String>(values.size());
+        FacesContext context = FacesContext.getCurrentInstance();
+        for (String value : values)
+        {
+            if (_isExpression(value))
+            {
+                // evaluate the ValueExpression
+                value = context.getApplication().evaluateExpressionGet(context, value, String.class);
+            }
+            target.add(value);
+        }
+        return target;
+    }
+    
+    private static boolean _isExpression(String text)
+    {
+        return text.indexOf("#{") != -1;
+    }
+
+}

@@ -63,6 +63,7 @@
 import javax.faces.model.SelectItem;
 import javax.faces.model.SelectItemGroup;
 
+import org.apache.myfaces.shared.application.NavigationUtils;
 import org.apache.myfaces.shared.component.DisplayValueOnlyCapable;
 import org.apache.myfaces.shared.component.EscapeCapable;
 import org.apache.myfaces.shared.config.MyfacesConfig;
@@ -1589,8 +1590,8 @@ public static String getOutcomeTargetHref(FacesContext facesContext,
             }
         }
         // handle NavigationCase parameters
-        Map<String, List<String>> navigationCaseParams = navigationCase
-                .getParameters();
+        Map<String, List<String>> navigationCaseParams = 
+            NavigationUtils.getEvaluatedNavigationParameters(navigationCase.getParameters());
         if (navigationCaseParams != null)
         {
             if (parameters == null)