Pull request #430: AGDNS-3952-udp-id

ainar-g · ainar-g · commit f00d992ce956 · 2026-05-04T14:47:19.000Z
Merge in GO/dnsproxy from AGDNS-3952-udp-id to master Squashed commit of the following: commit e74206b Author: Ainar Garipov <a.garipov@adguard.com> Date: Tue Apr 28 19:43:03 2026 +0300 upstream: imp code commit c281d1a Author: Ainar Garipov <a.garipov@adguard.com> Date: Tue Apr 28 17:27:08 2026 +0300 all: imp code commit 629588a Author: Ainar Garipov <a.garipov@adguard.com> Date: Tue Apr 28 14:17:51 2026 +0300 all: set id in udp when unset; go fix
diff --git a/Makefile b/Makefile
@@ -24,7 +24,7 @@ GOAMD64 = v1
 GOPROXY = https://proxy.golang.org|direct
 GOTELEMETRY = off
 OUT = dnsproxy
-GOTOOLCHAIN = go1.26.1
+GOTOOLCHAIN = go1.26.2
 RACE = 0
 REVISION = $${REVISION:-$$(git rev-parse --short HEAD)}
 VERSION = 0
diff --git a/bamboo-specs/bamboo.yaml b/bamboo-specs/bamboo.yaml
@@ -13,7 +13,7 @@
     # exact patch version as opposed to a minor one to make sure that this exact
     # version is actually used and not whatever the docker daemon on the CI has
     # cached a few months ago.
-    'dockerGo': 'adguard/go-builder:1.26.1--1'
+    'dockerGo': 'adguard/go-builder:1.26.2--1'
     'maintainer': 'Adguard Go Team'
     'name': 'dnsproxy'
 
diff --git a/docker/ci.Dockerfile b/docker/ci.Dockerfile
@@ -28,7 +28,7 @@
 #    needed.  Keep it in sync with bamboo-specs/bamboo.yaml.
 
 # NOTE:  Keep in sync with bamboo-specs/bamboo.yaml.
-ARG BASE_IMAGE=adguard/go-builder:1.26.1--1
+ARG BASE_IMAGE=adguard/go-builder:1.26.2--1
 
 # The dependencies stage is needed to install packages and tool dependencies.
 # This is also where binaries like osslsigncode, which may be required for tests
diff --git a/go.mod b/go.mod
@@ -1,6 +1,6 @@
 module github.com/AdguardTeam/dnsproxy
 
-go 1.26.1
+go 1.26.2
 
 require (
 	github.com/AdguardTeam/golibs v0.35.10
diff --git a/internal/cmd/proxy.go b/internal/cmd/proxy.go
@@ -526,8 +526,8 @@ func loadServersList(sources []string) []string {
 			servers = append(servers, source)
 		}
 
-		lines := strings.Split(string(data), "\n")
-		for _, line := range lines {
+		// TODO(a.garipov):  Be smarter about bytes vs. strings.
+		for line := range strings.SplitSeq(string(data), "\n") {
 			line = strings.TrimSpace(line)
 
 			// Ignore comments in the file.
diff --git a/proxy/errors.go b/proxy/errors.go
@@ -1,5 +1,4 @@
 //go:build !plan9
-// +build !plan9
 
 package proxy
 
diff --git a/proxy/errors_internal_test.go b/proxy/errors_internal_test.go
@@ -1,5 +1,4 @@
 //go:build !plan9
-// +build !plan9
 
 package proxy
 
diff --git a/proxy/errors_plan9.go b/proxy/errors_plan9.go
@@ -1,5 +1,4 @@
 //go:build plan9
-// +build plan9
 
 package proxy
 
diff --git a/proxy/proxy_internal_test.go b/proxy/proxy_internal_test.go
@@ -370,10 +370,7 @@ func newTxts(tb testing.TB, txtDataLen int) (txts []string) {
 
 	// *dns.TXT requires splitting the actual data into 256-byte chunks.
 	for i := range txtDataChunkNum {
-		r := txtDataChunkLen * (i + 1)
-		if r > txtDataLen {
-			r = txtDataLen
-		}
+		r := min(txtDataChunkLen*(i+1), txtDataLen)
 		txts[i] = string(randData[txtDataChunkLen*i : r])
 	}
 
@@ -600,8 +597,8 @@ func TestExchangeWithReservedDomains(t *testing.T) {
 		UpstreamConfig: newTestUpstreamConfigWithBoot(
 			t,
 			testTimeout,
-			"[/adguard.com/]1.2.3.4",
-			"[/google.ru/]2.3.4.5",
+			"[/adguard.com/]192.0.2.1",
+			"[/google.ru/]192.0.2.2",
 			"[/maps.google.ru/]#",
 			"1.1.1.1",
 		),
diff --git a/proxy/serverquic_internal_test.go b/proxy/serverquic_internal_test.go
@@ -289,10 +289,7 @@ func writeQUICStream(buf []byte, stream *quic.Stream) (err error) {
 	chunkSize := 400
 	for i := 0; i < len(buf); i += chunkSize {
 		chunkStart := i
-		chunkEnd := i + chunkSize
-		if chunkEnd > len(buf) {
-			chunkEnd = len(buf)
-		}
+		chunkEnd := min(i+chunkSize, len(buf))
 
 		_, err = stream.Write(buf[chunkStart:chunkEnd])
 		if err != nil {
diff --git a/upstream/doh.go b/upstream/doh.go
@@ -11,6 +11,7 @@ import (
 	"net/http"
 	"net/url"
 	"runtime"
+	"slices"
 	"sync"
 	"time"
 
@@ -695,13 +696,7 @@ func (p *dnsOverHTTPS) probeTLS(dialContext bootstrap.DialHandler, tlsConfig *tl
 
 // supportsH3 returns true if HTTP/3 is supported by this upstream.
 func (p *dnsOverHTTPS) supportsH3() (ok bool) {
-	for _, v := range p.tlsConf.NextProtos {
-		if v == string(HTTPVersion3) {
-			return true
-		}
-	}
-
-	return false
+	return slices.Contains(p.tlsConf.NextProtos, string(HTTPVersion3))
 }
 
 // supportsHTTP returns true if HTTP/1.1 or HTTP2 is supported by this upstream.
diff --git a/upstream/doq.go b/upstream/doq.go
@@ -285,7 +285,7 @@ func (p *dnsOverQUIC) getBytesPool() (pool *sync.Pool) {
 
 	if p.bytesPool == nil {
 		p.bytesPool = &sync.Pool{
-			New: func() interface{} {
+			New: func() (res any) {
 				b := make([]byte, dns.MaxMsgSize)
 
 				return &b
diff --git a/upstream/dot_internal_test.go b/upstream/dot_internal_test.go
@@ -66,17 +66,14 @@ func TestUpstream_dnsOverTLS_race(t *testing.T) {
 	// Use this upstream from multiple goroutines in parallel.
 	wg := sync.WaitGroup{}
 	for range count {
-		wg.Add(1)
-		go func() {
-			defer wg.Done()
-
+		wg.Go(func() {
 			pt := testutil.PanicT{}
 
 			req := createTestMessage()
 			resp, uErr := u.Exchange(req)
 			require.NoError(pt, uErr)
 			requireResponse(pt, req, resp)
-		}()
+		})
 	}
 
 	wg.Wait()
diff --git a/upstream/plain.go b/upstream/plain.go
@@ -95,11 +95,14 @@ func (p *plainDNS) dialExchange(
 	client := &dns.Client{Timeout: p.timeout}
 
 	conn := &dns.Conn{}
-	if network == networkUDP {
-		conn.UDPSize = dns.MinMsgSize
-	}
+	upstreamReq := setRequestForNetwork(req, conn, network)
+	defer func() {
+		if resp != nil {
+			resp.Id = req.Id
+		}
+	}()
 
-	logBegin(p.logger, addr, network, req)
+	logBegin(p.logger, addr, network, upstreamReq)
 	defer func() { logFinish(p.logger, addr, network, err) }()
 
 	ctx := context.Background()
@@ -109,22 +112,43 @@ func (p *plainDNS) dialExchange(
 	}
 	defer func(c net.Conn) { err = errors.WithDeferred(err, c.Close()) }(conn.Conn)
 
-	resp, _, err = client.ExchangeWithConn(req, conn)
+	resp, _, err = client.ExchangeWithConn(upstreamReq, conn)
 	if isExpectedConnErr(err) {
 		conn.Conn, err = dial(ctx, network, "")
 		if err != nil {
 			return nil, fmt.Errorf("dialing %s over %s again: %w", p.addr.Host, network, err)
 		}
 		defer func(c net.Conn) { err = errors.WithDeferred(err, c.Close()) }(conn.Conn)
 
-		resp, _, err = client.ExchangeWithConn(req, conn)
+		resp, _, err = client.ExchangeWithConn(upstreamReq, conn)
 	}
 
 	if err != nil {
 		return resp, fmt.Errorf("exchanging with %s over %s: %w", addr, network, err)
 	}
 
-	return resp, validatePlainResponse(req, resp)
+	return resp, validatePlainResponse(upstreamReq, resp)
+}
+
+// setRequestForNetwork sets connection options in conn and overrides the
+// upstream request, if necessary, depending on network.  If network is
+// [networkUDP] and orig has a zero ID, req is a copy of orig with a new ID to
+// increase entropy.  network must be either [networkUDP] or [networkTCP].  orig
+// and conn must not be nil.
+func setRequestForNetwork(orig *dns.Msg, conn *dns.Conn, network network) (req *dns.Msg) {
+	req = orig
+	if network != networkUDP {
+		return req
+	}
+
+	conn.UDPSize = dns.MinMsgSize
+
+	if orig.Id == 0 {
+		req = orig.Copy()
+		req.Id = dns.Id()
+	}
+
+	return req
 }
 
 // isExpectedConnErr returns true if the error is expected.  In this case,
diff --git a/upstream/upstream.go b/upstream/upstream.go
@@ -337,7 +337,15 @@ func logBegin(l *slog.Logger, addr string, n network, req *dns.Msg) {
 		qname = req.Question[0].Name
 	}
 
-	l.Debug("sending request", "addr", addr, "proto", n, "qtype", qtype, "qname", qname)
+	l.DebugContext(
+		context.TODO(),
+		"sending request",
+		"addr", addr,
+		"proto", n,
+		"qtype", qtype,
+		"qname", qname,
+		"id", req.Id,
+	)
 }
 
 // logFinish logs the end of DNS request resolution.  It should be called right

Original file line number	Diff line number	Diff line change
`@@ -526,8 +526,8 @@ func loadServersList(sources []string) []string {`
`526`	`526`	`servers = append(servers, source)`
`527`	`527`	`}`
`528`	`528`
`529`		`- lines := strings.Split(string(data), "\n")`
`530`		`- for _, line := range lines {`
	`529`	`+ // TODO(a.garipov): Be smarter about bytes vs. strings.`
	`530`	`+ for line := range strings.SplitSeq(string(data), "\n") {`
`531`	`531`	`line = strings.TrimSpace(line)`
`532`	`532`
`533`	`533`	`// Ignore comments in the file.`
Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,4 @@`
`1`	`1`	`//go:build !plan9`
`2`		`-// +build !plan9`
`3`	`2`
`4`	`3`	`package proxy`
`5`	`4`
Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,4 @@`
`1`	`1`	`//go:build plan9`
`2`		`-// +build plan9`
`3`	`2`
`4`	`3`	`package proxy`
`5`	`4`