[JIRA] Created: (STS-688) Resolving UrlBindings incorrectly: an unbound URL does not yield a 404 error

Resolving UrlBindings incorrectly: an unbound URL does not yield a 404 error
----------------------------------------------------------------------------

                 Key: STS-688
                 URL: http://www.stripesframework.org/jira/browse/STS-688
             Project: Stripes
          Issue Type: Bug
          Components: ActionBean Dispatching
    Affects Versions: Release 1.5.1
            Reporter: Oscar Westra van Holthe - Kind
            Priority: Minor


I have several ActionBean classes bound with these:
@UrlBinding("/")
@UrlBinding("/search/{text}")
@UrlBinding("/profile")
@UrlBinding("/admin")
@UrlBinding("/admin/{username}")

In addition to these, I have custom error pages defines for HTTP codes 403, 404 and 500.

When I navigate to /foobar I get the page for /, instead of the custom 404 error I expected.

What goes wrong here is that the closest match, @UrlBinding("/"), has no parameters. Therefore, it cannot match.
The bug probably is in UrlBindingFactory.getBindingPrototype(String path); it should check if a binding can match the URI. This probably only means an added check to see if the binding has a parameter that can be filled in.

--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://www.stripesframework.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

       

------------------------------------------------------------------------------
Register Now for Creativity and Technology (CaT), June 3rd, NYC. CaT
is a gathering of tech-side developers & brand creativity professionals. Meet
the minds behind Google Creative Lab, Visual Complexity, Processing, &
iPhoneDevCamp as they present alongside digital heavyweights like Barbarian
Group, R/GA, & Big Spaceship. http://p.sf.net/sfu/creativitycat-com 
_______________________________________________
Stripes-development mailing list
Stripes-development@...
https://lists.sourceforge.net/lists/listinfo/stripes-development

    [ http://www.stripesframework.org/jira/browse/STS-688?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=11704#action_11704 ]

Oscar Westra van Holthe - Kind commented on STS-688:
----------------------------------------------------

A question that came to me this morning: if a URL binding has no parameters, should it be available for prefix matching at all?

If the answer is no, then my previous assessment is wrong.

> Resolving UrlBindings incorrectly: an unbound URL does not yield a 404 error
> ----------------------------------------------------------------------------
>
>                 Key: STS-688
>                 URL: http://www.stripesframework.org/jira/browse/STS-688
>             Project: Stripes
>          Issue Type: Bug
>          Components: ActionBean Dispatching
>    Affects Versions: Release 1.5.1
>            Reporter: Oscar Westra van Holthe - Kind
>            Priority: Minor
>
> I have several ActionBean classes bound with these:
> @UrlBinding("/")
> @UrlBinding("/search/{text}")
> @UrlBinding("/profile")
> @UrlBinding("/admin")
> @UrlBinding("/admin/{username}")
> In addition to these, I have custom error pages defines for HTTP codes 403, 404 and 500.
> When I navigate to /foobar I get the page for /, instead of the custom 404 error I expected.
> What goes wrong here is that the closest match, @UrlBinding("/"), has no parameters. Therefore, it cannot match.
> The bug probably is in UrlBindingFactory.getBindingPrototype(String path); it should check if a binding can match the URI. This probably only means an added check to see if the binding has a parameter that can be filled in.

--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://www.stripesframework.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

       

------------------------------------------------------------------------------
OpenSolaris 2009.06 is a cutting edge operating system for enterprises
looking to deploy the next generation of Solaris that includes the latest
innovations from Sun and the OpenSource community. Download a copy and
enjoy capabilities such as Networking, Storage and Virtualization.
Go to: http://p.sf.net/sfu/opensolaris-get
_______________________________________________
Stripes-development mailing list
Stripes-development@...
https://lists.sourceforge.net/lists/listinfo/stripes-development

     [ http://www.stripesframework.org/jira/browse/STS-688?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Oscar Westra van Holthe - Kind updated STS-688:
-----------------------------------------------

    Attachment: STS-688.patch

As an additional test, /profile/gg also yields the page mapped to /profile.
This is not intended (@UrlBinding("/profile") has no parameters and cannot match anything but /profile).

As a solution, I propose to ignore bindings without parameters, like so (a unified diff is attached as a patch):

$ diff -w UrlBindingFactory.java.orig UrlBindingFactory.java
464a465,471
>         List<UrlBinding> parameters = binding.getParameters();
>         if (binding != null && !binding.getParameters().isEmpty())
>         {
>             // Ignore bindings without parameters: they are static, and hence already match.
>                       // Also, as an example, a binding of @UrlBinding("/profile") should not match "/profile/gg".
>                       // Fixes issue STS-688.
>
474a482
>         }

> Resolving UrlBindings incorrectly: an unbound URL does not yield a 404 error
> ----------------------------------------------------------------------------
>
>                 Key: STS-688
>                 URL: http://www.stripesframework.org/jira/browse/STS-688
>             Project: Stripes
>          Issue Type: Bug
>          Components: ActionBean Dispatching
>    Affects Versions: Release 1.5.1
>            Reporter: Oscar Westra van Holthe - Kind
>            Priority: Minor
>         Attachments: STS-688.patch
>
>
> I have several ActionBean classes bound with these:
> @UrlBinding("/")
> @UrlBinding("/search/{text}")
> @UrlBinding("/profile")
> @UrlBinding("/admin")
> @UrlBinding("/admin/{username}")
> In addition to these, I have custom error pages defines for HTTP codes 403, 404 and 500.
> When I navigate to /foobar I get the page for /, instead of the custom 404 error I expected.
> What goes wrong here is that the closest match, @UrlBinding("/"), has no parameters. Therefore, it cannot match.
> The bug probably is in UrlBindingFactory.getBindingPrototype(String path); it should check if a binding can match the URI. This probably only means an added check to see if the binding has a parameter that can be filled in.

--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://www.stripesframework.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

       

------------------------------------------------------------------------------
Crystal Reports - New Free Runtime and 30 Day Trial
Check out the new simplified licensing option that enables unlimited
royalty-free distribution of the report engine for externally facing
server and web deployment.
http://p.sf.net/sfu/businessobjects
_______________________________________________
Stripes-development mailing list
Stripes-development@...
https://lists.sourceforge.net/lists/listinfo/stripes-development

    [ http://www.stripesframework.org/jira/browse/STS-688?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=11711#action_11711 ]

Oscar Westra van Holthe - Kind commented on STS-688:
----------------------------------------------------

My tests:
- The patch works correctly against the code of both version 1.5.1 and a fresh subversion snapshot.
- The paths mentioned in the issue, that resolved to a non-matching bindings now yield a 404 error.
- Other paths, both with and without parameters, still resolve as they did.

I trust these tests completely test what I changed.

> Resolving UrlBindings incorrectly: an unbound URL does not yield a 404 error
> ----------------------------------------------------------------------------
>
>                 Key: STS-688
>                 URL: http://www.stripesframework.org/jira/browse/STS-688
>             Project: Stripes
>          Issue Type: Bug
>          Components: ActionBean Dispatching
>    Affects Versions: Release 1.5.1
>            Reporter: Oscar Westra van Holthe - Kind
>            Priority: Minor
>         Attachments: STS-688.patch
>
>
> I have several ActionBean classes bound with these:
> @UrlBinding("/")
> @UrlBinding("/search/{text}")
> @UrlBinding("/profile")
> @UrlBinding("/admin")
> @UrlBinding("/admin/{username}")
> In addition to these, I have custom error pages defines for HTTP codes 403, 404 and 500.
> When I navigate to /foobar I get the page for /, instead of the custom 404 error I expected.
> What goes wrong here is that the closest match, @UrlBinding("/"), has no parameters. Therefore, it cannot match.
> The bug probably is in UrlBindingFactory.getBindingPrototype(String path); it should check if a binding can match the URI. This probably only means an added check to see if the binding has a parameter that can be filled in.

--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://www.stripesframework.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

       

------------------------------------------------------------------------------
Crystal Reports - New Free Runtime and 30 Day Trial
Check out the new simplified licensing option that enables unlimited
royalty-free distribution of the report engine for externally facing
server and web deployment.
http://p.sf.net/sfu/businessobjects
_______________________________________________
Stripes-development mailing list
Stripes-development@...
https://lists.sourceforge.net/lists/listinfo/stripes-development

    [ http://www.stripesframework.org/jira/browse/STS-688?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=11810#action_11810 ]

Ben Gunter commented on STS-688:
--------------------------------

It was intentionally designed this way. Even basic servlet mappings behave this way. Whatever you bind the servlet to effectively acts as a prefix, and everything after that prefix is accessible by calling HttpServletRequest.getPathInfo(). There is also one little-known feature of Stripes that would be broken if "extra path info" were ignored. That is, the event name may be specified in the request path, as in /path/to/bean/eventName instead of using parameters. Because it would break backward compatibility and generally oppose how servlets have always worked, I don't think we can make this change.

> Resolving UrlBindings incorrectly: an unbound URL does not yield a 404 error
> ----------------------------------------------------------------------------
>
>                 Key: STS-688
>                 URL: http://www.stripesframework.org/jira/browse/STS-688
>             Project: Stripes
>          Issue Type: Bug
>          Components: ActionBean Dispatching
>    Affects Versions: Release 1.5.1
>            Reporter: Oscar Westra van Holthe - Kind
>            Priority: Minor
>         Attachments: STS-688.patch
>
>
> I have several ActionBean classes bound with these:
> @UrlBinding("/")
> @UrlBinding("/search/{text}")
> @UrlBinding("/profile")
> @UrlBinding("/admin")
> @UrlBinding("/admin/{username}")
> In addition to these, I have custom error pages defines for HTTP codes 403, 404 and 500.
> When I navigate to /foobar I get the page for /, instead of the custom 404 error I expected.
> What goes wrong here is that the closest match, @UrlBinding("/"), has no parameters. Therefore, it cannot match.
> The bug probably is in UrlBindingFactory.getBindingPrototype(String path); it should check if a binding can match the URI. This probably only means an added check to see if the binding has a parameter that can be filled in.

--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://www.stripesframework.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

       

------------------------------------------------------------------------------
Come build with us! The BlackBerry(R) Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9 - 12, 2009. Register now!
http://p.sf.net/sfu/devconference
_______________________________________________
Stripes-development mailing list
Stripes-development@...
https://lists.sourceforge.net/lists/listinfo/stripes-development

    [ http://www.stripesframework.org/jira/browse/STS-688?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=11823#action_11823 ]

Oscar Westra van Holthe - Kind commented on STS-688:
----------------------------------------------------

I read the servlet specification (version 2.5) again to see if I missed something.

In my interpretation of section SRV.11.2, the URL pattern "/profile" does not match "/profile/extraPath", while "/profile/*" does. "/profile/" is used as an exact match, just like "/profile" is. This means that the Stripes feature to always use prefix mapping is not according to the servlet specification.

However, I also made an error, in that "/" actually matches anything that otherwise cannot match. So when using the DynamicMappingFilter in Stripes, this mapping prevents 404 errors.

Assuming that not following the servlet specification is not bad, this leaves the following options:


1. Leave the implementation as it is

When we leave the implementation as it is, always using prefix mapping, the javadoc for the UrlBinding must be updated. The reason is twofold, one being that it only mentions prefix mapping like this:
        Clean URLs support both prefix mapping (/action/foo/{bar}), and extension mapping (/foo/{bar}.action).

In fact, the way I read the code is that /foo/{bar}.action is also used as a prefix! I doubt that this is intended.

Secondly, the event name is only documented to be handled like this:
        The special parameter name $event may be used to embed the event name in a clean URL.
        For example, given @UrlBinding("/foo/{$event}") the "bar" event could be invoked with the URL /foo/bar."

Note that no implementation of the ActionResolver interface mentions the prefix mapping either, and they would be a second place to look.


2. Make the URL bindings behave like documented

As documented now, URL bindings are an exact match. Not a match with extra path info, like when a servlet mapped to a URL pattern starting with a / and ending in /*. This means you'll need to use a parameter to get what otherwise would be extra path info, like this: @UrlBinding("/search/{text}"). The feature to specify the event name in the path must really be done like this: @UrlBinding("/profile/{$event}").


Obviously, adjusting the documentation is better for people who have become accustomed to the mismatch between documentation and implementation. I.e. backwards compatibility.

The current documentation however, is complex enough IMHO. Documenting the prefix mapping would further complicate things.

As a result, I would prefer an implementation that matches the current Javadoc.

> Resolving UrlBindings incorrectly: an unbound URL does not yield a 404 error
> ----------------------------------------------------------------------------
>
>                 Key: STS-688
>                 URL: http://www.stripesframework.org/jira/browse/STS-688
>             Project: Stripes
>          Issue Type: Bug
>          Components: ActionBean Dispatching
>    Affects Versions: Release 1.5.1
>            Reporter: Oscar Westra van Holthe - Kind
>            Assignee: Ben Gunter
>            Priority: Minor
>         Attachments: STS-688.patch
>
>
> I have several ActionBean classes bound with these:
> @UrlBinding("/")
> @UrlBinding("/search/{text}")
> @UrlBinding("/profile")
> @UrlBinding("/admin")
> @UrlBinding("/admin/{username}")
> In addition to these, I have custom error pages defines for HTTP codes 403, 404 and 500.
> When I navigate to /foobar I get the page for /, instead of the custom 404 error I expected.
> What goes wrong here is that the closest match, @UrlBinding("/"), has no parameters. Therefore, it cannot match.
> The bug probably is in UrlBindingFactory.getBindingPrototype(String path); it should check if a binding can match the URI. This probably only means an added check to see if the binding has a parameter that can be filled in.

--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://www.stripesframework.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

       

------------------------------------------------------------------------------
Come build with us! The BlackBerry(R) Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9 - 12, 2009. Register now!
http://p.sf.net/sfu/devconference
_______________________________________________
Stripes-development mailing list
Stripes-development@...
https://lists.sourceforge.net/lists/listinfo/stripes-development

    [ http://www.stripesframework.org/jira/browse/STS-688?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=11841#action_11841 ]

Oscar Westra van Holthe - Kind commented on STS-688:
----------------------------------------------------

A new alternative:

Allowing the binding @UrlBinding("") as a special case would also solve this, without the need to change anything else.

> Resolving UrlBindings incorrectly: an unbound URL does not yield a 404 error
> ----------------------------------------------------------------------------
>
>                 Key: STS-688
>                 URL: http://www.stripesframework.org/jira/browse/STS-688
>             Project: Stripes
>          Issue Type: Bug
>          Components: ActionBean Dispatching
>    Affects Versions: Release 1.5.1
>            Reporter: Oscar Westra van Holthe - Kind
>            Assignee: Ben Gunter
>            Priority: Minor
>         Attachments: STS-688.patch
>
>
> I have several ActionBean classes bound with these:
> @UrlBinding("/")
> @UrlBinding("/search/{text}")
> @UrlBinding("/profile")
> @UrlBinding("/admin")
> @UrlBinding("/admin/{username}")
> In addition to these, I have custom error pages defines for HTTP codes 403, 404 and 500.
> When I navigate to /foobar I get the page for /, instead of the custom 404 error I expected.
> What goes wrong here is that the closest match, @UrlBinding("/"), has no parameters. Therefore, it cannot match.
> The bug probably is in UrlBindingFactory.getBindingPrototype(String path); it should check if a binding can match the URI. This probably only means an added check to see if the binding has a parameter that can be filled in.

--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://www.stripesframework.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

       

------------------------------------------------------------------------------
Come build with us! The BlackBerry(R) Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9 - 12, 2009. Register now!
http://p.sf.net/sfu/devconference
_______________________________________________
Stripes-development mailing list
Stripes-development@...
https://lists.sourceforge.net/lists/listinfo/stripes-development