Close #60 - go vet warning #61
Conversation
Do not copy across http transport object
Update to use real default client instead
|
Squashed those commits during merge |
| @@ -95,8 +95,8 @@ func open(name string, cli *http.Client) (io.ReadCloser, error) { | |||
|
|
|||
| // httpClient returns http client with default options | |||
| func httpClient(insecure bool) *http.Client { | |||
| transport := *(http.DefaultTransport.(*http.Transport)) | |||
| client := &http.Client{} | |||
| transport := client.Transport.(*http.Transport) | |||
| transport.TLSClientConfig = &tls.Config{InsecureSkipVerify: insecure} | |||
There was a problem hiding this comment.
Hi. I think this will not work. transport in client is nil by default. It will be initialized with http.DefaultTransport only on first request.
Here is an output: panic: interface conversion: http.RoundTripper is nil, not *http.Transport.
May be this variant https://stackoverflow.com/a/46011355/519917 may be used, but i not sure all Go versions would work with it.
There was a problem hiding this comment.
Any reason we want to have InsecureSkipVerify? If not, maybe we can just have return &http.Client{}
There was a problem hiding this comment.
The reason is to go through invalid certificates. There are many of them around in proprietary systems.
There was a problem hiding this comment.
^^ that. We have to support InsecureSkipVerify, @adriantam.
There was a problem hiding this comment.
The old line does the right job. You'd have to set client.Transport = transport then.
|
Fixed in e1518c1 |
Do not copy across http transport object as the internal implementation of DefaultTransport contains mutex.
Instead, initialise the return transport with the identical DefautlTransport value.