Problem with curly braces while migrating from PrettyFaces 3.3.4 to Rewrite 2.0

[dparasam]

I am trying to migrate from PrettyFaces 3.3.4 to Rewrite 2.0 using the Rewrite PrettyFaces compatibility module.

For the most part it works, except for URLs with curly braces.

The annotation was something like this:
@com.ocpsoft.pretty.faces.annotation.URLMapping(
id = “some-page”,
pattern = “/myUrl/#{backingBean.param}”,
viewId = “/myUrl.xhtml”)

With PrettyFaces 3.3.4, a URL with curly braces in the parameter, for example, /myUrl/{curly}, was getting parsed but with Rewrite, the same URL throws this error:

No parameter [curly] was set in the pattern [/myUrl/{curly}]. Call address.set(“curly”, value); or remove the parameter from the pattern

I believe that PrettyFaces 3.3.3 couldn’t handle this as well but PrettyFaces 3.3.4 did.

For a more complete example, see http://ocpsoft.org/support/topic/problem-with-curly-braces-while-migrating-from-prettyfaces-3-3-4-to-rewrite-2-0/

[chkal]

I was able to reproduce this issue. And I started to work on a fix on a branch here:

https://github.com/ocpsoft/rewrite/tree/curly

However, I think this will take some time. Any help is welcome. ;)

[dparasam]

Any chance you have an ETA for the fix?
Thanks!

[lincolnthree]

So I've taken a look at this, and at Christian's work, and, there are two problems at hand. First... the problem where AddressBuilder was improperly treating them as a parameter, which Christian may have now resolved (still needs a little work to keep from having security problems.

Second, { and } are not valid characters for use in a URL. They must be encoded as: %7B and %7D. This makes me think that we really should (by default) be doing one of the following things to correct the URL. This can be done now using Rewrite's current feature set:

1. Fix URL encoding and use an internal Forward (the client never knows their URL was changed.)

    .addRule()
    .when(
       Path.matches("url")
    )
    .perform(
       Forward.to("url")
    )
    .where("url").matches("^(?!([!#$&-;=?-[]_a-z~]|%[0-9a-fA-F]{2})+)$").transposedBy(new URLCharacterEncodingTransposition())

1. Fix URL encoding and use a client redirect. (Example: http://stackoverflow.com/test{}example --> Redirects to http://stackoverflow.com/test%7B%7Dexample )

    .addRule()
    .when(
       Path.matches("{url}")
    )
    .perform(
       Redirect.to("url")
    )
    .where("url").matches("^(?!([!#$&-;=?-[]_a-z~]|%[0-9a-fA-F]{2})+)$").transposedBy(new URLCharacterEncodingTransposition())

1. Send 400 (Bad Request):

    .addRule()
    .when(
       Path.matches("url")
    )
    .perform(
       SendStatus.error(400);
    ).where("url").matches("^(?!([!#$&-;=?-[]_a-z~]|%[0-9a-fA-F]{2})+)$")

We could introduce a feature in Rewrite that simplifies this same type of correction, otherwise the URL is technically invalid and the server would need to throw a 500 error.

[lincolnthree]

On inspection of Christian's test, however, I see that the URL is actually encoded at the time it is sent, so there is another problem... hm.

[chkal]

Hey @lincolnthree. Thanks for having a look at this. I agree that my fix for the first problem isn't complete and/or perfect. This branch is more like a POC to see how easy it is to fix this.

There seem to be further problems in URLBuilder in regard to curly brace processing. But that class is actually deprecated. So perhaps we should try to get rid of it.

However, as there seem to be multiple places that we need to fix, we should perhaps address them one by one.

I think the first step would be to get this test green:

491cf97

Do you agree that this test should be green? There are encoded curly braces in the URL and the getPath() method should return the decode path. As curly braces are valid in the path (in encoded form), we should IMO no fail if there is no corresponding parameter. Especially because we are building an Address from an existing URL.

Do you think you could find some time in the next days to fix this in a clean way? I'm not very familiar with the way the parameter stuff in the AddressBuilder works. I think your are the regex king. ;)

Please ignore the curly branch. I'll remove it. The ignored tests are all in the master branch.