spring-projects / spring-projects/spring-framework

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.

status: waiting-for-triage
Dominant language
Java
Stars
60.2k
Forks
38.8k
Avg merge
5d 2h
Merged PRs (30d)
27

Description

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.

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.

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.

Assessment

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.