[Bug] IncludeViewParams in command action are not handled.

Splash Forums Rewrite Users [Bug] IncludeViewParams in command action are not handled.

This topic contains 14 replies, has 3 voices, and was last updated by  Lincoln Baxter III 2 years, 8 months ago.

Viewing 15 posts - 1 through 15 (of 15 total)
  • Author
    Posts
  • #24649

    djmj
    Participant

    Two only rules for testing:

    cb.addRule(Join.path("/foo").to("/faces/foo.xhtml));
    cb.addRule(Join.path("/foo/{bar}").to("faces/foo.xhtml"));

    Simple command component with action and includeViewParams are ignored by rewrite.

    <p:commandButton action="/faces/foo.xhtml?faces-redirect=true&includeViewParams=true"/>

    Results in following unmapped redirection:

    /faces/foo.xhtml?bar=barValue&x=value2 instead of /foo/barValue?x=value2

    2.0.5.final

    #24653

    Hmmm, interesting. There are a few things you could try:

    1. You could try to use a simple <h:commandButton> instead the PrimeFaces one

    2. You could try to refer to an action method which simply returns the navigation string instead of hardcoding it. Something like this:

    <p:commandButton action="#{something.navigate}"/>
    
    public class Something {
    
      public String navigate() {
        return "/faces/foo.xhtml?faces-redirect=true&includeViewParams=true";
      }
    
    }
    

    3. Could you try to remove the first rule for /foo. Perhaps this is caused by the fact that there are two rules for /faces/foo.xhtml.

    #24660

    djmj
    Participant

    This looks like a severe bug!

    3. Could you try to remove the first rule for /foo. Perhaps this is caused by the fact that there are two rules for /faces/foo.xhtml.

    I observed this behavior in many of my mappings which share rules to same view.

    Removing the first mapping resulted in correct link building.
    My multiple mapping rules for a single view worked fine in pretty-faces using xml mapping.

    Depending on the provided view-parameter’s the rule must be choosen that matches most of those view-parameters since it will be impossible to define a single rule for all combinations and targeted clean-urls.

    • This reply was modified 2 years, 11 months ago by  djmj.
    #24662

    Good to know that this is the root cause. Could you open a issue for that?

    https://github.com/ocpsoft/rewrite/issues

    #24663

    However, I’m not really sure if there are ways to handle all scenarios. What about this for example:

    cb.addRule(Join.path("/foo").to("/faces/something.xhtml));
    cb.addRule(Join.path("/bar").to("/faces/something.xhtml"));
    
    #24665

    BTW: Your second rule is missing a leading / in the to() part. Is this related to your problem?

    #24667

    djmj
    Participant

    cb.addRule(Join.path(“/foo”).to(“/faces/something.xhtml));
    cb.addRule(Join.path(“/bar”).to(“/faces/something.xhtml”));

    There would only an issue if both view-params foo and bar are present.
    Then the first rule could be applied as default.

    This problem is not bound to commandbutton, it happens for all navigation cases. Using h:link and so on.

    No the / is not missing in my real mapping.

    I dont have a github account right now. Could you file the issue please?, else I would create one.

    • This reply was modified 2 years, 11 months ago by  djmj.
    #24669

    I created the issue:

    https://github.com/ocpsoft/rewrite/issues/126

    Just a note regarding the example I posted. In this example there are no Rewrite parameters and not view parameters. So I’m not really sure how Rewrite should decide to which of the two paths (/foo and /foo) it should rewrite the URL in the outbound case. We have to discuss how to handle this consistently.

    #24672

    djmj
    Participant

    Oh yeah you are right there are no view-parameter in your example.

    Maybe a warning should be logged that there is a ambiguity for the used combination of view-parameters (including none).

    #25479

    djmj
    Participant

    Any updates on this issue?

    #25486

    Unfortunately not. AFAIK there is still no agreement how to implement this in a consistent way.

    #25503

    djmj
    Participant

    Hmm, is it not straightforward but I do not know how it looks behind the scene in rewrite?

    • The target ‘view-id’ is checked for its containing query-parameter keys.
    • The ‘best’ match of all rewrite rules to this target ‘view-id/url’ is searched. Here best match should be defined as the rule that is matching all of the containing query-parameter keys. If there are multiple rules matching the same unique combination a warning is printed on rewrite action and first rule is taken or the rule itself is ignored by initial configuration setup (with warning).

    Btw: Resolving of a rewritten url to its view-id works with multiple parameters and multiple rules, but not the other way around.

    Just my idea to this topic.

    I also do not understand lincolns id approach &org.ocpsoft.rewrite.join.id=1.

    #25504

    Rewrite basically tries to find a rule that is matching against the outbound URL. The first Join that matches wins. It is AFAIK not possible to implement something like “the best match” wins. And I doubt that this be easy and/or useful.

    Lincolns idea was to use an additional query parameter that you would have to add to the outcome to control which Join wins. See these rules as an example:

    .addRule(Join.path(“/foo”).to(“/faces/something.xhtml))
    .withId("foo")
    
    .addRule(Join.path(“/bar”).to(“/faces/something.xhtml”))
    .withId("bar")
    

    No you could choose which rule should apply for the outbound rewriting by returning something like this:

    public String navigate() {
      return "/faces/something.xhtml?faces-redirect=true&org.ocpsoft.rewrite.join.id=bar";
    }
    

    Joins would check for this query parameter. If it exists, they would match only if the outbound URL matches AND if the Join’s id matches the id from the query parameter.

    #25506

    djmj
    Participant

    But in predecessor pretty-faces it worked without problems?

    Adding a “id” param to the query string is not a usefull aproach in my opinion.

    Since the query-parameters that will be included in the outcome must be known before invoking navigation.
    This defies the purpose of “includeViewParams”, as well as any other generic inclusion. In my application I seldom know all query parameters in the bean since they are always defined in the jsf markup.

    Using “org.ocpsoft.rewrite.join.id” is a security risk showing implementation informations in query url. But this key name could be custom configured.

    #25507

    Actually, @djmj, the ?org.ocpsoft.rewrite.join.id=blah is not a security risk because it is only used internally. It is removed before the URL is sent to the client. This is the same as using ?faces-redirect=true – JSF removes the parameter and it does not appear on the client side.

Viewing 15 posts - 1 through 15 (of 15 total)

You must be logged in to reply to this topic.

Comments are closed.