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

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

Open
dhermes opened this issue Nov 26, 2019 · 1 comment
Labels
v3.x.y Version 3 series

Comments

@dhermes
Copy link
Contributor

dhermes commented Nov 26, 2019

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
Copy link
Contributor Author

dhermes commented Nov 26, 2019

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 dhermes added the v3.x.y Version 3 series label Dec 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v3.x.y Version 3 series
Projects
None yet
Development

No branches or pull requests

1 participant