net/http: NewRequest panics if body is typed nil

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

$ go version
go version go1.11.11 linux/amd64

Does this issue reproduce with the latest release?

Yep. Go Playground link: https://play.golang.org/p/UuBFg4Z31NY

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

go env Output

$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="$HOME/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="$HOME/go"
GOPROXY=""
GORACE=""
GOROOT="$HOME/sdk/go1.11.11"
GOTMPDIR=""
GOTOOLDIR="$HOME/sdk/go1.11.11/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
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-build796944957=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Passing a nil as a function argument and then passing that argument into http.NewRequest as the body parameter (3rd parameter), causes http.NewRequest to panic.

https://play.golang.org/p/UuBFg4Z31NY

What did you expect to see?

A new *http.Request with an empty body and no error are returned OR a nil pointer and a non-nil error are returned. There should not be a panic.

What did you see instead?

http.NewRequest panics due to a nil pointer dereference.

2 thoughts on “net/http: NewRequest panics if body is typed nil

  1. The primary point of the existing nil check is to discover whether the (optional) request body was provided. If the body argument wasn’t optional, there would be no use for that check.

    What’s the use case for passing a non-nil io.Reader containing a nil *bytes.Buffer to NewRequest? This most likely signals a bug in the calling code. The proposed changes to NewRequest would just cover it up. The code in question should panic.

    edit: don’t get too hung up on those special cases. For the sake of this discussion, they can be ignored entirely, I think. A nil *bytes.Buffer is going to panic at the first invocation of its Read method. It is thus not a useful thing to pass around as an io.Reader. NewRequest shouldn’t be in the business of trying to predict whether the provided interface value really implements the io.Reader contract. It should just call the Read method.

Comments are closed.