Skip to content
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

Unable to retrieve FlashMap when contains "+"(half-space) in the request parameters [SPR-11821] #16441

Closed
spring-projects-issues opened this issue May 27, 2014 · 14 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: duplicate A duplicate of another issue

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented May 27, 2014

Kazuki Shimizu opened SPR-11821 and commented

Probably, URL decode processing are wrong in AbstractFlushMapManager#decodeParameters.

e.g.)
Controller's logic is follows :

redirectAttributes.addAttribute("ab", "a  b");
redirectAttributes.addAttribute("params[0]", "val0");
redirectAttributes.addAttribute("params[1]", "val1");

redirect url is follows:

/xxx?ab=a++b&params%5B0%5D=val0&params%5B1%5D=val1

FlashMap#getTargetRequestParams() is follows:

{ab=[a++b], params[0]=[val0], params[1]=[val1]}

I think value of "ab" parameter should be "a b".


Affects: 3.2.9

Reference URL: #16129

Issue Links:

0 votes, 5 watchers

@spring-projects-issues
Copy link
Collaborator Author

Eujung Kim commented

The parameter comparision logic in AbstractFlashMapManager.java has been fixed to use decoded value.
And I submitted pull request.
#565

@spring-projects-issues
Copy link
Collaborator Author

Kazuki Shimizu commented

Hi Eujung

Probably, your modification is not correct.
And parameter name have same problem.

e.g.1)
If specified '+' as request parameter as following, behavior is correct ?

redirectAttributes.addAttribute("key", "val+ue"); 

e.g.2)
If specified '%20' as request parameter as following, behavior is correct ?

redirectAttributes.addAttribute("key", "val%20ue"); 

If possible, please confirm application behavior.

@spring-projects-issues
Copy link
Collaborator Author

Kazuki Shimizu commented

Hi Rossen

I sent the PR to solve this issue.
Please review this PR and if OK, merge to master branch.
I hope backport to 3.2.x branch.

#583

@spring-projects-issues
Copy link
Collaborator Author

Kazuki Shimizu commented

Sorry.
I re-sent the PR to solve this issue.
#584

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Jul 7, 2014

Rossen Stoyanchev commented

The solution in the pull request with URLDecoder won't work because that class is meant for HTML form decoding and not URI decoding (check its Javadoc).

What's encoding the space as "+"? It should be encoded as %20.

For further background you can also check the comments under #10187.

@spring-projects-issues
Copy link
Collaborator Author

Kazuki Shimizu commented

In RedirectView#urlEncode(String, String), " " has encoded to "+" using java.net.URLEncoder.encode(String, String).
Should be modified the logic of RedirectView#urlEncode(String, String) ?

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Jul 7, 2014

Rossen Stoyanchev commented

Yes indeed. You can work around this by using a URI variable in the redirect string, e.g. "/xxx?ab={ab}", which is encoded through UriUtils. I'm not sure we can easily change the current behavior of RediretView#urlEncode without causing issues. It is a protected method so you can override it and return the RedirectView sub-class from your controller method.

Both of these of course are workarounds. I am going to link this to #16543 to be considered as part of a broader effort to address such issues.

@spring-projects-issues
Copy link
Collaborator Author

Kazuki Shimizu commented

You can work around this by using a URI variable in the redirect string,...

thank you for advice.

If override, following implementation is correct ?

RediretView#urlEncode

protected String urlEncode(String input, String encodingScheme) throws UnsupportedEncodingException {
    return (input != null ? URLEncoder.encode(input, encodingScheme) : null);
}

Sub-class

protected String urlEncode(String input, String encodingScheme) throws UnsupportedEncodingException {
    return (input != null ? UriUtils.encodeQueryParam(input, encodingScheme) : null);
}

@spring-projects-issues
Copy link
Collaborator Author

Kazuki Shimizu commented

I had tried the way to using a URI variable in the redirect string.
" " are encoded to "%20", but "+" not encoded to "%2B" ...
As path, "+" is not required the encoding.
But as query parameter, "+" is required the encoding.

Probably this behavior is bug ...

@spring-projects-issues
Copy link
Collaborator Author

Kazuki Shimizu commented

I want modify the RediretView#urlEncode implementation as following:
How do you think?
If OK, modify and send PR.

protected String urlEncode(String input, String encodingScheme) throws UnsupportedEncodingException {
    return (input != null ? UriUtils.encodeQueryParam(input, encodingScheme) : null);
}

@spring-projects-issues
Copy link
Collaborator Author

Kazuki Shimizu commented

I tried modify again. Please review this PR.
#586

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Jul 8, 2014

Rossen Stoyanchev commented

Thanks, what you did should work but is guaranteed to cause regressions to existing applications. As I indicated above we will need to find another way to address this, perhaps together with #16543 which is about configuring centrally URI encoding related options and using them consistently throughout the framework.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Jul 9, 2014

Kazuki Shimizu commented

I understand.
Until this issue(or #16543) resolved, apply a work around.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented May 7, 2015

Rossen Stoyanchev commented

This should be fixed after #17170 was resolved.

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: duplicate A duplicate of another issue
Projects
None yet
Development

No branches or pull requests

2 participants