New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
net/http: multipart ReadForm fails to remove temporary files on io.Copy error #16296
Comments
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. |
I think this is probably too late for 1.7, but @bradfitz can decide. |
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. |
Alright, thanks for the feedback! |
@Egojard might you be interested in mailing a CL for this fix since the tree is open for Go1.8? |
CL https://golang.org/cl/30410 mentions this issue. |
CL https://golang.org/cl/33639 mentions this issue. |
CL https://golang.org/cl/33640 mentions this issue. |
…copy Always close the file regardless of whether the copy succeeds or fails. Pass along the close error if the copy succeeds Updates #16296 Fixes #17965 Change-Id: Ib394655b91d25750f029f17b3846d985f673fb50 Reviewed-on: https://go-review.googlesource.com/30410 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-on: https://go-review.googlesource.com/33639 Reviewed-by: Chris Broadfoot <cbro@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
…copy Always close the file regardless of whether the copy succeeds or fails. Pass along the close error if the copy succeeds Updates #16296 Change-Id: Ib394655b91d25750f029f17b3846d985f673fb50 Reviewed-on: https://go-review.googlesource.com/30410 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-on: https://go-review.googlesource.com/33640 Reviewed-by: Chris Broadfoot <cbro@golang.org>
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:
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 :
and this does confirm the error :
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.
go version
)?go 1.6.2 windows/amd64
go env
)?windows 10 64 bits
set GOARCH=amd64 set GOBIN= set GOEXE=.exe set GOHOSTARCH=amd64 set GOHOSTOS=windows set GOOS=windows
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.multipart temporary files deleted
multipart files remain in temp folder.
The text was updated successfully, but these errors were encountered: