resty icon indicating copy to clipboard operation
resty copied to clipboard

SetTLSClientConfig doesn't work for custom http.RoundTripper implementation

Open dimitrimus opened this issue 5 years ago • 3 comments

I have a wrapper over *http.Transport. It implements http.RoundTripper interface and my wrapper implements it as well. Then I set this Transport to http.Client.Transport field which is http.RoundTripper too. And then I create resty.Client with my http.Client using constructor function NewWithClient.

Everything works well, but when I call SetTLSClientConfig function on resty client it checks whether underlying client Transport field is of type *http.Transport: https://github.com/go-resty/resty/blob/608c8d777d0edf2d69b0510a5c5ddf716177f187/client.go#L891-L896 The problem is that my custom RoundTripper is still *http.Transport, but this function doesn't work and it logs error.

I would like to have something like TLSConfigurable interface with two methods:

type TLSConfigurable interface {
	TLSConfig() *tls.Config
	SetTLSConfig(config *tls.Config)
}

and check whether c.httpClient.Transport implements it in SetTLSClientConfig:

func (c *Client) SetTLSClientConfig(config *tls.Config) *Client {
	if configurable, ok := c.httpClient.Transport.(TLSConfigurable); ok {
		configurable.SetTLSConfig(config)
		return c
	}
...

I'm ready to make PR with this interface and fixes.

dimitrimus avatar Sep 22 '20 10:09 dimitrimus

@dimitrimus Thanks for reaching out.

I see your suggestion of introducing a new interface to support custom http.RoundTripper. I would suggest keeping both implementations together with the order of evaluation in the PR. So, it works with backward compatibility for all resty users.

jeevatkm avatar Oct 07 '20 05:10 jeevatkm

@dimitrimus I thought to check with you. Are you still interested in making PR?

jeevatkm avatar Jan 11 '21 02:01 jeevatkm