opentracing-contrib/go-stdlib

nil pointer dereference in getConn(), span is nil

fho opened this issue · 2 comments

fho commented

I tried to introduce http request tracing in one of our applications and stumpled about a nil pointer dereference that was triggered by a testcase.

The nil pointer dereference can be reproduced with the following program that follows the go-stdlib Godoc documentation:

package main

import (
	"context"
	"net/http"

	"github.com/opentracing-contrib/go-stdlib/nethttp"
	"github.com/opentracing/opentracing-go"
)

func main() {
	ctx := context.Background()

	req, err := http.NewRequest("POST", "http://localhost:8080/", nil)
	if err != nil {
		panic(err.Error())
	}

	req = req.WithContext(ctx)
	req, ht := nethttp.TraceRequest(opentracing.GlobalTracer(), req)
	defer ht.Finish()

	_, err = http.DefaultClient.Do(req)
	if err != nil {
		panic(err.Error())
	}
}

I expect that either http.DefaultClient.Do() succeeds or returns an error (because e.g. nothing is listening on localhost:8080).

But it triggers the following panic in go-stdlib:

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

goroutine 1 [running]:
sisu.sh/go/vendor/github.com/opentracing/opentracing-go/ext.stringTagName.Set(...)
	/home/fho/git/workspace/src/sisu.sh/go/vendor/github.com/opentracing/opentracing-go/ext/tags.go:170
sisu.sh/go/vendor/github.com/opentracing-contrib/go-stdlib/nethttp.(*Tracer).getConn(0xc0000b4380, 0xc0000ae292, 0xe)
	/home/fho/git/workspace/src/sisu.sh/go/vendor/github.com/opentracing-contrib/go-stdlib/nethttp/client.go:257 +0x106
net/http.(*Transport).getConn(0x9c2e20, 0xc00009cdb0, 0x0, 0x744464, 0x4, 0xc0000ae292, 0xe, 0x0, 0x0, 0x0, ...)
	/usr/lib/go/src/net/http/transport.go:955 +0xf4
net/http.(*Transport).roundTrip(0x9c2e20, 0xc00010e200, 0x59bfca, 0x74446b, 0xc0000dbb78)
	/usr/lib/go/src/net/http/transport.go:467 +0x6ef
net/http.(*Transport).RoundTrip(0x9c2e20, 0xc00010e200, 0x9c2e20, 0x0, 0x0)
	/usr/lib/go/src/net/http/roundtrip.go:17 +0x35
net/http.send(0xc00010e200, 0x7a6c60, 0x9c2e20, 0x0, 0x0, 0x0, 0xc0000b0038, 0x8, 0x1, 0x0)
	/usr/lib/go/src/net/http/client.go:250 +0x461
net/http.(*Client).send(0x9c7fc0, 0xc00010e200, 0x0, 0x0, 0x0, 0xc0000b0038, 0x0, 0x1, 0x30)
	/usr/lib/go/src/net/http/client.go:174 +0xfb
net/http.(*Client).do(0x9c7fc0, 0xc00010e200, 0x0, 0x0, 0x0)
	/usr/lib/go/src/net/http/client.go:641 +0x279
net/http.(*Client).Do(...)
	/usr/lib/go/src/net/http/client.go:509
main.main()
	/home/fho/git/workspace/src/sisu.sh/go/code/testtracing.go:23 +0x181
exit status 2

The panic happens because h.sp is nil when it's passed here: https://github.com/opentracing-contrib/go-stdlib/blob/master/nethttp/client.go#L255.

I didn't initialized the opentracing.GlobalTracer() in my program. So it uses the Nooptracer. I expect that the operations succeed nonetheless without a panic.

I'm using:

  • go version go1.12.8 linux/amd64
  • github.com/opentracing-contrib/go-stdlib cf7a6c9
  • github.com/opentracing/opentracing-go v1.1.0
fho commented

Nevermind, I did not set the http transport for the client: :-/

	http.Client{Transport: &nethttp.Transport{}}
fho commented

I think this is nonetheless an issue.

If the Transport of the http client must be set to prevent the nil pointer exception, it means that we can not inject e.g. a mock http clients in tests that don't have this transport setting.

  • 👍3