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

net/http: panic() on ioutil.ReadAll in caddy on go 692df21 #17525

Closed
wendigo opened this issue Oct 20, 2016 · 2 comments
Closed

net/http: panic() on ioutil.ReadAll in caddy on go 692df21 #17525

wendigo opened this issue Oct 20, 2016 · 2 comments

Comments

@wendigo
Copy link

wendigo commented Oct 20, 2016

Please answer these questions before submitting your issue. Thanks!

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

go version devel +692df21 Thu Oct 20 08:48:44 2016 +0000 darwin/amd64

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

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/mateusz.gajewski/Desktop/Projects/"
GORACE=""
GOROOT="/usr/local/Cellar/go/HEAD-692df21/libexec"
GOTOOLDIR="/usr/local/Cellar/go/HEAD-692df21/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/43/1y0rrqr55zq3f8cj5wpkt1kmhc7r33/T/go-build099060148=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"
PKG_CONFIG="pkg-config"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"

What did you do?

go get -u -d github.com/mholt/caddy
go test github.com/mholt/caddy/...

What did you expect to see?

All tests pass as in Go 1.7.3

What did you see instead?

panic: runtime error: invalid memory address or nil pointer dereference [recovered]
    panic: runtime error: invalid memory address or nil pointer dereference [recovered]
    panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x20 pc=0xbf4d4]

goroutine 23 [running]:
panic(0x4a18a0, 0x796300)
    /usr/local/Cellar/go/HEAD-692df21/libexec/src/runtime/panic.go:527 +0x1a0
testing.tRunner.func1(0xc42014e240)
    /usr/local/Cellar/go/HEAD-692df21/libexec/src/testing/testing.go:603 +0x287
panic(0x4a18a0, 0x796300)
    /usr/local/Cellar/go/HEAD-692df21/libexec/src/runtime/panic.go:485 +0x241
io/ioutil.readAll.func1(0xc420154ae8)
    /usr/local/Cellar/go/HEAD-692df21/libexec/src/io/ioutil/ioutil.go:30 +0x135
panic(0x4a18a0, 0x796300)
    /usr/local/Cellar/go/HEAD-692df21/libexec/src/runtime/panic.go:485 +0x241
bytes.(*Buffer).ReadFrom(0xc420154a40, 0x0, 0x0, 0xc4201c4000, 0x0, 0x200)
    /usr/local/Cellar/go/HEAD-692df21/libexec/src/bytes/buffer.go:179 +0x134
io/ioutil.readAll(0x0, 0x0, 0x200, 0x0, 0x0, 0x0, 0x0, 0x0)
    /usr/local/Cellar/go/HEAD-692df21/libexec/src/io/ioutil/ioutil.go:33 +0x150
io/ioutil.ReadAll(0x0, 0x0, 0x0, 0x0, 0x0, 0xa1eb, 0x55d50d4c)
    /usr/local/Cellar/go/HEAD-692df21/libexec/src/io/ioutil/ioutil.go:42 +0x3e
github.com/mholt/caddy/caddyhttp/log.TestLogRequestBody.func1(0x744800, 0xc4200745a0, 0xc4201580f0, 0x4ea760, 0xc420074501, 0xc42007dd10)
    /Users/mateusz.gajewski/Desktop/Projects/src/github.com/mholt/caddy/caddyhttp/log/log_test.go:79 +0x5c
github.com/mholt/caddy/caddyhttp/httpserver.HandlerFunc.ServeHTTP(0x544280, 0x744800, 0xc4200745a0, 0xc4201580f0, 0xc42007dce0, 0xc420154c58, 0x118f0)
    /Users/mateusz.gajewski/Desktop/Projects/src/github.com/mholt/caddy/caddyhttp/httpserver/middleware.go:72 +0x44
github.com/mholt/caddy/caddyhttp/log.Logger.ServeHTTP(0x740240, 0x544280, 0xc420088060, 0x1, 0x1, 0x0, 0x744b80, 0xc42007edc0, 0xc4201580f0, 0xc420088050, ...)
    /Users/mateusz.gajewski/Desktop/Projects/src/github.com/mholt/caddy/caddyhttp/log/log.go:40 +0x2eb
github.com/mholt/caddy/caddyhttp/log.TestLogRequestBody(0xc42014e240)
    /Users/mateusz.gajewski/Desktop/Projects/src/github.com/mholt/caddy/caddyhttp/log/log_test.go:112 +0x52f
testing.tRunner(0xc42014e240, 0x544288)
    /usr/local/Cellar/go/HEAD-692df21/libexec/src/testing/testing.go:637 +0x81
created by testing.(*T).Run
    /usr/local/Cellar/go/HEAD-692df21/libexec/src/testing/testing.go:674 +0x2c0

Panic happens when draining response.Body with ioutil.ReadAll() into bytes.Buffer https://github.com/mholt/caddy/blob/master/caddyhttp/log/log_test.go#L79

@bradfitz
Copy link
Contributor

This is a result of 4859f6a but it's actually looks like it just reveals a bug of sort in Caddy:

https://github.com/mholt/caddy/blob/master/caddyhttp/log/log_test.go#L107

http.NewRequest returns an outgoing client request, which that test passes to ServerHTTP (which takes incoming server requests). Those types of Requests have different semantics.

Caddy should probably use https://golang.org/pkg/net/http/httptest/#NewRequest instead, which makes incoming server requests instead of client requests.

/cc @mholt

@bradfitz bradfitz changed the title panic() on ioutil.ReadAll in caddy on go 692df21 net/http: panic() on ioutil.ReadAll in caddy on go 692df21 Oct 20, 2016
@mholt
Copy link

mholt commented Oct 20, 2016

Ah, nice find you guys!! We'll fix it over in Caddy. Thanks for the quick report/action. (You can close this issue.)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants