Context leaks when requests are nested

[GoogleCodeExporter]

If WroContextFilter runs multiple times on the same request (such as if there 
is a forward or error, and the filter mapping in web.xml specifies the REQUEST, 
ERROR, and FORWARD dispatchers), then Context will leak Context instances. 
Context.CONTEXT_MAP will keep accumulating Context mappings which are never 
removed.

The problem seems to be:
1) In the first filter invocation, 
https://github.com/alexo/wro4j/blob/1.7.x/wro4j-core/src/main/java/ro/isdc/wro/h
ttp/WroContextFilter.java#L60 calls Context.set, which creates a correlationId, 
let a be that correlationId, and stores a Context instance (let that instance 
be x) at Context.CONTEXT_MAP under (a, x)
2) Then, in the second invocation, 
https://github.com/alexo/wro4j/blob/1.7.x/wro4j-core/src/main/java/ro/isdc/wro/h
ttp/WroContextFilter.java#L60 creates a second correlationId (let b be that 
correlationId), and stores a Context instance (let that instance be y) at 
Context.CONTEXT_MAP under (b, y)
3) Popping the filter stack, 
https://github.com/alexo/wro4j/blob/1.7.x/wro4j-core/src/main/java/ro/isdc/wro/h
ttp/WroContextFilter.java#L63 runs calling Context.unset(), which removes 
Context.CONTEXT_MAP (b, y) and sets correlationId to null at 
https://github.com/alexo/wro4j/blob/1.7.x/wro4j-core/src/main/java/ro/isdc/wro/c
onfig/Context.java#L166
4) Popping the filter stack again, 
https://github.com/alexo/wro4j/blob/1.7.x/wro4j-core/src/main/java/ro/isdc/wro/h
ttp/WroContextFilter.java#L63 runs a second time, calling Context.unset(). 
correlationId is null at 
https://github.com/alexo/wro4j/blob/1.7.x/wro4j-core/src/main/java/ro/isdc/wro/c
onfig/Context.java#L163 . That's it... Context.CONTEXT_MAP(a, x) is abandoned, 
and can never be removed.

Original issue reported on code.google.com by candrews...@gmail.com on 12 Dec 2013 at 4:28

[GoogleCodeExporter]

Original comment by alex.obj...@gmail.com on 12 Dec 2013 at 4:31

• Added labels: Milestone-Release-1.7.3

[GoogleCodeExporter]

I think one possible fix is to change 
https://github.com/alexo/wro4j/blob/1.7.x/wro4j-core/src/main/java/ro/isdc/wro/h
ttp/WroContextFilter.java#L55 from this:

  public final void doFilter(final ServletRequest req, final ServletResponse res, final FilterChain chain)
      throws IOException, ServletException {
    final HttpServletRequest request = (HttpServletRequest) req;
    final HttpServletResponse response = (HttpServletResponse) res;
    try {
      Context.set(Context.webContext(request, response, this.filterConfig), getWroConfiguration());
      chain.doFilter(request, response);
    } finally {
      Context.unset();
    }
  }

to this:

  public final void doFilter(final ServletRequest req, final ServletResponse res, final FilterChain chain)
      throws IOException, ServletException {
    final HttpServletRequest request = (HttpServletRequest) req;
    final HttpServletResponse response = (HttpServletResponse) res;
    Context.set(Context.webContext(request, response, this.filterConfig), getWroConfiguration());
    final String correlationId = Context.getCorrelationId();
    try {
      chain.doFilter(request, response);
    } finally {
      Context.setCorrelationId(correlationId);
      Context.unset();
    }
  }

Original comment by candrews...@gmail.com on 12 Dec 2013 at 4:45

[GoogleCodeExporter]

I committed a workaround for the current snapshot version (1.7.2-SNAPSHOT) of 
the grails plugin for testing purposes: 
https://github.com/wro4j/wro4j-grails-plugin/commit/cb45ec94c58a5ddb0bf132c48a93
30ca69495d48

Original comment by candrews...@gmail.com on 13 Dec 2013 at 1:06

[GoogleCodeExporter]

Thinking about it more, I think treating the Context as a stack makes more 
sense. So I propose this method:

  public final void doFilter(final ServletRequest req, final ServletResponse res, final FilterChain chain)
      throws IOException, ServletException {
    final HttpServletRequest request = (HttpServletRequest) req;
    final HttpServletResponse response = (HttpServletResponse) res;
    String oldCorrelationId = null;
    if(Context.isContextSet()) oldCorrelationId = Context.getCorrelationId();
    Context.set(Context.webContext(request, response, this.filterConfig), getWroConfiguration());
    final String correlationId = Context.getCorrelationId();
    try {
      chain.doFilter(request, response);
    } finally {
      Context.setCorrelationId(correlationId);
      Context.unset();
      if(oldCorrelationId != null) Context.setCorrelationId(oldCorrelationId);
    }
  }

Original comment by candrews...@gmail.com on 13 Dec 2013 at 6:17

[GoogleCodeExporter]

Submitted a pull request: https://github.com/alexo/wro4j/pull/166

Original comment by candrews...@gmail.com on 13 Dec 2013 at 6:57

[GoogleCodeExporter]

Fixed in branch 1.7.x. Thanks for the contribution!

Original comment by alex.obj...@gmail.com on 13 Dec 2013 at 10:06

• Changed state: Fixed