Skip to content

Fix temp file leak in SaveMultipartFile on cross-device rename failure#2311

Merged
erikdubbelboer merged 1 commit into
valyala:masterfrom
itxaiohanglover:fix/save-multipart-file-temp-leak
Jul 4, 2026
Merged

Fix temp file leak in SaveMultipartFile on cross-device rename failure#2311
erikdubbelboer merged 1 commit into
valyala:masterfrom
itxaiohanglover:fix/save-multipart-file-temp-leak

Conversation

@itxaiohanglover

Copy link
Copy Markdown
Contributor

Summary

When os.Rename fails in SaveMultipartFile (e.g., source /tmp and destination on different filesystems), the copy fallback path leaves the original multipart temp file orphaned in /tmp.

This adds defer os.Remove(ff.Name()) after reopening the file for the copy path. The defer captures ff.Name() (the temp file path) before ff is reassigned to the destination file, ensuring cleanup on all return paths.

Fixes #1439

Background

As noted in #1439, Go's mime/multipart writes uploads exceeding 8KB to temp files in /tmp/multipart-*. SaveMultipartFile attempts os.Rename for an atomic move, but when that fails (cross-device), it falls back to copy — without removing the source temp file.

Test plan

  • Verify SaveMultipartFile still works when rename succeeds (no behavior change — early return before defer)
  • Verify temp file is removed after copy fallback
  • Verify temp file is removed if os.Create or copyZeroAlloc fails

When os.Rename fails (e.g., /tmp and destination on different filesystems),
the copy fallback path leaves the original multipart temp file orphaned
in /tmp. Add defer os.Remove(ff.Name()) to clean it up after copying.
@erikdubbelboer erikdubbelboer merged commit a26e974 into valyala:master Jul 4, 2026
18 of 19 checks passed
@erikdubbelboer

Copy link
Copy Markdown
Collaborator

Thanks!

erikdubbelboer pushed a commit that referenced this pull request Jul 5, 2026
* refactor: improve internal interface names

* test: fix host comparison in FuzzURIParse (#2313)

The fuzzer compares the parsed host against net/url and lowercases the
input to hide that fasthttp normalizes the host while net/url does not.
That trick cannot reach bytes hidden behind percent-encoding: "%4c"
survives ToLower and only decodes to 'L' afterwards, so fasthttp
returns "l" where net/url keeps "L". Found by the cifuzz job on an
unrelated pull request.

Lower the expectation instead, byte-wise via lowercaseBytes, since
strings.ToLower would mangle non-UTF-8 bytes like a decoded "%80".
Both hosts are added as seeds.

* perf: avoid redundant scans when parsing request headers (#2312)

* perf: avoid redundant scans when parsing request headers

RequestHeader.parse walks the header block twice before the lines are
actually parsed: readRawHeaders looks for the end of the block, then
headerScanner searches for the CRLFCRLF terminator once more. The
scanner now reuses the block end that readRawHeaders already found,
guarded by a check that the block really ends in CRLFCRLF. Blocks with
bare LF terminators fail that check and take the old search path, so
those odd cases keep behaving exactly as before.

While at it:

- trim header values with a plain loop, bytes.TrimLeft rebuilds its
  ASCII set on every call for the " \t" cutset
- drop the bytes.IndexByte(key, ' ') checks in front of
  normalizeHeaderKey, it already refuses keys with invalid bytes
  such as spaces
- simplify isValidHeaderKey, the noCanon flag changed nothing

benchstat (Apple M2 Pro, n=10):

RequestHeaderRead-12    79.48n ± 1%   63.22n ± 3%   -20.46% (p=0.000)
ServerGet1ReqPerConn-12 1.693µ ± 3%   1.671µ ± 1%   ~ (p=0.123)

* perf: reuse scanner work when parsing header lines

readContinuedLineSlice already scans every header line for the colon,
so hand that position to next() instead of finding it again with
bytes.Cut. Lines handed out there can never start with a space or tab
(the first line is rejected by the scanner, later ones are joined into
the previous header as continuations), so trimming never shifts the
position.

isValidHeaderKey now also reports whether the key contains a space that
survives trailing-whitespace trimming. The per-byte key loops in
parseHeaders and parseTrailer only existed to derive exactly that flag,
their reject branches were unreachable since the scanner validates
first. The response quirk that a space in a key closes the connection
is kept.

readLine loses its searchStart loop: the scanner truncates the block at
the terminator, so every line is guaranteed to end in \n.

benchstat (Apple M2 Pro, n=10), on top of the previous commit:

RequestHeaderRead-12    63.22n ± 3%   56.06n ± 1%   -11.33% (p=0.000)

and against master:

RequestHeaderRead-12    79.48n ± 1%   56.06n ± 1%   -29.48% (p=0.000)

* Fix temp file leak in SaveMultipartFile on cross-device rename failure (#2311)

When os.Rename fails (e.g., /tmp and destination on different filesystems),
the copy fallback path leaves the original multipart temp file orphaned
in /tmp. Add defer os.Remove(ff.Name()) to clean it up after copying.

* re-enable forcetypeassert (#2316)

* test: normalize Go test names (#2317)

* feat: prefix sentinel error strings (#2319)

* fix(pprofhandler): use exact path matching to prevent debug data exposure (#2302)

* fix(pprofhandler): use exact path matching to prevent debug data exposure

PprofHandler uses bytes.HasPrefix for routing, which means
/debug/pprof/cmdlineFoo matches the cmdline handler. This leaks
process command-line arguments to any path that starts with a valid
pprof endpoint prefix.

Replace HasPrefix with matchPprofPath, which requires an exact match
(or exact match + trailing slash, matching net/http/pprof's
DefaultServeMux behaviour). This prevents /debug/pprof/cmdlineFoo,
/debug/pprof/profileX, etc. from serving sensitive debug data.

Fixes #2258

* fix(pprofhandler): resolve lint issues — remove unused var, fix gci formatting, remove builtin shadow

- Remove unused pprofPrefix variable (gci unused linter)
- Fix gci import formatting in pprof.go and pprof_test.go
- Remove custom min() function that shadows predeclared builtin (gocritic builtinShadowDecl)
- Add missing trailing newline in pprof.go

Addresses maintainer review feedback on #2302.

* fix(pprofhandler): return 404 for paths with trailing slash

net/http/pprof does not register handlers for paths ending in /, so
requesting /debug/pprof/heap/ returns 404 via the default mux.

Update matchPprofPath to use strict equality only (no trailing slash
matching) and add an early 404 check in PprofHandler for any path
ending in /.

Renames test from AcceptsTrailingSlash to RejectsTrailingSlash and
updates matchPprofPath test to expect false for trailing slash.

---------

Co-authored-by: xbrxr03 <[email protected]>

* write fs compressed cache with os.CreateTemp to block symlink follow (#2321)

* refactor: improve internal interface names

* requestStreamHeader -> bodyStreamHeader

---------

Co-authored-by: RW <[email protected]>
Co-authored-by: artboy <[email protected]>
Co-authored-by: Md Abrar Ibn Habib <[email protected]>
Co-authored-by: xbrxr03 <[email protected]>
Co-authored-by: alhuda <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fasthttp.SaveMultipartFile() generate tmp file without clean

2 participants