upstream: fix docs, revert opts order, imp code

README.md

@@ -123,11 +123,6 @@ The plain DNS upstream server may be specified in several ways:
     ./dnsproxy -l 127.0.0.1 -u udp://dns.google -u udp://1.1.1.1
     ```
 
- -  With a hostname or plain IP address and the `dns://` scheme (Deprecated):
-    ```shell
-    ./dnsproxy -l 127.0.0.1 -u dns://dns.google -u dns://1.1.1.1
-    ```
-
  -  With a hostname or plain IP address and the `tcp://` scheme to force using
     TCP:
     ```shell

upstream/bootstrap.go

@@ -80,12 +80,13 @@ func newBootstrapperResolved(upsURL *url.URL, options *Options) (*bootstrapper,
 // newBootstrapper initializes a new bootstrapper instance
 // address -- original resolver address string (i.e. tls://one.one.one.one:853)
 // options -- Upstream customization options
-func newBootstrapper(u *url.URL, options *Options) (*bootstrapper, error) {
+func newBootstrapper(u *url.URL, options *Options) (b *bootstrapper, err error) {
 	resolvers := []*Resolver{}
 	if len(options.Bootstrap) != 0 {
 		// Create a list of resolvers for parallel lookup
 		for _, boot := range options.Bootstrap {
-			r, err := NewResolver(boot, options)
+			var r *Resolver
+			r, err = NewResolver(boot, options)
 			if err != nil {
 				return nil, err
 			}

upstream/upstream.go

@@ -25,34 +25,54 @@ type Upstream interface {
 
 // Options for AddressToUpstream func
 type Options struct {
-	// VerifyServerCertificate used to be set to crypto/tls
-	// Config.VerifyPeerCertificate for DNS-over-HTTPS, DNS-over-QUIC,
-	// DNS-over-TLS.
-	VerifyServerCertificate func(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error
-
-	// VerifyDNSCryptCertificate is the callback the DNSCrypt server certificate
-	// will be passed to.  It's called in dnsCrypt.exchangeDNSCrypt.
-	// Upstream.Exchange method returns any error caused by it.
-	VerifyDNSCryptCertificate func(cert *dnscrypt.Cert) error
-
 	// Bootstrap is a list of DNS servers to be used to resolve
 	// DNS-over-HTTPS/DNS-over-TLS hostnames.  Plain DNS, DNSCrypt, or
 	// DNS-over-HTTPS/DNS-over-TLS with IP addresses (not hostnames) could be
 	// used.
 	Bootstrap []string
 
-	// List of IP addresses of the upstream DNS server.  If not empty, bootstrap
-	// DNS servers won't be used at all.
-	ServerIPAddrs []net.IP
-
 	// Timeout is the default upstream timeout.  It's also used as a timeout for
 	// bootstrap DNS requests.  Zero value disables the timeout.
 	Timeout time.Duration
 
+	// List of IP addresses of the upstream DNS server.  If not empty, bootstrap
+	// DNS servers won't be used at all.
+	ServerIPAddrs []net.IP
+
 	// InsecureSkipVerify disables verifying the server's certificate.
 	InsecureSkipVerify bool
+
+	// VerifyServerCertificate used to be set to crypto/tls
+	// Config.VerifyPeerCertificate for DNS-over-HTTPS, DNS-over-QUIC,
+	// DNS-over-TLS.
+	VerifyServerCertificate func(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error
+
+	// VerifyDNSCryptCertificate is the callback the DNSCrypt server certificate
+	// will be passed to.  It's called in dnsCrypt.exchangeDNSCrypt.
+	// Upstream.Exchange method returns any error caused by it.
+	VerifyDNSCryptCertificate func(cert *dnscrypt.Cert) error
 }
 
+const (
+	// defaultPortPlain is the default port for plain DNS.
+	defaultPortPlain = 53
+
+	// defaultPortDoH is the default port for DNS-over-HTTPS.
+	defaultPortDoH = 443
+
+	// defaultPortDoT is the default port for DNS-over-TLS.
+	defaultPortDoT = 853
+
+	// defaultPortDoQ is the default port for DNS-over-QUIC.
+	//
+	// Early experiments MAY use port 8853. This port is marked in the IANA
+	// registry as unassigned.  Note that prior to version -02 of this draft,
+	// experiments were directed to use port 784.
+	//
+	// See https://datatracker.ietf.org/doc/html/draft-ietf-dprive-dnsoquic-02#section-10.2.1.
+	defaultPortDoQ = 8853
+)
+
 // AddressToUpstream converts addr to an Upstream instance:
 //
 //  8.8.8.8:53 or udp://dns.adguard.com for plain DNS;
@@ -92,23 +112,27 @@ func AddressToUpstream(addr string, opts *Options) (u Upstream, err error) {
 	return &plainDNS{address: netutil.JoinHostPort(host, int(portN)), timeout: opts.Timeout}, nil
 }
 
-// urlToBoot creates an instance of the bootstrapper with the specified options
-// options -- Upstream customization options
-func urlToBoot(resolverURL *url.URL, opts *Options) (b *bootstrapper, err error) {
+// urlToBoot creates a bootstrapper with the specified options.
+func urlToBoot(u *url.URL, opts *Options) (b *bootstrapper, err error) {
 	if len(opts.ServerIPAddrs) == 0 {
-		return newBootstrapper(resolverURL, opts)
+		return newBootstrapper(u, opts)
 	}
 
-	return newBootstrapperResolved(resolverURL, opts)
+	return newBootstrapperResolved(u, opts)
 }
 
 // urlToUpstream converts uu to an Upstream using opts.
 func urlToUpstream(uu *url.URL, opts *Options) (u Upstream, err error) {
 	switch sch := uu.Scheme; sch {
 	case "sdns":
 		return stampToUpstream(uu, opts)
+	// TODO(e.burkov): Remove in the next major-minor release.
 	case "dns":
-		log.Info("warning: using %q scheme is deprecated", sch)
+		log.Info(
+			"warning: using %q scheme is deprecated and will be removed in future versions; "+
+				"use \"udp\" instead",
+			sch,
+		)
 
 		return newPlain(uu, opts.Timeout, false), nil
 	case "udp", "tcp":
@@ -166,10 +190,10 @@ func stampToUpstream(upsURL *url.URL, opts *Options) (Upstream, error) {
 	return nil, fmt.Errorf("unsupported protocol %v in %s", stamp.Proto, upsURL)
 }
 
-// addPort is a helper function that appends port if needed
-func addPort(u *url.URL, port string) {
-	if u.Port() == "" {
-		u.Host = net.JoinHostPort(u.Host, port)
+// addPort appends port to u if needed.
+func addPort(u *url.URL, port int) {
+	if u != nil && u.Port() == "" {
+		u.Host = netutil.JoinHostPort(u.Host, port)
 	}
 }
 

upstream/upstream_doh.go

@@ -50,7 +50,7 @@ var _ Upstream = &dnsOverHTTPS{}
 
 // newDoH returns the DNS-over-HTTPS Upstream.
 func newDoH(uu *url.URL, opts *Options) (u Upstream, err error) {
-	addPort(uu, "443")
+	addPort(uu, defaultPortDoH)
 
 	var b *bootstrapper
 	b, err = urlToBoot(uu, opts)

upstream/upstream_dot.go

@@ -25,7 +25,7 @@ var _ Upstream = &dnsOverTLS{}
 
 // newDoT returns the DNS-over-TLS Upstream.
 func newDoT(uu *url.URL, opts *Options) (u Upstream, err error) {
-	addPort(uu, "853")
+	addPort(uu, defaultPortDoT)
 
 	var b *bootstrapper
 	b, err = urlToBoot(uu, opts)

upstream/upstream_plain.go

@@ -22,7 +22,7 @@ var _ Upstream = &plainDNS{}
 
 // newPlain returns the plain DNS Upstream.
 func newPlain(uu *url.URL, timeout time.Duration, preferTCP bool) (u *plainDNS) {
-	addPort(uu, "53")
+	addPort(uu, defaultPortPlain)
 
 	return &plainDNS{
 		address:   uu.Host,

upstream/upstream_quic.go

@@ -31,12 +31,7 @@ var _ Upstream = &dnsOverQUIC{}
 
 // newDoQ returns the DNS-over-QUIC Upstream.
 func newDoQ(uu *url.URL, opts *Options) (u Upstream, err error) {
-	// Early experiments MAY use port 8853. This port is marked in the IANA
-	// registry as unassigned.  (Note that prior to version -02 of this
-	// draft, experiments were directed to use port 784.)
-	//
-	// See https://datatracker.ietf.org/doc/html/draft-ietf-dprive-dnsoquic-02#section-10.2.1.
-	addPort(uu, "8853")
+	addPort(uu, defaultPortDoQ)
 
 	var b *bootstrapper
 	b, err = urlToBoot(uu, opts)