AbstractCachingViewResolver - caching redirect views leads to memory leak [SPR-10065]

[spring-projects-issues]

Michał Jaśtak opened SPR-10065 and commented

When user uses URL prefixed with "redirect:" as the method invocation result in Controller, it is cached as the whole (with provided parameters) in AbstractCachingViewResolver. Because the parameters for redirect may vary for the same URL used in redirect, and HashMap based cache is used, that leads to memory leak.

PS: this problem exists also in 2.5.x, I didn't checked how far in the Spring history it reaches

---

Affects: 3.1.3

Reference URL: http://vard-lokkur.blogspot.com/2012/12/springs-web-mvc-redirect-to-memory-leak.html

Issue Links:

• Performance improvement on AbstractCachingViewResolver [SPR-3145] #7831 Performance improvement on AbstractCachingViewResolver

Referenced from: commits f19bc57, 9deaefe

Backported to: 3.1.4

1 votes, 7 watchers

[spring-projects-issues]

Rossen Stoyanchev commented

In the example from the referenced blog post, the same redirect URL can be expressed as a URI template, which would ensure the same view name is used as the cache key every time.

In other words instead of:

return "redirect:form.html?entityId=" + entityId;

Do this:

return "redirect:form.html?entityId={entityId}";

In the above example, entityId can be a model attribute or if it is present as a URI variable on the current request, then it'll work fine.

In Spring 3.1+ it is actually preferable to use RedirectAttributes:

@RequestMapping(method = RequestMethod.POST)
public String onPost(RedirectAttributes attrs) {
    ...
    attrs.addAttribute(entityId, 123);
    return "redirect:form.html;   // resulting URL has entityId=123
}

The above would also work before Spring 3.1 as long as entityId is in the model. However, using RedirectAttributes is preferable. See the reference documentation for more detail on that.

In summary I think AbstractCachingViewResolver does what it's expected to do. It has a flag to disable caching but that should not be necessary. The main concern is that the memory leak can occur if a redirect URL is constructed in the controller and ends being different every time. However, I don't see an easy way to detect that.

[spring-projects-issues]

Juergen Hoeller commented

It doesn't hurt to use a LinkedHashMap with a configurable cache limit (default 1024) here, similar to how we do it in CachingMetadataReaderFactory and NamedParameterJdbcTemplate. I have added this for 3.2 GA and 3.1.4 now.

However, Rossen's advice still applies: Ideally, don't build custom redirect URLs with such changing ids through String concatenation. Just like you don't concatenate values into SQL Strings either but rather use PreparedStatements.

Juergen

[spring-projects-issues]

Michał Jaśtak commented

@Rossen - I agree that redirect methods presented by you are a way better than one used by me, in fact should be used instead - thanks for recalling them here :) - It doesn't change the fact, that method used by me is allowed too, and shouldn't lead to memory leak anyway.

Thank you all for correcting this very fast!

[spring-projects-issues]

Adedayo E commented

@Rossen Please can you verify this is true "The above would also work before Spring 3.1 as long as entityId is in the model". A quick test i did on at least version 3.0.7 does not seem to validate the statement. Thanks!!

[spring-projects-issues]

Rossen Stoyanchev commented

Adedayo E, it has always worked that way for primitive model attributes (int, long, etc). From the Javadoc of RedirectView:

By default all primitive model attributes (or collections thereof) are exposed as
HTTP query parameters (assuming they've not been used as URI template variables),
but this behavior can be changed by overriding the
isEligibleProperty(String, Object) method.

Also see RedirectTests.singleParam() for example.

[spring-projects-issues]

Adedayo E commented

@Rossen My comment relates to being able to achieve redirect:form.html?entityId={entityId} with previous Spring MVC versions (I use 3.0.7). My assumption was that your initial statement suggestions this was possible with previous versions. When I looked into the source of RedirectView (3.1.3) I see expandUriTemplateVariables = true. This is not present in version 3.0.7. Only commented because I spent some time trying to get it to work with 3.0.7, but it wasn’t working. My URIs look smtng like "redirect:/org/{orgId}/cars" vs "redirect:/org/"+orgId+"/cars". Thanks for the great work!

[spring-projects-issues]

Rossen Stoyanchev commented

URI variables are not supported in redirect URLs before 3.1. Apologies if my comment was not clear but "the above would also work before Spring 3.1" was in reference to the preceding example, not all examples.