net/http: multipart ReadForm fails to remove temporary files on io.Copy error

[Egojard]

I upload a rather large file to a http.Server as a value of a form in a http POST request. The server is a basic http.Server, set with its Server.ReadTimeout and Server.WriteTimeout at one second, i.e. way too short to handle the request. I am trying to get the file by calling
func (r *Request) FormFile(key string)

The function fails with a timeout error, which is perfectly expected, but it leaves in the temporary folder multipart-xxxxxx files. I think I have traced it back to mime\multipart\formdata.go, l 74:

file, err := ioutil.TempFile("", "multipart-")
            if err != nil {
                return nil, err
            }
            defer file.Close()
            _, err = io.Copy(file, io.MultiReader(&b, p))
            if err != nil {
                os.Remove(file.Name())
                return nil, err
            }

We notice that in this case, if io.Copy fails (which is what happens with my short timeouts) we are going to call os.Remove BEFORE calling file.Close, leading to a os.Remove failure. I confirmed this by adding some logs :

file, err := ioutil.TempFile("", "multipart-")
            if err != nil {
                log.Printf("Error at Tempfile: %+v\n", err)
                return nil, err
            }
            defer func() {
                log.Printf("File Closing : %s\n", file.Name())
                if fileCloseErr := file.Close(); fileCloseErr != nil {
                    log.Printf("File close failed : %+v\n", fileCloseErr)
                }
            }()
            _, err = io.Copy(file, io.MultiReader(&b, p))
            if err != nil {
                log.Printf("Error at io.Copy: %v\n", err)
                if osRemoveErr := os.Remove(file.Name()); osRemoveErr != nil {
                    log.Printf("os Remove failed: %+v\n", osRemoveErr)
                }
                return nil, err
            }

and this does confirm the error :

[2016-07-08 12:00:17] <Server> Error at io.Copy: multipart: Part Read: WSARecv tcp 127.0.0.1:8084: i/o timeout
[2016-07-08 12:00:17] <Server> os Remove failed: remove C:\cygwin64\tmp\multipart-183404555: Le processus ne peut pas accéder au fichier car ce fichier est utilisé par un autre processus.
[2016-07-08 12:00:17] <Server> File Closing : C:\cygwin64\tmp\multipart-183404555

First line is the expected timeout error
Second line is the attempt to remove the file (French part on second line being the classic Windows error "The process cannot access the file because it is being used by another process")
Third line is defered function closing the file after we tried to delete it.

1. What version of Go are you using (go version)?
   go 1.6.2 windows/amd64
2. What operating system and processor architecture are you using (go env)?
   windows 10 64 bits
   set GOARCH=amd64 set GOBIN= set GOEXE=.exe set GOHOSTARCH=amd64 set GOHOSTOS=windows set GOOS=windows
3. What did you do?
   Tried to parse a file sent through a HTTP POST rquest with func (r *Request) FormFile(key string) on a http.Server with timeouts too short for the operation to suceed.
4. What did you expect to see?
   multipart temporary files deleted
5. What did you see instead?
   multipart files remain in temp folder.

[Egojard]

Fix is basic, just manually close the file before os.Remove and move the defer file.Close() (or just call it manually a second time) below this first error management but I'd love to see this fixed for go 1.7. Given the time frame you may fix it faster on your side than if it gets processed as a first-time submission, but if you'd rather have me submit a patch I'll look into the process early next week.

[ianlancetaylor]

I think this is probably too late for 1.7, but @bradfitz can decide.

[bradfitz]

It's been like this for the past 7 releases, since Go 1.0 (https://github.com/golang/go/blob/release-branch.go1/src/pkg/mime/multipart/formdata.go#L78) so I think there's no rush doing this in Go 1.7.

It also only affects Windows, and leaving crap in $TMP should be fine in general: the operating system should be cleaning TMP regularly or as needed.

But we can fix it for Go 1.8.

[Egojard]

Alright, thanks for the feedback!

[odeke-em]

@Egojard might you be interested in mailing a CL for this fix since the tree is open for Go1.8?

[gopherbot]

CL https://golang.org/cl/30410 mentions this issue.

[gopherbot]

CL https://golang.org/cl/33639 mentions this issue.

[gopherbot]

CL https://golang.org/cl/33640 mentions this issue.