ContentCachingRequestWrapper doesn't cache multipart data

Open Beginner friendly
#36,668 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
65/100
Issue type
Documentation
Clarity
Mostly clear
Activity status
Quiet
Tech stack
java
Domain
documentation

Research direction

Start with ContentCachingRequestWrapper and AbstractRequestLoggingFilter, especially the beforeRequest and afterRequest behavior described in the issue. Document that multipart getPart/getParts data may be unavailable after DispatcherServlet cleanupMultipart removes temporary files, and clarify the resulting limitation for request logging; done means the relevant class or filter documentation explains this behavior.

Written by the indexing model from the issue text.

Description

status: waiting-for-triage

As the title says, ContentCachingRequestWrapper does not handle application/x-www-form-urlencoded requests. Particularly, the methods

public Collection<Part> getParts() throws IOException, ServletException;

public Part getPart(String name) throws IOException, ServletException;

from jakarta.servlet.http.HttpServletRequest are not implemented.

Taking AbstractRequestLoggingFilter (which uses ContentCachingRequestWrapper) as example, in its beforeRequest method, calling something like request.getPart("file").getInputStream().readAllBytes() gives the expected result. It's even possible to call it multiple times, so logging it here would be possible and it could still be consumed by the application later. However, this is true for Tomcat and maybe other containers, because they implement multipart handling via temporary files, but I don't think it's required by the spec.

The problem is, in the afterRequest method, ContentCachingRequestWrapper.getContentAsByteArray() is still available and gives the request body, but request.getPart("file").getInputStream() is not, but instead throws java.nio.file.NoSuchFileException. This is because of org.springframework.web.servlet.DispatcherServlet#cleanupMultipart, which deletes the temporary files before the filterChain.doFilter call in AbstractRequestLoggingFilter returns.

One workaround would be to have something like

class PartWrapper implements Part {
	// implement Part methods:
	// [...] 

	@Override
	public void delete() throws IOException {
		// don't delete
	}

	public void reallyDelete() throws IOException {
		part.delete();
	}
}

and

request = new ContentCachingRequestWrapper(request) {
	@Override
	public Collection<Part> getParts() throws IOException, ServletException {
		return super.getParts().stream().map(p -> (Part) new PartWrapper(p)).toList();
	}

	@Override
	public Part getPart(String name) throws IOException, ServletException {
		return new PartWrapper(super.getPart(name));
	}
};

and remember to call reallyDelete when the filter is done.

Other than that, I don't have a good/simple solution to this. As I think it's not an often needed feature, I think the main outcome of this issue should be to simply document this in ContentCachingRequestWrapper and/or AbstractRequestLoggingFilter.

Dominant language
Java
Stars
60.2k
Forks
38.8k
Avg merge
4d 11h
Merged PRs (30d)
30

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from spring-projects/spring-framework

All issues in spring-projects/spring-framework

Similar issues

More Java issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.