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

nil pointer dereference in getConn(), span is nil #47

Open
fho opened this issue Aug 19, 2019 · 2 comments
Open

nil pointer dereference in getConn(), span is nil #47

fho opened this issue Aug 19, 2019 · 2 comments

Comments

@fho
Copy link

fho commented Aug 19, 2019

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 fho changed the title nil pointer dereference, span is nil nil pointer dereference in getConn(), span is nil Aug 19, 2019
@fho
Copy link
Author

fho commented Aug 19, 2019

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

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

@fho fho closed this as completed Aug 19, 2019
@fho
Copy link
Author

fho commented Aug 19, 2019

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.

@fho fho reopened this Aug 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant