mirror of
https://github.com/gin-gonic/gin.git
synced 2026-09-04 22:53:34 +08:00
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
This commit is contained in:
parent
34dac209ff
commit
2c86eae948
10
context.go
10
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 {
|
||||
|
||||
@ -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
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user