net/http: NewRequest panics when given a nil instance of *strings.Reader

[xaanit]

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

$ go version
go version go1.13.7 windows/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
set GO111MODULE=
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\Jacob\AppData\Local\go-build
set GOENV=C:\Users\Jacob\AppData\Roaming\go\env
set GOEXE=.exe
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users\Jacob\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=D:\Go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLDIR=D:\Go\pkg\tool\windows_amd64
set GCCGO=gccgo
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=C:\Users\Jacob\AppData\Local\Temp\go-build483288042=/tmp/go-build -gno-record-gcc-switches

What did you do?

Playground: https://play.golang.org/p/5DED9DdFmld
Code in-case playground doesn't work:

package main

import (
	"fmt"
	"net/http"
	"os"
	"strings"
)

func succeeds() {
	_, err := http.NewRequest(http.MethodGet, "", nil)
	if err != nil {
		fmt.Printf("Err from succeeds: %+v\n", err)
		os.Exit(0)
	}
}

func fails() {
	var reader *strings.Reader = nil
	_, err := http.NewRequest(http.MethodGet, "", reader)
	if err != nil {
		fmt.Printf("Err from fails: %+v\n", err)
		os.Exit(0)
	}
}

func main() {
	defer func() {
		if x := recover(); x != nil {
			if err, ok := x.(error); ok {
				fmt.Printf("Err from defer: %+v\n", err)
				panic(err)
			} else {
				panic(x)
			}
		}
	}()
	succeeds()
	fmt.Println("after suceeds")
	fails()
	fmt.Println("after fails")
}

What did you expect to see?

It to work, as GET requests wouldn't have bodies.

What did you see instead?

A panic as it tries to, assumedly, execute this line:

		case *strings.Reader:
			req.ContentLength = int64(v.Len())

[mdlayher]

I don't think there's anything to be done here. Passing a nil *strings.Reader is a clear programming error as noted by the nil pointer dereference panic, regardless of the HTTP method.

[xaanit]

Then why does passing nil directly not throw the same panic? If you shouldn't be passing nil at all that is fine, but the behaviour is inconsistent.

[seankhliao]

https://golang.org/doc/faq#nil_error

[ianlancetaylor]

Closing because this does not seem to be a bug. Please comment if you disagree.