From 2c86eae9485cee7934431a83cd418b60358d141a Mon Sep 17 00:00:00 2001 From: RubenPari Date: Sat, 8 Aug 2026 08:29:00 +0200 Subject: [PATCH] fix(context): clean multipart temps after ServeHTTP net/http only RemoveAlls the original Request. Middleware that replaces c.Request via WithContext leaves /tmp/multipart-* from FormFile/MultipartForm orphaned. Clean c.Request at ServeHTTP end. Fixes #4278 --- context.go | 10 ++++++ context_test.go | 88 +++++++++++++++++++++++++++++++++++++++++++++++++ gin.go | 1 + 3 files changed, 99 insertions(+) diff --git a/context.go b/context.go index 1dc730e3..f4cb5931 100644 --- a/context.go +++ b/context.go @@ -117,6 +117,16 @@ func (c *Context) reset() { *c.skippedNodes = (*c.skippedNodes)[:0] } +// cleanupMultipartForm removes temporary files created by ParseMultipartForm +// on the request currently attached to the context. net/http only cleans the +// original request it passed into ServeHTTP; middleware that replaces +// c.Request (e.g. WithContext) leaves those temps orphaned otherwise (#4278). +func (c *Context) cleanupMultipartForm() { + if c.Request != nil && c.Request.MultipartForm != nil { + _ = c.Request.MultipartForm.RemoveAll() + } +} + // Copy returns a copy of the current context that can be safely used outside the request's scope. // This has to be used when the context has to be passed to a goroutine. func (c *Context) Copy() *Context { diff --git a/context_test.go b/context_test.go index e8d305e4..449bc4aa 100644 --- a/context_test.go +++ b/context_test.go @@ -274,6 +274,94 @@ func TestSaveUploadedFileWithPermissionFailed(t *testing.T) { require.Error(t, c.SaveUploadedFile(f, dst, mode)) } +func multipartRequestWithFile(t *testing.T, field, filename string, size int) (*http.Request, string) { + t.Helper() + var body bytes.Buffer + mw := multipart.NewWriter(&body) + w, err := mw.CreateFormFile(field, filename) + require.NoError(t, err) + _, err = w.Write(bytes.Repeat([]byte("x"), size)) + require.NoError(t, err) + require.NoError(t, mw.Close()) + req := httptest.NewRequest(http.MethodPost, "/upload", &body) + req.Header.Set("Content-Type", mw.FormDataContentType()) + return req, mw.FormDataContentType() +} + +func multipartTempPath(t *testing.T, fh *multipart.FileHeader) string { + t.Helper() + f, err := fh.Open() + require.NoError(t, err) + defer f.Close() + of, ok := f.(*os.File) + require.True(t, ok, "expected on-disk multipart part (*os.File) so temp path is observable; raise MaxMultipartMemory spill threshold if this fails") + return of.Name() +} + +func TestServeHTTPCleansMultipartFormAfterWithContext(t *testing.T) { + router := New() + router.MaxMultipartMemory = 32 // force temp file + + router.Use(func(c *Context) { + c.Request = c.Request.WithContext(context.WithValue(c.Request.Context(), struct{}{}, 1)) + c.Next() + }) + + var tmpPath string + router.POST("/upload", func(c *Context) { + fh, err := c.FormFile("file") + require.NoError(t, err) + tmpPath = multipartTempPath(t, fh) + _, err = os.Stat(tmpPath) + require.NoError(t, err, "temp file must exist during handler") + c.Status(http.StatusOK) + }) + + req, _ := multipartRequestWithFile(t, "file", "big.bin", 2048) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + assert.Equal(t, http.StatusOK, w.Code) + require.NotEmpty(t, tmpPath) + _, err := os.Stat(tmpPath) + assert.True(t, os.IsNotExist(err), "temp file %s must be removed after ServeHTTP when middleware used WithContext", tmpPath) +} + +func TestServeHTTPCleansMultipartFormWithoutRequestReplace(t *testing.T) { + router := New() + router.MaxMultipartMemory = 32 // force temp file + + var tmpPath string + router.POST("/upload", func(c *Context) { + fh, err := c.FormFile("file") + require.NoError(t, err) + tmpPath = multipartTempPath(t, fh) + _, err = os.Stat(tmpPath) + require.NoError(t, err, "temp file must exist during handler") + c.Status(http.StatusOK) + }) + + req, _ := multipartRequestWithFile(t, "file", "big.bin", 2048) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + assert.Equal(t, http.StatusOK, w.Code) + require.NotEmpty(t, tmpPath) + _, err := os.Stat(tmpPath) + assert.True(t, os.IsNotExist(err), "temp file %s must be removed after ServeHTTP", tmpPath) +} + +func TestContextCleanupMultipartFormNilSafe(t *testing.T) { + c, _ := CreateTestContext(httptest.NewRecorder()) + c.Request = nil + assert.NotPanics(t, func() { c.cleanupMultipartForm() }) + + c, _ = CreateTestContext(httptest.NewRecorder()) + c.Request, _ = http.NewRequest(http.MethodPost, "/", nil) + c.Request.MultipartForm = nil + assert.NotPanics(t, func() { c.cleanupMultipartForm() }) +} + // TestSaveUploadedFileToExistingDir is a regression test for issue #4622. // SaveUploadedFile must not call os.Chmod on a directory that already exists, // because the process may not own it (e.g. /tmp on Linux/macOS), where chmod diff --git a/gin.go b/gin.go index 2e033bf3..7c74fad1 100644 --- a/gin.go +++ b/gin.go @@ -671,6 +671,7 @@ func (engine *Engine) ServeHTTP(w http.ResponseWriter, req *http.Request) { engine.handleHTTPRequest(c) + c.cleanupMultipartForm() engine.pool.Put(c) }