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

io/ioutil: disallow nil readers in NopCloser #33409

Open
palsivertsen opened this issue Aug 1, 2019 · 2 comments

Comments

@palsivertsen
Copy link

commented Aug 1, 2019

Not sure if this is worth fixing, but thought I should create an issue in case anyone else experience the same issue.

What did you do?

If you create a new http.Request, feed it a ioutil.NopCloser with a nil reader and try to send the request using a http.Client, you'll get a panic with a stack trace from another thread.

package main

import (
	"io"
	"io/ioutil"
	"log"
	"net/http"
)

func main() {
	post(nil)
	log.Print("post nil ok")
	post(ioutil.NopCloser(nil))
	log.Print("post nil nop closer ok")
}

func post(body io.Reader) {
	req, err := http.NewRequest(http.MethodPost, "https://example.com", body)
	if err != nil {
		log.Fatal(err)
	}

	c := http.Client{}
	_, derr := c.Do(req)
	if derr != nil {
		log.Fatal(derr)
	}
}
$ go run .
2019/08/01 16:25:19 post nil ok
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x4b142e]

goroutine 14 [running]:
io/ioutil.(*nopCloser).Read(0xc00028b470, 0xc00050a000, 0x4000, 0x4000, 0x7fb4ddb98678, 0xc00028b470, 0x7fb4ddb98678)
	<autogenerated>:1 +0x2e
net/http.(*http2clientStream).writeRequestBody(0xc0002f5900, 0x7fb4ddb98658, 0xc00028b470, 0x7fb4ddb98678, 0xc00028b470, 0x0, 0x0)
	/usr/local/go/src/net/http/h2_bundle.go:7664 +0x521
net/http.(*http2Transport).getBodyWriterState.func1()
	/usr/local/go/src/net/http/h2_bundle.go:8878 +0xc2
created by net/http.http2bodyWriterState.scheduleBodyWrite
	/usr/local/go/src/net/http/h2_bundle.go:8925 +0xa3
exit status 2

What did you expect to see?

ioutil.NopCloser(nil) should return nil
OR
c.Do(req) should panic with stack trace leading back to this call
OR
ioutil.NopCloser(nil) should panic with stack trace leading back to this call

What did you see instead?

Panic from an internal go routine created the http package.

Does this issue reproduce with the latest release (go1.12.7)?

Yes. (tested 1.12.6 and 1.12.7)

System details

go version go1.12.6 linux/amd64
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/pal/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/pal/projects/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
GOROOT/bin/go version: go version go1.12.6 linux/amd64
GOROOT/bin/go tool compile -V: compile version go1.12.6
uname -sr: Linux 4.15.0-54-generic
Distributor ID:	Ubuntu
Description:	Ubuntu 16.04.6 LTS
Release:	16.04
Codename:	xenial
/lib/x86_64-linux-gnu/libc.so.6: GNU C Library (Ubuntu GLIBC 2.23-0ubuntu11) stable release version 2.23, by Roland McGrath et al.

@ianlancetaylor ianlancetaylor changed the title Improvement to ioutil: Disallow nil readers in NopCloser io/ioutil: disallow nil readers in NopCloser Aug 1, 2019

@ianlancetaylor ianlancetaylor added this to the Go1.14 milestone Aug 1, 2019

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Aug 1, 2019

Seems like it would be more or less OK for ioutil.NopCloser(nil) to panic. It's hard to see how that would be meaningful. Although it's also not clear to me that this is worth fixing.

@palsivertsen

This comment has been minimized.

Copy link
Author

commented Aug 6, 2019

It's hard to see how that would be meaningful.

In this case a panic from ioutil.NopCloser(nil) would produce a stack trace back to the third line in the main function (post(ioutil.NopCloser(nil))) instead of:

io/ioutil.(*nopCloser).Read(0xc00028b470, 0xc00050a000, 0x4000, 0x4000, 0x7fb4ddb98678, 0xc00028b470, 0x7fb4ddb98678)
	<autogenerated>:1 +0x2e
net/http.(*http2clientStream).writeRequestBody(0xc0002f5900, 0x7fb4ddb98658, 0xc00028b470, 0x7fb4ddb98678, 0xc00028b470, 0x0, 0x0)
	/usr/local/go/src/net/http/h2_bundle.go:7664 +0x521
net/http.(*http2Transport).getBodyWriterState.func1()
	/usr/local/go/src/net/http/h2_bundle.go:8878 +0xc2
created by net/http.http2bodyWriterState.scheduleBodyWrite
	/usr/local/go/src/net/http/h2_bundle.go:8925 +0xa3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.