RedirectView does not URL encode expanded path variable [SPR-8646]

[spring-projects-issues]

!!Use pwebb rather than philw opened SPR-8646 and commented

When using RedirectView to redirect to a URL that includes a path variable the model value is not correctly URL encoded. This differs from the query parameter where URL encoding occurs.

Eg:

RedirectView view = new RedirectView("/ab/{cd}");
view.render(Collections.singletonMap("cd", "C D"), request, response);
// Currently throws URISyntaxException, Should redirect to /ab/C+D

---

Affects: 3.1 M2

Referenced from: commits 498d81f

[spring-projects-issues]

!!Use pwebb rather than philw commented

Also would it be possible to accept a null model argument and have it treated as an empty Map?

[spring-projects-issues]

!!Use pwebb rather than philw commented

Actually the description is wrong, the redirect should be /ab/C%20D. See UriUtils.encodePath()

[spring-projects-issues]

Rossen Stoyanchev commented

The challenge is with providing a consistent behavior. Up to now RedirectView only encoded appended query parameters. If the redirect URL was "/ab/C D", the controller would need to return "redirect:/ab/C%20D" to ensure the client gets an encoded redirect location. With redirect URI templates "/ab/{cd}" we have the option to introduce URI encoding by default without affecting existing applications. However you would now get encoding behavior depending on whether the redirect string has URI variables or not, which is inconsistent.

We could add an option to enable URI encoding in RedirectView and also in UrlBasedViewResolver so it can be enabled globally.

[spring-projects-issues]

Rossen Stoyanchev commented

Also would it be possible to accept a null model argument and have it treated as an empty Map?

It would contradict AbstractView#renderMergedOutputModel, which says the model is never null.

[spring-projects-issues]

!!Use pwebb rather than philw commented

This is my current work around:

   private String doCreateTargetUrl(Map<String, ?> model, HttpServletRequest request)
           throws UnsupportedEncodingException {
       Map<String, Object> mutableModel = new HashMap<String, Object>();
       if (model != null) {
           mutableModel.putAll(model);
       }

       // Work around Spring encoding bug SPR-8646
       String enc = this.encodingScheme;
       if (enc == null) {
           enc = request.getCharacterEncoding();
       }
       if (enc == null) {
           enc = WebUtils.DEFAULT_CHARACTER_ENCODING;
       }
       UriTemplate uriTemplate = createUriTemplate(getUrl(), enc);
       for (String variable : uriTemplate.getVariableNames()) {
           Object value = mutableModel.get(variable);
           value = UriUtils.encodePath(value == null ? "" : value.toString(), enc);
           mutableModel.put(variable, value);
       }

       return createTargetUrl(mutableModel, request);
   }

This will encode model elements when they are used as URL parameters but leave the existing behaviour of not encoding the complete URL.

For example:

new RedirectView("/a b/{cd}").render(new Model("cd","c d"),... = "/a b/c%20d"

Notice how the "a b" section has not been encoded but the model element "c d" has. This follows the existing behaviour of the RedirectView where any model item is encoded. Without this, the job of the developer becomes a bit more onerous as they need to know where model elements are used and hence if they need to be encoded or not before as they are stored in the model.

One other option that has been suggested is to have a flag to indicate that the entire RedirectURL should be encoded. This would break back compatibility if enabled by default, but perhaps could be an opt-in option.

new RedirectView(ENCODE_FULLY,"/a b/{cd}").render(new Model("cd","c d"),... = "/a%20b/c%20d"

Or perhaps new factory methods:

RenderView.nonEncoded("/a b/{cd}").ren...
RenderView.encoded("/a b/{cd}").ren...

[spring-projects-issues]

Rossen Stoyanchev commented

URI template variables in the target URL in RedirectView are now encoded. As pointed out already this is consistent with the existing behavior of RedirectView where values that come from the model (to expand URI variables or to be appended as query params) are encoded while the literal parts of the redirect URL are left as is.