Skip to content

Spring request mapping annotation does not map an encoded URI correctly [SPR-10306] #14940

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
spring-projects-issues opened this issue Feb 15, 2013 · 11 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: declined A suggestion or change that we don't feel we should currently apply

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Feb 15, 2013

Alexander Hawley opened SPR-10306 and commented

When using the Spring request mapping annotation (org.springframework.web.bind.annotation.RequestMapping), the value attribute does not map an encoded URI correctly. Some encoded reserved characters are excluded.

For example, the slash character /, encoded as %2f, is excluded.

Results as follows.

List of strings which contain reserved characters:

List<String> paths = Arrays.asList(
    "foo%boo"
    ,"foo/boo"
    ,"foo?boo"
    ,"foo=boo"
    ,"foo&boo"
    ,"foo#boo"
    ,"foo$boo"
    ,"foo+boo"
    ,"foo,boo"
    ,"foo:boo"
    ,"foo;boo"
    ,"foo@boo"
);

Controller action to receive requests:

@RequestMapping(value = "/encoded/{value}", method = RequestMethod.GET)
public @ResponseBody String encoded_show() {
    return "encoded_show";
}

Results in the URIs & responses (when requested):

path: "foo%boo"
uri: "/encoded/foo%25boo"
controller_action: "encoded_show"

path: "foo/boo"
uri: "/encoded/foo%2Fboo"
controller_action: (404 Not Found)

path: "foo?boo"
uri: "/encoded/foo%3Fboo"
controller_action: "encoded_show"

path: "foo=boo"
uri: "/encoded/foo%3Dboo"
controller_action: "encoded_show"

path: "foo&boo"
uri: "/encoded/foo%26boo"
controller_action: "encoded_show"

path: "foo#boo"
uri: "/encoded/foo%23boo"
controller_action: "encoded_show"

path: "foo$boo"
uri: "/encoded/foo%24boo"
controller_action: "encoded_show"

path: "foo+boo"
uri: "/encoded/foo%2Bboo"
controller_action: "encoded_show"

path: "foo,boo"
uri: "/encoded/foo%2Cboo"
controller_action: "encoded_show"

path: "foo:boo"
uri: "/encoded/foo%3Aboo"
controller_action: "encoded_show"

path: "foo;boo"
uri: "/encoded/foo%3Bboo"
controller_action: "encoded_show"

path: "foo@boo"
uri: "/encoded/foo%40boo"
controller_action: "encoded_show"

In order to catch the request for foo/boo (encoded as foo%2Fboo), an additional controller action must be used with wildcard instead of path parameter:

@RequestMapping(value = "/encoded/**", method = RequestMethod.GET)
public @ResponseBody String encoded_any() {
    return "encoded_any";
}

Any ideas?

Thanks.

-AH


Affects: 3.1 GA

Issue Links:

1 votes, 3 watchers

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

URLEncoder is not for encoding URLs as its name would imply, it's actually for encoding HTML form data as its javadoc indicates. Try using UriUtils.encode(..).

@spring-projects-issues
Copy link
Collaborator Author

Alexander Hawley commented

I apologize for the confusion. URLEncoder doesn't have anything to do with this bug. I just used that library to generate test values in order to demonstrate the encoding problem with RequestMapping.

The bug is that RequestMapping doesn't map an encoded URI correctly.

@spring-projects-issues
Copy link
Collaborator Author

Alexander Hawley commented

I have removed the unrelated code, hopefully the issue is clearer now.

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

Once the request URI is decoded, the additional "/" from "foo%25boo", means the lookup path is now "/encoded/foo/boo" and it's no longer possible to interpret without treating it as a path segment.

The only thing you could do is set the "urlDecode" property of your HandlerMapping to false in which case the lookup path will remain "/encoded/foo%25boo" while the path variable will still be decoded and so value will be "foo/bar". Keep in mind this property will affect all other request mappings in the same way.

@spring-projects-issues
Copy link
Collaborator Author

Alexander Hawley commented

Thanks for the follow-up.

That was my diagnosis as well. This is what is causing the bug. The URI is decoded incorrectly.

If the URI template libraries were modified to use per-parameter encoding and decoding, then you could encode anything in a template parameter.

A slash encoded as %2F in a path segment is allowed as per HTTP/URI specifications.

I only tested the reserved characters.

(Edit: grammar)

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

Hm, I am not sure we're actually saying the same thing.. The URI is decoded correctly. Decoding happens by default for all incoming requests, because it's easier to express mappings that don't have encoded characters. In your case, the decoding actually gets in the way. Hence my recommendation to change the default setting of the "urlDecode" property of the HandlerMapping. That will turn off decoding. In other words, this is all expected behavior, not a bug. Hope it makes sense.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Mar 1, 2013

Alexander Hawley commented

You mean to say that you expect the URI template to work this way, requiring a toggle for all-or-nothing decoding?

I don't see why the template parameter has anything to do with wanting encoded or decoded text in the rest of the template.

That is not how it works in other MVC or URI templating systems.

Not to mention, regardless of the toggle switch for request mappings, the URI templates built in the tag libs do not coincide with this behavior.

From an implementation perspective, a developer might want to have any of these combinations:

template: /movies/{id}
matches: /movies/1
matches: /movies/foo
matches: /movies/fooboo
matches: /movies/foo%2Fboo

template: /movie showtimes/{id}
matches: /movie%20showtimes/1
matches: /movie%20showtimes/foo
matches: /movie%20showtimes/fooboo
matches: /movie%20showtimes/foo%2Fboo

template: /movie%20showtimes/{id}
matches: /movie%20showtimes/1
matches: /movie%20showtimes/foo
matches: /movie%20showtimes/fooboo
matches: /movie%20showtimes/foo%2Fboo

template: /movie+showtimes/{id}
matches: /movie%20showtimes/1
matches: /movie%20showtimes/foo
matches: /movie%20showtimes/fooboo
matches: /movie%20showtimes/foo%2Fboo

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

You mean to say that you expect the URI template to work this way, requiring a toggle for all-or-nothing decoding?

Correct. The incoming URL is encoded (e.g. "/foo%20bar"). Request mapping patterns on the other hand are usually decoded (e.g. @RequestMapping("/foo bar")). In order to match the URL to a request mapping pattern, the URL must be decoded.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Mar 4, 2013

Alexander Hawley commented

Okay. I see your point. This functionality could be considered an improvement or feature request.

But what about the other related tickets marked as duplicates?

Even with the switch to make request mapping use encoded values, the Spring URI template library (for controller redirects) and the Spring tag lib (for view links/forms) do not encode URIs correctly. So if you toggle your request mappings to accept the URI /movies/foo%2Fboo for the template /movies/{id}, you can't actually generate the URI /movies/foo%2Fboo using Spring.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Feb 24, 2015

Rossen Stoyanchev commented

It should be possible to generate the URI from the client side using Spring but it could be easier. Please track #17347.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Feb 24, 2015

Rossen Stoyanchev commented

Sorry that was a link to the wrong ticket. See the resolution for #16028.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

No branches or pull requests

2 participants