Go version

go version go1.24.4 darwin/arm64

Output of go env in your module/workspace:

none

What did you do?

https://github.com/open-telemetry/opentelemetry-go-contrib/issues/5946#issuecomment-3028290240

What did you see happen?

Please check the relevant links.

What did you expect to see?

What I want to confirm is whether it is a underlying error, or if it is an issue arising from improper usage.

Comment From: dr2chase

@neild, @rsc, maybe this is for you?

Comment From: flc1125

@seankhliao I think this issue might not be related to user habits and should not have been closed.

When we call request.WithContext, most users are not aware that they need to execute request.ParseMultipartForm.RemoveAll(). This situation essentially goes beyond the scope of any framework.

Go provides these capabilities but does not offer best practices. Users thus don't understand how to properly handle them.

Improper handling may even lead to IO saturation, causing service abnormalities.

Therefore, I suggest reopening the issue to obtain further information.

The following situations exist (tested and confirmed):

  1. We executed request.WithContext() before executing request.ParseMultipartForm, but did not manually call RemoveAll on the new Request—it will be unable to delete the files;
  2. We executed request.WithContext() before executing request.ParseMultipartForm, and manually called RemoveAll on the new Request—it will delete the files;
  3. We executed request.WithContext() after executing request.ParseMultipartForm, and did not manually call RemoveAll on the new Request—it will delete the files.

Comment From: neild

Sorry for the delay in responding.

Let me preface this by saying that everything around multipart forms and http.Request is unfortunate, but changing anything is difficult without breaking existing use cases.

There are several Request fields and methods which deal with multipart form parsing, either explicitly or implicitly:

  1. Request.ParseMultipartForm parses a multipart form, which may result in some amount of form content being stored as temporary files on disk. A number of Request methods call ParseMultipartForm implicitly.
  2. Request.MultipartForm is a field containing the parsed multipart form.
  3. Request.WithContext makes a shallow copy of a request, copying over the previous MultipartForm value.
  4. Request.Clone makes a deep copy of a request. Confusingly, I think it makes a deep copy of the in-memory parts of the MultipartForm but retains the same on-disk files. (This isn't relevant to the current discussion, just mentioning it for completeness.)

In addition, there's an interaction with the HTTP server: When a server handler returns, if the Request passed to the handler has a MultipartForm field set the server calls MultipartForm.RemoveAll to delete the on-disk temporary files.

So if a middleware handler calls Request.WithContext, passes the new Request to a subsidiary handler, and the subsidiary handler calls Request.ParseMultipartForm (implicitly or explicitly), the server will not automatically delete the on-disk temporary files for the form after the handler returns, because the original request passed to the handler does not have its MultipartForm field set. The form parsing happened in a copy of that request, and the server doesn't know about that copy.

Changing this behavior seems complicated and subtle. Perhaps we could have some form of cleanup mechanism that spans cloned requests? I'm not sure what that would look like.

Working with the existing behavior (unfortunate though it may be), I'd recommend that a middleware layer that clones the request passed to a subsidiary handler either call MultipartForm.RemoveAll itself after the subsidiary handler returns, or copy the cloned Request.MultipartForm value back to the original request before returning.