go-sdk icon indicating copy to clipboard operation
go-sdk copied to clipboard

v3: `r2.OptTLSRootCAs()` has "typed nil" issue when checking for a nil transport

Open dhermes opened this issue 5 years ago • 1 comments

When checking

		if r.Client.Transport == nil {
			r.Client.Transport = &http.Transport{}
		}

the check will fail if the transport is a typed nil. For example:

package main

import (
	"crypto/x509"
	"fmt"
	"net/http"

	"github.com/blend/go-sdk/r2"
)

func main() {
	var transport *http.Transport
	certPool, err := x509.SystemCertPool()
	if err != nil {
		panic(err)
	}

	req := r2.New(
		"https://web.invalid",
		// NOTE: Transport **must** come before the root CAs since the CAs get set
		//       **on** the transport.
		r2.OptTransport(transport),
		r2.OptTLSRootCAs(certPool),
	)
	fmt.Println(req)
}

results in

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

goroutine 1 [running]:
github.com/blend/go-sdk/r2.OptTLSRootCAs.func1(0xc0000dfb80, 0x0, 0x0)
        .../go/src/github.com/blend/go-sdk/r2/opt_tls_root_cas.go:26 +0x2b7
github.com/blend/go-sdk/r2.New(0x0, 0x0, 0xc000175f30, 0x4, 0x4, 0x1d)
        .../go/src/github.com/blend/go-sdk/r2/request.go:30 +0x1a2
main.main()
        .../main.go:26 +0x12a
exit status 2

dhermes avatar Nov 26 '19 19:11 dhermes

Is reflect.ValueOf(r.Client.Transport).IsNil() the best option we have? Or do the callers just need to be aware of the limitation and write code like

	opts := []r2.Option{}
	// NOTE: Transport **must** come before the root CAs since the CAs get set
	//       **on** the transport.
	if transport != nil {
		opts = append(opts, r2.OptTransport(transport))
	}
	opts = append(opts, r2.OptTLSRootCAs(certPool))
	req := r2.New("https://web.invalid", opts...)

dhermes avatar Nov 26 '19 19:11 dhermes