blackducksoftware / hub-client-go

Hub Client for Go (golang)
Apache License 2.0
9 stars 19 forks source link

client.HttpGetJSON panics with nil pointer dereference when Client.Timeout exceeded #27

Open johnmccabe opened 5 years ago

johnmccabe commented 5 years ago

Hi, I've been hitting problems with some API calls to Hub timing out resulting in a panic due to an error being returned along with *http.Response being nil.

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

goroutine 1 [running]:
github.com/blackducksoftware/hub-client-go/hubclient.readResponseBody(0x0, 0xc000208500, 0x0, 0x141d980)
        /Users/xxxxxxx/workspace/go/src/github.com/blackducksoftware/hub-client-go/hubclient/client.go:420 +0x37
github.com/blackducksoftware/hub-client-go/hubclient.(*Client).HttpGetJSON(0xc0000ae370, 0xc0002d61c0, 0x70, 0x132ce40, 0xc00000c460, 0xc8, 0x70, 0xc00006ac00)
        /Users/xxxxxxx/workspace/go/src/github.com/blackducksoftware/hub-client-go/hubclient/client.go:225 +0x3ed
github.com/blackducksoftware/hub-client-go/hubclient.(*Client).ListProjectVersions(0xc0000ae370, 0xc00001a410, 0x8, 0xc000020300, 0x5e, 0x0, 0x0, 0x0, 0x0, 0xc00000c3e0, ...)
        /Users/xxxxxxx/workspace/go/src/github.com/blackducksoftware/hub-client-go/hubclient/projects-client.go:98 +0x171
main.main()
        /Users/xxxxxxx/workspace/go/src/xxxxxxx/xxxxxxx/hub-query/main.go:68 +0xc7e
exit status 2

resp is nil here with the panic in readResponseBody occurring when an attempt to access resp.Body is made:

https://github.com/blackducksoftware/hub-client-go/blob/e27076af6370051227bbe0a09f2b3506e7126f32/hubclient/client.go#L174-L177

I've tweaked the hubclient locally (handled the nil in this single location and set a dial timeout) and was able to see that in my case the underlying error being thrown (resulting in resp == nil) was:

net/http: request canceled (Client.Timeout exceeded while awaiting headers)

The error handling should be tweaked to account for the possibility of resp being nil.

tandr commented 4 years ago

@johnmccabe Would you be so kind and try to reproduce it? We have modified readResponseBody to handle nil resp some time ago.