dev@javaserverfaces.java.net

Seeking Review: 1210 P2 Implicit Nav Query String Bug

From: Ed Burns <Ed.Burns_at_Sun.COM>
Date: Sat, 18 Jul 2009 22:03:15 -0400

https://javaserverfaces.dev.java.net/issues/show_bug.cgi?id=1210

Issue: 1210

This patch fixes several problems related to implicit navigation and
redirect.

SECTION: Modified Files
----------------------------
M jsf-ri/src/com/sun/faces/application/NavigationHandlerImpl.java

- REDIRECT_EQUALS_TRUE, INCLUDE_VIEW_PARAMS_EQUALS_TRUE

  As far as I can tell, the existing regexp didn't do exactly as
  expected. The simplified regexp, in concert with other changes, does.

- Here's the big ticket item in this change-bundle, the bit about the questionMark.

  From what I can tell, this wasn't catching the question mark in the
  case of an actual implicit navigation. I've added a systest for this
  case.

- Also, we need to gracefully handle the pathological case of the empty
  query string. This does so and includes a ProjectStage.Development too.

- The two finds for the REDIRECT_EQUALS_TRUE and
  INCLUDE_VIEW_PARAMS_EQUALS_TRUE are independent. Fix the if to make
  them so.

- Modify the m.group() to account for the new regexps

M jsf-ri/src/com/sun/faces/LogStrings.properties

- Add

+jsf.navigation_invalid_query_string=JSF1069: Invalid query string in navigation outcome {0}

I hope it's ok to just tack a new JSF**** message on to the end.

M jsf-ri/src/com/sun/faces/resources/Messages.properties
M jsf-ri/src/com/sun/faces/resources/Messages_en.properties
M jsf-ri/src/com/sun/faces/resources/Messages_fr.properties
M jsf-ri/src/com/sun/faces/resources/Messages_es.properties
M jsf-ri/src/com/sun/faces/resources/Messages_de.properties
M jsf-ri/src/com/sun/faces/util/MessageUtils.java

- Add

+com.sun.faces.NAVIGATION_INVALID_QUERY_STRING=Invalid query string in outcome''{0}''

M jsf-ri/systest/src/com/sun/faces/systest/implicitnav/ImplicitNavigationTestCase.java
A jsf-ri/systest/web/implicitnav/implicitNavRedirect.xhtml
A jsf-ri/systest/web/implicitnav/implicitNavRedirect02.xhtml

- automated test.

SECTION: Diffs

Index: jsf-ri/src/com/sun/faces/LogStrings.properties
===================================================================
--- jsf-ri/src/com/sun/faces/LogStrings.properties (revision 7557)
+++ jsf-ri/src/com/sun/faces/LogStrings.properties (working copy)
@@ -113,6 +113,7 @@
 jsf.cannot_instantiate_component_error=JSF1068: Cannot instantiate component with component-type {0}
 jsf.application.legacy_facelet_viewhandler_detected=JSF1069: Disabling the JSF 2.0 Facelets ViewHandler as an older FaceletViewHandler, {0}, has been explicitly configured. \
   If this is not desired behavior, remove the older FaceletViewHandler and library from your application.
+jsf.navigation_invalid_query_string=JSF1069: Invalid query string in navigation outcome {0}
 # the following three messages are duplicated in javax.faces.LogStrings.properties
 jsf.context.exception.handler.log_before={0} caught during beforePhase() processing of {1} : UIComponent-ClientId={2}, Message={3}
 jsf.context.exception.handler.log_after={0} caught during afterPhase() processing of {1} : UIComponent-ClientId={2}, Message={3}
Index: jsf-ri/src/com/sun/faces/application/NavigationHandlerImpl.java
===================================================================
--- jsf-ri/src/com/sun/faces/application/NavigationHandlerImpl.java (revision 7557)
+++ jsf-ri/src/com/sun/faces/application/NavigationHandlerImpl.java (working copy)
@@ -91,8 +91,8 @@
      * Flag indicated the current mode.
      */
     private boolean development;
- private static final Pattern REDIRECT_EQUALS_TRUE = Pattern.compile("(?:\\?|&amp;|&)(faces-redirect=true(&|$))");
- private static final Pattern INCLUDE_VIEW_PARAMS_EQUALS_TRUE = Pattern.compile("(?:\\?|&amp;|&)(includeViewParams=true(&|$))");
+ private static final Pattern REDIRECT_EQUALS_TRUE = Pattern.compile("(.*)(faces-redirect=true)(.*)");
+ private static final Pattern INCLUDE_VIEW_PARAMS_EQUALS_TRUE = Pattern.compile("(.*)(includeViewParams=true)(.*)");
 
 
     // ------------------------------------------------------------ Constructors
@@ -467,23 +467,40 @@
         int questionMark = viewIdToTest.indexOf('?');
         String queryString;
         if (-1 != questionMark) {
- queryString = viewIdToTest.substring(questionMark);
- viewIdToTest = viewIdToTest.substring(0, questionMark);
+ int viewIdLen = viewIdToTest.length();
+ if (viewIdLen <= (questionMark+1)) {
+ if (logger.isLoggable(Level.SEVERE)) {
+ logger.log(Level.SEVERE, "jsf.navigation_invalid_query_string",
+ viewIdToTest);
+ }
+ if (development) {
+ String key;
+ Object[] params;
+ key = MessageUtils.NAVIGATION_INVALID_QUERY_STRING_ID;
+ params = new Object[]{viewIdToTest};
+ FacesMessage m = MessageUtils.getExceptionMessage(key, params);
+ m.setSeverity(FacesMessage.SEVERITY_WARN);
+ context.addMessage(null, m);
+ }
+ queryString = null;
+ viewIdToTest = viewIdToTest.substring(0, questionMark);
+ } else {
+ queryString = viewIdToTest.substring(questionMark + 1);
+ viewIdToTest = viewIdToTest.substring(0, questionMark);
 
- Matcher m = REDIRECT_EQUALS_TRUE.matcher(queryString);
- if (m.find()) {
- isRedirect = true;
- queryString = queryString.replace(m.group(1), "");
-
+ Matcher m = REDIRECT_EQUALS_TRUE.matcher(queryString);
+ if (m.find()) {
+ isRedirect = true;
+ queryString = queryString.replace(m.group(2), "");
+ }
                 m = INCLUDE_VIEW_PARAMS_EQUALS_TRUE.matcher(queryString);
                 if (m.find()) {
                     isIncludeViewParams = true;
- queryString = queryString.replace(m.group(1), "");
+ queryString = queryString.replace(m.group(2), "");
                 }
             }
 
             if (queryString != null && queryString.length() > 0) {
- queryString = queryString.substring(1);
                 String[] queryElements = Util.split(queryString, "&amp;|&");
                 for (int i = 0, len = queryElements.length; i < len; i ++) {
                     String[] elements = Util.split(queryElements[i], "=");
Index: jsf-ri/src/com/sun/faces/resources/Messages.properties
===================================================================
--- jsf-ri/src/com/sun/faces/resources/Messages.properties (revision 7557)
+++ jsf-ri/src/com/sun/faces/resources/Messages.properties (working copy)
@@ -112,6 +112,7 @@
 com.sun.faces.MODELUPDATE_ERROR=Model Update failure for value ''{0}'' in model''{1}''.
 com.sun.faces.NAVIGATION_NO_MATCHING_OUTCOME=Unable to find matching navigation case from view ID ''{0}'' for outcome ''{1}''
 com.sun.faces.NAVIGATION_NO_MATCHING_OUTCOME_ACTION=Unable to find matching navigation case from view ID ''{0}'' for action ''{1}'' with outcome ''{2}''
+com.sun.faces.NAVIGATION_INVALID_QUERY_STRING=Invalid query string in outcome''{0}''
 com.sun.faces.NAMED_OBJECT_NOT_FOUND_ERROR=Expression Error: Named Object: ''{0}'' not found.
 com.sun.faces.NOT_NESTED_IN_FACES_TAG_ERROR=Not nested in a UIComponentTag Error for tag with handler class: ''{0}''.
 com.sun.faces.NOT_NESTED_IN_TYPE_TAG_ERROR=The {0} tag should be nested within a tag that is associated with a component of type {1}.
Index: jsf-ri/src/com/sun/faces/resources/Messages_en.properties
===================================================================
--- jsf-ri/src/com/sun/faces/resources/Messages_en.properties (revision 7557)
+++ jsf-ri/src/com/sun/faces/resources/Messages_en.properties (working copy)
@@ -113,6 +113,7 @@
 com.sun.faces.NAMED_OBJECT_NOT_FOUND_ERROR=Expression Error: Named Object: ''{0}'' not found.
 com.sun.faces.NAVIGATION_NO_MATCHING_OUTCOME=Unable to find matching navigation case from view ID ''{0}'' for outcome ''{1}''
 com.sun.faces.NAVIGATION_NO_MATCHING_OUTCOME_ACTION=Unable to find matching navigation case from view ID ''{0}'' for action ''{1}'' with outcome ''{2}''
+com.sun.faces.NAVIGATION_INVALID_QUERY_STRING=Invalid query string in outcome''{0}''
 com.sun.faces.NOT_NESTED_IN_FACES_TAG_ERROR=Not nested in a UIComponentTag Error for tag with handler class: ''{0}''.
 com.sun.faces.NOT_NESTED_IN_TYPE_TAG_ERROR=The {0} tag should be nested within a tag that is associated with a component of type {1}.
 com.sun.faces.NO_DTD_FOUND_ERROR=Unable to locate DTD with PUBLIC ID ''{0}'' at path ''{1}''.
@@ -168,4 +169,4 @@
 
 com.sun.faces.JS_RESOURCE_WRITING_ERROR=Can''t write the JavaScript file to client.
 
-com.sun.faces.RESOURCE_TARGET_NOT_AVAILABLE="One or more resources have the target of ''{0}'', but no ''{0}'' component has been defined within the view.
\ No newline at end of file
+com.sun.faces.RESOURCE_TARGET_NOT_AVAILABLE="One or more resources have the target of ''{0}'', but no ''{0}'' component has been defined within the view.
Index: jsf-ri/src/com/sun/faces/resources/Messages_fr.properties
===================================================================
--- jsf-ri/src/com/sun/faces/resources/Messages_fr.properties (revision 7557)
+++ jsf-ri/src/com/sun/faces/resources/Messages_fr.properties (working copy)
@@ -105,6 +105,7 @@
 com.sun.faces.NULL_CONFIGURATION=No Configuration loaded for the application.
 com.sun.faces.NAVIGATION_NO_MATCHING_OUTCOME=Unable to find matching navigation case from view ID ''{0}'' for outcome ''{1}''
 com.sun.faces.NAVIGATION_NO_MATCHING_OUTCOME_ACTION=Unable to find matching navigation case from view ID ''{0}'' for action ''{1}'' with outcome ''{2}''
+com.sun.faces.NAVIGATION_INVALID_QUERY_STRING=Invalid query string in outcome''{0}''
 com.sun.faces.ERROR_OPENING_FILE=Can''t open configuration file: ''{0}''.
 com.sun.faces.ERROR_REGISTERING_DTD=Can''t register DTD: ''{0}''.
 com.sun.faces.INVALID_INIT_PARAM=Invalid value: ''{0}'', for initialization parameter: ''{1}''. Acceptable values are 'true' or 'false'. Defaulting to 'false'.
@@ -164,4 +165,4 @@
 com.sun.faces.JS_RESOURCE_WRITING_ERROR=Can''t write the JavaScript file to client.
 com.sun.faces.EVAL_ATTR_UNEXPECTED_TYPE=Evaluation of expression for attribute ''{0}'' resulted in unexpected type. Expected {1}, but received {2}.
 
-com.sun.faces.RESOURCE_TARGET_NOT_AVAILABLE="One or more resources have the target of ''{0}'', but no ''{0}'' component has been defined within the view.
\ No newline at end of file
+com.sun.faces.RESOURCE_TARGET_NOT_AVAILABLE="One or more resources have the target of ''{0}'', but no ''{0}'' component has been defined within the view.
Index: jsf-ri/src/com/sun/faces/resources/Messages_es.properties
===================================================================
--- jsf-ri/src/com/sun/faces/resources/Messages_es.properties (revision 7557)
+++ jsf-ri/src/com/sun/faces/resources/Messages_es.properties (working copy)
@@ -105,6 +105,7 @@
 com.sun.faces.NULL_CONFIGURATION=No Configuration loaded for the application.
 com.sun.faces.NAVIGATION_NO_MATCHING_OUTCOME=Unable to find matching navigation case from view ID ''{0}'' for outcome ''{1}''
 com.sun.faces.NAVIGATION_NO_MATCHING_OUTCOME_ACTION=Unable to find matching navigation case from view ID ''{0}'' for action ''{1}'' with outcome ''{2}''
+com.sun.faces.NAVIGATION_INVALID_QUERY_STRING=Invalid query string in outcome''{0}''
 com.sun.faces.ERROR_OPENING_FILE=Can''t open configuration file: ''{0}''.
 com.sun.faces.ERROR_REGISTERING_DTD=Can''t register DTD: ''{0}''.
 com.sun.faces.INVALID_INIT_PARAM=Invalid value: ''{0}'', for initialization parameter: ''{1}''. Acceptable values are 'true' or 'false'. Defaulting to 'false'.
@@ -165,4 +166,4 @@
 com.sun.faces.JS_RESOURCE_WRITING_ERROR=Can''t write the JavaScript file to client.
 com.sun.faces.EVAL_ATTR_UNEXPECTED_TYPE=Evaluation of expression for attribute ''{0}'' resulted in unexpected type. Expected {1}, but received {2}.
 
-com.sun.faces.RESOURCE_TARGET_NOT_AVAILABLE="One or more resources have the target of ''{0}'', but no ''{1}'' component has been defined within the view.
\ No newline at end of file
+com.sun.faces.RESOURCE_TARGET_NOT_AVAILABLE="One or more resources have the target of ''{0}'', but no ''{1}'' component has been defined within the view.
Index: jsf-ri/src/com/sun/faces/resources/Messages_de.properties
===================================================================
--- jsf-ri/src/com/sun/faces/resources/Messages_de.properties (revision 7557)
+++ jsf-ri/src/com/sun/faces/resources/Messages_de.properties (working copy)
@@ -103,6 +103,7 @@
 com.sun.faces.CONFIG_RENDERER_REGISTRATION_MISSING_RENDERKIT=Unable to register renderers with RenderKit using id {0}. RenderKit does not exist.
 com.sun.faces.NAVIGATION_NO_MATCHING_OUTCOME=Unable to find matching navigation case from view ID ''{0}'' for outcome ''{1}''
 com.sun.faces.NAVIGATION_NO_MATCHING_OUTCOME_ACTION=Unable to find matching navigation case from view ID ''{0}'' for action ''{1}'' with outcome ''{2}''
+com.sun.faces.NAVIGATION_INVALID_QUERY_STRING=Invalid query string in outcome''{0}''
 com.sun.faces.NULL_CONFIGURATION=No Configuration loaded for the application.
 com.sun.faces.ERROR_OPENING_FILE=Can''t open configuration file: ''{0}''.
 com.sun.faces.ERROR_REGISTERING_DTD=Can''t register DTD: ''{0}''.
@@ -164,4 +165,4 @@
 com.sun.faces.JS_RESOURCE_WRITING_ERROR=Can''t write the JavaScript file to client.
 com.sun.faces.EVAL_ATTR_UNEXPECTED_TYPE=Evaluation of expression for attribute ''{0}'' resulted in unexpected type. Expected {1}, but received {2}.
 
-com.sun.faces.RESOURCE_TARGET_NOT_AVAILABLE="One or more resources have the target of ''{0}'', but no ''{0}'' component has been defined within the view.
\ No newline at end of file
+com.sun.faces.RESOURCE_TARGET_NOT_AVAILABLE="One or more resources have the target of ''{0}'', but no ''{0}'' component has been defined within the view.
Index: jsf-ri/src/com/sun/faces/util/MessageUtils.java
===================================================================
--- jsf-ri/src/com/sun/faces/util/MessageUtils.java (revision 7557)
+++ jsf-ri/src/com/sun/faces/util/MessageUtils.java (working copy)
@@ -304,6 +304,8 @@
           "com.sun.faces.NAVIGATION_NO_MATCHING_OUTCOME";
     public static final String NAVIGATION_NO_MATCHING_OUTCOME_ACTION_ID =
           "com.sun.faces.NAVIGATION_NO_MATCHING_OUTCOME_ACTION";
+ public static final String NAVIGATION_INVALID_QUERY_STRING_ID =
+ "com.sun.faces.NAVIGATION_INVALID_QUERY_STRING";
         public static final String OUTCOME_TARGET_BUTTON_NO_MATCH =
                   "com.sun.faces.OUTCOME_TARGET_BUTTON_NO_MATCH";
         public static final String OUTCOME_TARGET_LINK_NO_MATCH =
Index: jsf-ri/systest/src/com/sun/faces/systest/implicitnav/ImplicitNavigationTestCase.java
===================================================================
--- jsf-ri/systest/src/com/sun/faces/systest/implicitnav/ImplicitNavigationTestCase.java (revision 7557)
+++ jsf-ri/systest/src/com/sun/faces/systest/implicitnav/ImplicitNavigationTestCase.java (working copy)
@@ -42,6 +42,8 @@
 import com.gargoylesoftware.htmlunit.html.HtmlSubmitInput;
 import com.sun.faces.htmlunit.AbstractTestCase;
 import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException;
+import com.gargoylesoftware.htmlunit.html.HtmlButtonInput;
+import com.gargoylesoftware.htmlunit.html.HtmlTextInput;
 
 public class ImplicitNavigationTestCase extends AbstractTestCase {
 
@@ -150,4 +152,62 @@
 
 
        }
+
+
+ public void testImplicitNavigationWithredirect() throws Exception {
+
+ HtmlPage page = getPage("/faces/implicitnav/implicitNavRedirect.xhtml");
+ String text;
+ final String SEARCH_TEXT = "SEARCH_TEXT";
+
+ // case 1, h:button, make sure the input value is lost.
+ HtmlTextInput input = (HtmlTextInput) page.getElementById("input");
+ input.setValueAttribute(SEARCH_TEXT);
+ HtmlButtonInput buttonButton = (HtmlButtonInput) page.getElementById("httpGet");
+
+ page = buttonButton.click();
+ text = page.asText();;
+ assertTrue(!text.contains(SEARCH_TEXT));
+
+ // case 2 h:commandButton that does redirect. Make sure a redirect is
+ // performed and the value is lost.
+ client.setRedirectEnabled(false);
+ boolean exceptionThrown = false;
+ page = getPage("/faces/implicitnav/implicitNavRedirect.xhtml");
+ input = (HtmlTextInput) page.getElementById("input");
+ input.setValueAttribute(SEARCH_TEXT);
+ HtmlSubmitInput submitButton = (HtmlSubmitInput) page.getElementById("httpPostRedirect");
+ try {
+ page = submitButton.click();
+ } catch (FailingHttpStatusCodeException e) {
+ assertEquals(302, e.getStatusCode());
+ exceptionThrown = true;
+ }
+ assertTrue(exceptionThrown);
+ client.setRedirectEnabled(true);
+ page = submitButton.click();
+ text = page.asText();;
+ assertTrue(!text.contains(SEARCH_TEXT));
+
+ // case 3 h:commandButton with empty query string
+ page = getPage("/faces/implicitnav/implicitNavRedirect.xhtml");
+ input = (HtmlTextInput) page.getElementById("input");
+ input.setValueAttribute(SEARCH_TEXT);
+ submitButton = (HtmlSubmitInput) page.getElementById("httpPostInvalidQueryString");
+ page = submitButton.click();
+ text = page.asText();
+ assertTrue(text.contains(SEARCH_TEXT));
+ assertTrue(text.contains("Invalid query string in outcome\'implicitNavRedirect02?\'"));
+
+ // case 4: h:commandButton, regular post.
+ page = getPage("/faces/implicitnav/implicitNavRedirect.xhtml");
+ input = (HtmlTextInput) page.getElementById("input");
+ input.setValueAttribute(SEARCH_TEXT);
+ submitButton = (HtmlSubmitInput) page.getElementById("httpPost");
+ page = submitButton.click();
+ text = page.asText();;
+ assertTrue(text.contains(SEARCH_TEXT));
+
+ }
+
 }
Index: jsf-ri/systest/web/implicitnav/implicitNavRedirect.xhtml
===================================================================
--- jsf-ri/systest/web/implicitnav/implicitNavRedirect.xhtml (revision 0)
+++ jsf-ri/systest/web/implicitnav/implicitNavRedirect.xhtml (revision 0)
@@ -0,0 +1,17 @@
+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
+<html xmlns="http://www.w3.org/1999/xhtml"
+ xmlns:h="http://java.sun.com/jsf/html"
+ xmlns:f="http://java.sun.com/jsf/core">
+<h:head>
+ <title>A Simple JavaServer Faces 2.0 View</title>
+</h:head>
+<h:body>
+ <h:form prependId="false">
+ <p>Enter first name: <h:inputText id="input" value="#{test1.stringProperty}" /></p>
+ <p><h:button id="httpGet" value="HTTP GET" outcome="implicitNavRedirect02" /></p>
+ <p><h:commandButton id="httpPostRedirect" value="HTTP POST with Redirect" action="implicitNavRedirect02?faces-redirect=true" /></p>
+ <p><h:commandButton id="httpPostInvalidQueryString" value="HTTP POST with invalid query string" action="implicitNavRedirect02?" /></p>
+ <p><h:commandButton id="httpPost" value="HTTP POST" action="implicitNavRedirect02" /></p>
+ </h:form>
+</h:body>
+</html>
Index: jsf-ri/systest/web/implicitnav/implicitNavRedirect02.xhtml
===================================================================
--- jsf-ri/systest/web/implicitnav/implicitNavRedirect02.xhtml (revision 0)
+++ jsf-ri/systest/web/implicitnav/implicitNavRedirect02.xhtml (revision 0)
@@ -0,0 +1,15 @@
+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
+<html xmlns="http://www.w3.org/1999/xhtml"
+ xmlns:h="http://java.sun.com/jsf/html">
+<h:head>
+ <title>Page 02</title>
+</h:head>
+<h:body>
+ <p>Page 02</p>
+ <p>Value of First name: &lt;#{test1.stringProperty}&gt;.</p>
+
+ <p>The value will be empty if either of the "HTTP POST with Redirect" or the
+ "HTTP GET" buttons were pressed. If the "HTTP POST" button was pressed,
+ the value will not be empty.</p>
+</h:body>
+</html>

--