net/http: zombie connection leaked after Server.IdleTimeout

[lesismal]

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

$ go version
go version go1.17 linux/amd64

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="amd64"
GOBIN=""
GOCACHE="/home/ubuntu/.cache/go-build"
GOENV="/home/ubuntu/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/ubuntu/dev/gopath/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/ubuntu/dev/gopath"
GOPRIVATE=""
GOPROXY="direct"
GOROOT="/home/ubuntu/dev/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/ubuntu/dev/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.17"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
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 -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build2048112507=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Here are the code that can reproduce it:

// server.go
package main

import (
	"io"
	"net/http"
	_ "net/http/pprof"
	"time"
)

func main() {
	mux := &http.ServeMux{}
	mux.HandleFunc("/echo", func(w http.ResponseWriter, r *http.Request) {
		data, _ := io.ReadAll(r.Body)
		w.Write(data)
	})

	server := http.Server{
		Addr:        "localhost:8080",
		Handler:     mux,

		// Set IdleTimeout to 3s
		IdleTimeout: 3 * time.Second,
	}
	server.ListenAndServe()
}

// client.go
package main

import (
	"log"
	"net"
	"time"
)

func main() {
	go func() {
		for i := 0; true; i++ {
			// Keep printing alive until the conn is closed and the process exit
			log.Printf("alive %vs", i)
			time.Sleep(time.Second)
		}
	}()

	conn, err := net.Dial("tcp", "localhost:8080")
	if err != nil {
		panic(err)
	}

	// step 1: Send the first request
	reqData := []byte("POST /echo HTTP/1.1\r\nHost: localhost:8080\r\nContent-Length: 5\r\nAccept-Encoding: gzip\r\n\r\nhello")
	_, err = conn.Write(reqData)
	if err != nil {
		panic(err)
	}

	// step 2: Recv the first response
	resData := make([]byte, 1024)
	n, err := conn.Read(resData)
	if err != nil {
		panic(err)
	}
	log.Printf("resData: \n-----\n%v\n-----\nerror: %v", string(resData[:n]), err)

	// step 3: Send 4 bytes to server, no longer send the left data of a full request
	_, err = conn.Write(reqData[:4])
	if err != nil {
		log.Fatal(err)
	}

	// step 4: Read and wait the server close the conn.
	// The server will not close the conn and will block here
	_, err = conn.Read(resData)
	if err != nil {
		log.Fatal(err)
	}
}

# client output
2022/03/11 21:39:44 resData:
-----
HTTP/1.1 200 OK
Date: Fri, 11 Mar 2022 13:39:44 GMT
Content-Length: 5
Content-Type: text/plain; charset=utf-8

hello
-----
error: <nil>
2022/03/11 21:39:44 alive 1s
2022/03/11 21:39:45 alive 2s
2022/03/11 21:39:46 alive 3s
...
2022/03/11 21:41:44 alive 120s
...
...

What did you expect to see?

The server should close the client conn when the client didn't send all the request data after Server.IdleTimeout

What did you see instead?

The server didn't close the clien conn

[rhysh]

Setting ReadHeaderTimeout in your example code works for me. Does that solve it for you too?

My interpretation of the http.Server settings here is that once the server sees the first few bytes of the request, instead of being an "idle" connection subject to the "IdleTimeout" setting, the connection instead is considered active and subject to the "ReadHeaderTimeout" setting.

[lesismal]

Yes, ReadHeaderTimeout works, but not everyone sets it.
I found another thing with the same server code, but the client sends only part of the data of a request, will also leak:

// client.go
package main

import (
	"log"
	"net"
	"time"
)

func main() {
	go func() {
		for i := 1; true; i++ {
			// Keep printing alive until the conn is closed and the process exit
			time.Sleep(time.Second)
			log.Printf("alive %vs", i)
		}
	}()

	conn, err := net.Dial("tcp", "localhost:8080")
	if err != nil {
		panic(err)
	}

	reqData := []byte("POST /echo HTTP/1.1\r\nHost: localhost:8080\r\nContent-Length: 5\r\nAccept-Encoding: gzip\r\n\r\nhello")
	// step 1: Send part of the data of a request
	_, err = conn.Write(reqData[:4])
	if err != nil {
		panic(err)
	}

	// step 2: Recv the first response
	// The server will not close the conn and will block here
	resData := make([]byte, 1024)
	n, err := conn.Read(resData)
	if err != nil {
		panic(err)
	}
	log.Printf("resData: \n-----\n%v\n-----\nerror: %v", string(resData[:n]), err)
}

ReadHeaderTimeout also works for this.

For these two client examples, both need users to set Server.ReadHeaderTimeout, but many people don't set it.

Slow connection attack, the device of the client power off, wifi changed, entering an elevator may cause that the client will never send TCP.FIN to the server, then the conn will never be closed and the goroutine serving the conn will never exit.

It seems like the famous frameworks based on net/http such as gin, use the default configuration as their default configuration:
https://github.com/gin-gonic/gin/blob/master/gin.go#L378

Many people use net/http and gin's default configuration, then, many projects may have this issue.

As a default configuration, I think it should not leave this side effect.

[lesismal]

I think if ReadHeaderTimeout is zero, use someDefaultTimeout may be better.

[lesismal]

Maybe golang itself should leave it to users to set the right configuration, but some frameworks should handle their default configuration.
So I'll close this but open some issues to other frameworks.

Thanks for your time!