feat(resolvers/HTTPS): add HTTP proxy support #119

sam · 2022-09-27T21:03:57Z

sam commented

2022-09-27 21:03:57 +00:00

Turns out this was really easy and probably a nice-to-have

sam added 1 commit 2022-09-27 21:03:58 +00:00

feat(resolvers/HTTPS): add HTTP proxy support

continuous-integration/drone/push Build is passing

Details

continuous-integration/drone/pr Build is passing

Details

f22ca2f82f

grumbulon reviewed 2022-09-27 23:11:36 +00:00

pkg/resolvers/HTTPS.go Outdated

					
				@ -26,6 +27,17 @@ func (resolver *HTTPSResolver) LookUp(msg *dns.Msg) (util.Response, error) {

					httpR := &http.Client{

grumbulon commented

2022-09-27 23:11:27 +00:00

This is small and meant to be more of a talking point than a hard and fast thing that needs to be done.

I was thinking since HTTPSResolver is declared here do we need to declare a new struct we can keep everything inside of the HTTPSResolver object like so

// HTTPSResolver is for DNS-over-HTTPS queries.
type HTTPSResolver struct {
	client http.Client
	opts   util.Options
}

var _ Resolver = (*HTTPSResolver)(nil)

// LookUp performs a DNS query.
func (resolver *HTTPSResolver) LookUp(msg *dns.Msg) (util.Response, error) {
	var resp util.Response

	resolver.client = http.Client{
		Timeout: resolver.opts.Request.Timeout,
		Transport: &http.Transport{
			MaxConnsPerHost:     1,
			MaxIdleConns:        1,
			MaxIdleConnsPerHost: 1,
			Proxy:               http.ProxyFromEnvironment,
			TLSClientConfig: &tls.Config{
				//nolint:gosec // This is intentional if the user requests it
				InsecureSkipVerify: resolver.opts.TLSNoVerify,
				ServerName:         resolver.opts.TLSHost,
			},
		},
	}

and on line 59 we can keep it all under the resolver as res, err := resolver.client.Do(req)

Besides that lgtm, just curious what you think about this.

This is small and meant to be more of a talking point than a hard and fast thing that needs to be done. I was thinking since HTTPSResolver is declared here do we need to declare a new struct we can keep everything inside of the HTTPSResolver object like so ```Go // HTTPSResolver is for DNS-over-HTTPS queries. type HTTPSResolver struct { client http.Client opts util.Options } var _ Resolver = (*HTTPSResolver)(nil) // LookUp performs a DNS query. func (resolver *HTTPSResolver) LookUp(msg *dns.Msg) (util.Response, error) { var resp util.Response resolver.client = http.Client{ Timeout: resolver.opts.Request.Timeout, Transport: &http.Transport{ MaxConnsPerHost: 1, MaxIdleConns: 1, MaxIdleConnsPerHost: 1, Proxy: http.ProxyFromEnvironment, TLSClientConfig: &tls.Config{ //nolint:gosec // This is intentional if the user requests it InsecureSkipVerify: resolver.opts.TLSNoVerify, ServerName: resolver.opts.TLSHost, }, }, } ``` and on [line 59](https://git.froth.zone/sam/awl/src/commit/f22ca2f82f5d49a6f1ba80b7814dc9cca71ce0a1/pkg/resolvers/HTTPS.go#L59) we can keep it all under the resolver as ` res, err := resolver.client.Do(req)` Besides that lgtm, just curious what you think about this.

sam added 1 commit 2022-09-28 14:55:01 +00:00

fix: apply suggested change

continuous-integration/drone/push Build is passing

Details

continuous-integration/drone/pr Build is passing

Details

937d0826e3