You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@myfaces.apache.org by Erik Gustavson <go...@gmail.com> on 2006/06/18 08:54:05 UTC

wildcard navrule bug in NavigationHandlerImpl?

Hello all,

I was just testing out the SVN trunk in my application and I think a subtle
logic bug crept into NavigationHandlerImpl handling on wildcard matches
during this revision:

http://svn.apache.org/viewvc/myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/NavigationHandlerImpl.java?r1=410138&r2=414102&diff_format=h

Here is the scenario:

there are two navigation rules:

    <navigation-rule>
        <from-view-id>/foo.jsp</from-view-id>
        <navigation-case>
            <from-outcome>success</from-outcome>
            <to-view-id>/bar.jsp</to-view-id>
        </navigation-case>
    </navigation-rule>

    <navigation-rule>
        <navigation-case>
            <from-outcome>foobar</from-outcome>
            <to-view-id>/foobar.jsp</to-view-id>
        </navigation-case>
    </navigation-rule>

Assuming you are in the view /foo.jsp and your action method returns
"foobar" you would expect to end up matching on the wildcard navigation rule
and be sent to /foobar.jsp.

It looks like the logic on lines 71-72 and 75-79 are erroneous and should be
removed as they are present in the getNavigationCase(...) method called on
line 73. The net effect of the problem is that even if you find a good match
from line 73, if your viewId returns a list from the casesMap the code ends
up overwriting the previously determined navigation case with a null - this
send you right back to rerendering the view as if you nav case wasn't there
at all.

I took out the lines in question and tested locally and everything seems to
work for me...

-Erik


Diff:
Index:
core/impl/src/main/java/org/apache/myfaces/application/NavigationHandlerImpl.java
===================================================================
---
core/impl/src/main/java/org/apache/myfaces/application/NavigationHandlerImpl.java
(revision 415098)
+++
core/impl/src/main/java/org/apache/myfaces/application/NavigationHandlerImpl.java
(working copy)
@@ -68,18 +68,8 @@
             return;
         }

-        String viewId = facesContext.getViewRoot().getViewId();
-        Map casesMap = getNavigationCases(facesContext);
         NavigationCase navigationCase = getNavigationCase(facesContext,
fromAction, outcome);

-        List casesList = (List)casesMap.get(viewId);
-        if (casesList != null)
-        {
-            // Exact match?
-            navigationCase = calcMatchingNavigationCase(casesList,
fromAction, outcome);
-        }
-
-

Re: wildcard navrule bug in NavigationHandlerImpl?

Posted by Erik Gustavson <go...@gmail.com>.
Matthias,

I do not have permissions in Jira to assign issues, but I've added it as
MYFACES-1340 for you - http://issues.apache.org/jira/browse/MYFACES-1340

thanks,
Erik

On 6/18/06, Matthias Wessendorf <ma...@apache.org> wrote:
>
> To late for me to check it...
> can you bring it to the jira and asign the "bug" to me.
> Will take a look next days.
>
> thx,
> matthias
>
> --
> Matthias Wessendorf
> Aechterhoek 18
> 48282 Emsdetten
> blog: http://jroller.com/page/mwessendorf
> mail: mwessendorf-at-gmail-dot-com
>

Re: wildcard navrule bug in NavigationHandlerImpl?

Posted by Matthias Wessendorf <ma...@apache.org>.
To late for me to check it...
can you bring it to the jira and asign the "bug" to me.
Will take a look next days.

thx,
matthias

On 6/17/06, Erik Gustavson <go...@gmail.com> wrote:
> Hello all,
>
> I was just testing out the SVN trunk in my application and I think a subtle
> logic bug crept into NavigationHandlerImpl handling on wildcard matches
> during this revision:
>
> http://svn.apache.org/viewvc/myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/NavigationHandlerImpl.java?r1=410138&r2=414102&diff_format=h
>
> Here is the scenario:
>
> there are two navigation rules:
>
>     <navigation-rule>
>         <from-view-id>/foo.jsp</from-view-id>
>         <navigation-case>
>             <from-outcome>success</from-outcome>
>             <to-view-id>/bar.jsp</to-view-id>
>         </navigation-case>
>     </navigation-rule>
>
>     <navigation-rule>
>         <navigation-case>
>             <from-outcome>foobar</from-outcome>
>             <to-view-id>/foobar.jsp</to-view-id>
>         </navigation-case>
>     </navigation-rule>
>
> Assuming you are in the view /foo.jsp and your action method returns
> "foobar" you would expect to end up matching on the wildcard navigation rule
> and be sent to /foobar.jsp.
>
> It looks like the logic on lines 71-72 and 75-79 are erroneous and should be
> removed as they are present in the getNavigationCase(...) method called on
> line 73. The net effect of the problem is that even if you find a good match
> from line 73, if your viewId returns a list from the casesMap the code ends
> up overwriting the previously determined navigation case with a null - this
> send you right back to rerendering the view as if you nav case wasn't there
> at all.
>
> I took out the lines in question and tested locally and everything seems to
> work for me...
>
> -Erik
>
>
> Diff:
> Index:
> core/impl/src/main/java/org/apache/myfaces/application/NavigationHandlerImpl.java
> ===================================================================
> ---
> core/impl/src/main/java/org/apache/myfaces/application/NavigationHandlerImpl.java
>   (revision 415098)
> +++
> core/impl/src/main/java/org/apache/myfaces/application/NavigationHandlerImpl.java
>   (working copy)
> @@ -68,18 +68,8 @@
>              return;
>          }
>
> -        String viewId = facesContext.getViewRoot().getViewId();
> -        Map casesMap = getNavigationCases(facesContext);
>          NavigationCase navigationCase = getNavigationCase(facesContext,
> fromAction, outcome);
>
> -        List casesList = (List)casesMap.get(viewId);
> -        if (casesList != null)
> -        {
> -            // Exact match?
> -            navigationCase =
> calcMatchingNavigationCase(casesList, fromAction, outcome);
> -        }
> -
> -
>
>
>
>
>


-- 
Matthias Wessendorf
Aechterhoek 18
48282 Emsdetten
blog: http://jroller.com/page/mwessendorf
mail: mwessendorf-at-gmail-dot-com