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

[wendigo]

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]

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

[mholt]

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