net/http/httptest: wrong ContentLength for request with http.NoBody

[jfrancisco0]

Go version

go version go1.22.5 darwin/arm64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/jfrancisco/Library/Caches/go-build'
GOENV='/Users/jfrancisco/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/jfrancisco/go/pkg/mod'
GONOPROXY='github.com/<redacted>/*'
GONOSUMDB='github.com/<redacted>/*'
GOOS='darwin'
GOPATH='/Users/jfrancisco/go'
GOPRIVATE='github.com/<redacted>/*'
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/opt/homebrew/Cellar/go/1.22.5/libexec'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/opt/homebrew/Cellar/go/1.22.5/libexec/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.22.5'
GCCGO='gccgo'
AR='ar'
CC='cc'
CXX='c++'
CGO_ENABLED='1'
GOMOD='/Users/jfrancisco/golang/src/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/x9/7wxj63rn0f573d7zdjy3m63w0000gn/T/go-build2393148245=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

The gocritic linter suggests I should use http.NoBody instead of nil for empty body requests. However, httptest.NewRequest() shows different behaviour when http.NoBody is sent vs when nil is sent. Note that this is only the case for the httptest package, as http.NewRequest() - from the http package - works as expected.

See the example from https://go.dev/play/p/n9UxgUsAfM8

What did you see happen?

Creating a request with a http.NoBody body results in a ContentLength value of -1 instead of the expected 0 you would get with a nil body.

httptest.NewRequest("GET", "/", nil)
http.NewRequest("GET", "/", http.NoBody)

set ContentLength as 0, but

httptest.NewRequest("GET", "/", http.NoBody)

sets ContentLength as -1.

This makes my tests fail, as the server returns an error. According to the documentation, ContentLength: -1 means unknown, which should not be the case for http.NoBody, as that's and empty body (length 0). This happens because http.NoBody is a io.ReadCloser, which doesn't match any of the types in the switch.

What did you expect to see?

The request with a http.NoBody body should have ContentLength: 0.

The http package never sets a negative ContentLength for backwards compatibility (source).

I suggest that either the same logic is ported over to the httptest package, or that http.NoBody is accounted for as an exception, like it was in the http package before the reversal in this commit.

[gabyhelp]

Related Issues and Documentation

• net/http: client do not set Content-Length: 0 with an io.Reader with no bytes. #20257 (closed)
• net/http/httptest: NewRequest sets body to `{}` instead of nil #20965 (closed)
• net/http: NewRequest started setting ContentLength to -1 and breaking people? #18117 (closed)
• net/http: Content-Length: 0 is not set for PATCH requests with empty body #40978 (closed)
• net/http: Content-Length is not set in outgoing request when using ioutil.NopCloser #34295 (closed)
• net/http/httputil: DumpResponse return error when ContentLength=2 with Body length 0, but it's a response for http.MethodHead request #42478
• proposal: net/http/httptest: Recorder.Result() should always set Response.Body to http.NoBody when response is empty #39290 (closed)
• net/http: discrepancy between HeadContentLength of "" in http2 vs http1 #13532 (closed)
• net/http: accepts invalid empty Content-Length header #61679 (closed)
• proposal: net/http: automatically set Content-Length from Len method on Body #41182 (closed)

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[cherrymui]

As you already see, httptest.NewRequest is documented as

The provided body may be nil. If the body is of type bytes.Reader, strings.Reader, or *bytes.Buffer, the Request.ContentLength is set.

So it is clearly fine to pass a nil body. There is no need to change it.

Also, as http.NoBody is not any of those types, the ContentLength is "unset", which matches the documentation. Maybe we want to explicitly document it set to -1?

cc @neild

[gopherbot]

Change https://go.dev/cl/599815 mentions this issue: net/http/httptest: match net/http ContentLength behavior for http.NoBody