When a controller method redirect to any url, the framework automatically appends all the model attributes in url query string even if those attributes are stored in session. [SPR-8636]

[spring-projects-issues]

Angel opened SPR-8636 and commented

Example

@Controller
@SessionAttributes({"context"})
public class AuthenticationAction {
    @RequestMapping("/login")
    public String login(@RequestParam(required=false, "errorCode") String errorCode,
                        @ModelAttribute("context") String context) {
        return "redirect:/action/login/" + context;
    }
}

This example will redirect to the url: .../action/login/{context}?context=value

The parameter "context" and its value should not appear in the url query string by default in this case because they are already in session.

---

Affects: 3.1 M2

Issue Links:

• Provide precise control over the attributes used in a redirect scenario [SPR-6796] #11462 Provide precise control over the attributes used in a redirect scenario ("duplicates")

1 votes, 2 watchers

[spring-projects-issues]

Rossen Stoyanchev commented

In this particular case you can use a URI template. Since "context" is used in the URL it won't be appended:

return "redirect:/action/login/{context}"

What does the rest of your controller look like? In particular I'm wondering when "context" is created and when it is removed from the session? Perhaps it is a candidate for the flash attribute mechanism planned in #11130?

[spring-projects-issues]

Thibaut Fagart commented

I'll second the original poster.
Also, it's inconsistent with other controller Method argument annotations like @RequestParam which don't add their params to the model.
for example

@RequestMapping(value = "/{sayOrShout}/hello", params = "name")
public String sayHello(@PathVariable("sayOrShout") String sayOrShout, @RequestParam("name") String name) {
    return "redirect:/{sayOrShout}/hello/"+name;
}

You can't use

"redirect:/{sayOrShout}/hello/{name}" 
```as name is not in the model, nor can you do 
```"redirect:/"+sayOrShout+"/hello/"+name 
```or you end up with the original issue.

The other workaround would be 

@RequestMapping(value = "/{sayOrShout}/hello", params = "name")
public String sayHello(@PathVariable("sayOrShout") String sayOrShout, @RequestParam("name") String name, ModelMap model) {
model.remove("sayOrShout");
return "redirect:/{sayOrShout}/hello/"+name;
}

But that doesn't look any nicer

[spring-projects-issues]

Rossen Stoyanchev commented

Thibaud, is "sayOrShout" a session attribute (the original poster's point)? In which case I wonder why you want it as a session attribute if it is available as a path variable?

Assuming "sayOrShout" is a session attribute, it would be appended as a query parameter to the redirect URL if you used:

"redirect:/" + sayOrShout + "/hello/" + name

However, it wouldn't be appended if you used:

"redirect:/{sayOrShout}/hello/" + name

And if the name is added to the model, it could look like this:

@RequestMapping(value = "/{sayOrShout}/hello", params = "name")
public String sayHello(@PathVariable("sayOrShout") String sayOrShout, @RequestParam("name") String name, ModelMap model) {
    model.addAttribute("name", name);
    return "redirect:/{sayOrShout}/hello/{name}";
}

Comparing to your last workaround sample I don't understand why "sayOrShout" is removed from the model. If "sayOrShout" is used in the redirect string, it needs to be in the model.

[spring-projects-issues]

Rossen Stoyanchev commented

Angel, not taking anything from the issue you've raised, I'm wondering if "context" needs to be a session attribute if it is present in the URL?

[spring-projects-issues]

Thibaut Fagart commented

Rossen thanks for the quick reply.

In my case (creating some sample mvc 3.1 apps for internal use), sayOrShout is only a @PathVariable , no session is involved.
However it still gets appended to the redirectUrl (which I didn't want) if I don't "consume" it by using an URI template (or unless I manually remove it from the model).

All your examples are correct, but look inconsistent to me : why would there be a need to handle differently @RequestParam injected values and @PathVariables ? why would one be injected in the model and not the other ?

I'm not sure which one is better (have both injected , or have none injected), but I believe it should be consistent.

Last as for the initial issue (having model attributes added to redirect urls unless you explicitly work around it) doesn't seem right, and you're probably going to break some applications as that behaviour is new with 3.1

[spring-projects-issues]

Thibaut Fagart commented

Rossen, as you noticed, I messed up my last example, it was intended to be

@RequestMapping(value = "/{sayOrShout}/hello", params = "name")
public String sayHello(@PathVariable("sayOrShout") String sayOrShout, @RequestParam("name") String name, ModelMap model) {
    model.remove("sayOrShout");
    return "redirect:/"+sayOrShout+"/hello/"+name;
}

[spring-projects-issues]

Rossen Stoyanchev commented

Thibaut, I'm guessing you're using 3.1 M2? Some improvements have been made since then to RedirectView -- specifically current request URI variables are not added to the model. They are only used for filling in URI variables (if any) in the redirect string.

If you use the latest code (3.1.0.BUILD-SNAPSHOT), then you won't see anything appended with this example:

@RequestMapping(value = "/{sayOrShout}/hello", params = "name")
public String sayHello(@PathVariable("sayOrShout") String sayOrShout, @RequestParam("name") String name, ModelMap model) {
    return "redirect:/"+sayOrShout+"/hello/"+name;
}

This should work as well:

@RequestMapping(value = "/{sayOrShout}/hello", params = "name")
public String sayHello(@PathVariable("sayOrShout") String sayOrShout, @RequestParam("name") String name, ModelMap model) {
    return "redirect:/{sayOrShout}/hello/"+name;
}

URI variables in the current URL are likely to be needed when creating a redirect URL. This is why they are made available if you use URI variables in the redirect string. If you prefer you can explicitly add all attributes required for the redirect URL to the model.

Last as for the initial issue (having model attributes added to redirect urls unless you explicitly work around it) doesn't seem right, and you're probably going to break some applications as that behaviour is new with 3.1

Appending model attributes to the URL as query parameters in RedirectView is not new to 3.1. It has existed for a while (see #5775, #5995, #11462). We plan to address these issues in 3.1. Ideally for a redirect you should start with an empty model and pick its content.

[spring-projects-issues]

Angel commented

Hi all, I was not expecting this issue to be that controversial :-)

Rossen, I understand your point that all the request parameters are lost when you redirect and you have to append them to the URL query string if you want to keep them for the next request. In my opinion that is a dodgy workaround and the framework should not do that by default. You can make it optional, but not enforce that because this is a limitation of the Java EE standard request life cycle scopes and also because that way it is not possible to maintain "clean" URLs. The real clean and elegant solution to this issue is to have a "synthetic" Flash Scope which sits between the Request and Session scopes which will make the implementation of the "Redirect-after-Post" pattern easier. Although the official implementation in the Spring MVC 3.1 version is under development, you can apply the extension that comes in the Spring MVC showcase by Keith Donald which you can find at: https://github.com/SpringSource/spring-mvc-showcase.

One of the problems I see is that I have not put the whole controller code and that is causing confusion:

@Controller
@SessionAttributes({"context"})
public class AuthenticationAction {

    /**
     * Method used to handle possible login error messages and disply the corresponding login page again
     *
     * @param errorCode
     * @param context
     * @param request
     * @return
     */
    @RequestMapping("/login")
    public String login(@RequestParam(required=false, value = "errorCode") String errorCode,
                        @ModelAttribute("context") String context
                        HttpServletRequest request) {

        if (StringUtils.isNotBlank(errorCode)) {
            if (errorCode.equals("1")) {
                FlashScope.setErrorMessageKey("error.authentication.invalid.user", request);
            }
        }

        context = StringUtils.isNotBlank(context) ? WebConstants.WebParams.SRC_NRM : context;
        return "redirect:/action/login/" + context;
    }

    /**
     * Method to display the login screen and setup the styles
     *
     * @param environment
     * @param session
     * @return
     */
    @RequestMapping("/login/{environment}")
    public String login(@PathVariable() String environment,
                        HttpSession session) {

        
        // Whatever I do with environment parameter code
        // ....

        return "authentication/login";
    }

When the fist controller method is executed, the resulting redirection URL is something like: ```
.../action/login/nrm?context=nrm

```.../action/login/nrm

IMHO, the feature to put the parameters in the URL query string by default should be removed or at least make it optional.

Cheers,
Angel.

[spring-projects-issues]

Rossen Stoyanchev commented

Angel, thanks for posting a more complete example.

Do I understand correctly that "context" is a session attribute added to the session by some code outside of this controller? In which case accepting an HttpSession argument might be simpler than the combination of @SessionAttributes and a @ModelAttribute argument. Actually a more common use case for using @SessionAttributes is when a controller needs to store a model attribute in the session over several requests. You would typically use a SessionStatus argument on the last request to signal the session attribute can be removed.

That aside in your case, "context" is injected specifically to be included in the redirect URL. Consider returning this instead:

return "redirect:/action/login/{context}";

Since "context" is in the model, the URI variable {context} will be substituted and as a result will not be appended.

IMHO, the feature to put the parameters in the URL query string by default should be removed or at least make it optional.

This feature has been available for a long time and it would be difficult to just change the default behavior without affecting applications. There is actually a way to disable it through the setExposeModelAttributes flag on RedirectView. There are several related improvements in the latest 3.1 including support for URI variables in redirect strings, flash scope, and precise control over the content of the model before a redirect.

[spring-projects-issues]

Angel commented

Thanks Rossen,
Your suggestion certainly worked, but it looks like magic to me, cause I don't understand how that small change made such a difference. Good to know though.

[spring-projects-issues]

Thibaut Fagart commented

Rossen, I just switched to the latest 3.1.0 SNAPSHOT published in the maven repository, and it indeed seems to work as expected.

[spring-projects-issues]

Rossen Stoyanchev commented

Angel, Thibaut, at last I have a more complete response to provide. There are several JIRA tickets that more or less discuss the same issues. To avoid repeating myself in all of them please read the following comments. It's also worth mentioning that flash attribute support is now available (#11130) in that may help in some redirect scenarios if use of the HTTP session is an option.