Skip to content
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

x/net/http2: When using large FormFile under HTTP2 and TLS, /tmp will not be cleared. #48413

Closed
SmallTianTian opened this issue Sep 16, 2021 · 4 comments · May be fixed by golang/net#113
Closed
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.

Comments

@SmallTianTian
Copy link

What version of Go are you using (go version)?

$ go version
go version go1.17 darwin/arm64

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env

GO111MODULE="on"
GOARCH="arm64"
GOBIN="/Users/tick/go/bin"
GOCACHE="/Users/tick/Library/Caches/go-build"
GOENV="/Users/tick/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="arm64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/tick/go/pkg/mod"
GOOS="darwin"
GOPATH="/Users/tick/go"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/opt/homebrew/Cellar/go/1.17/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/opt/homebrew/Cellar/go/1.17/libexec/pkg/tool/darwin_arm64"
GOVCS=""
GOVERSION="go1.17"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/gy/5ff3jzqj6gj5jc38zq91vd2h0000gn/T/go-build1908200583=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

This is my code, please generate your own certificate

package main

import (
    "fmt"
    "net/http"
)

func main() {
    han := func(rw http.ResponseWriter, r *http.Request) {
        _, fh, err := r.FormFile("test")
        if err != nil {
            rw.Write([]byte(fmt.Sprintf("read from file failed, Err: %v", err)))
            return
        }
        rw.Write([]byte(fmt.Sprintf("total %d's byte", fh.Size)))
    }

    s := &http.Server{
        Addr:    ":8080",
        Handler: http.HandlerFunc(han),
    }
    s.ListenAndServeTLS("./server.crt", "./server.key")
}

You can upload a large form using the following command.

curl -k -F 'test=@/path/to/large/file' https://localhost:8080

Note: The file must be large enough to exceed 32M for the following reasons: request form to large will create tmp to save.

// too big, write to disk and flush buffer

What did you expect to see?

At the end of the HTTP request, files in /tmp will be destroyed.

What did you see instead?

Tmp file still exists.

@seankhliao
Copy link
Member

Duplicate of #20253

@seankhliao seankhliao marked this as a duplicate of #20253 Sep 16, 2021
@SmallTianTian
Copy link
Author

I've seen this issue before, but no one has tried to reproduce it in an HTTP2 + TLS environment.

Non-http2 +TLS cases will go to these code logic https://github.com/golang/go/blob/master/src/net/http/server.go#L1616-L1618 and the /tmp file will indeed be cleaned.
But in the HTTP2+TLS case, the logic https://github.com/golang/go/blob/master/src/net/http/server.go#L1839-L1849 that would go here bypasses the cleanup.
Just because HTTP2 and TLS are used, you will get different behaviors. Need call multipartForm.removeall by yourself.

@seankhliao
Copy link
Member

cc @bradfitz @tombergan

@seankhliao seankhliao reopened this Sep 17, 2021
@seankhliao seankhliao added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 17, 2021
@seankhliao
Copy link
Member

https://go.dev/cl/423055 was merged

@golang golang locked and limited conversation to collaborators Aug 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants