From 482215de6ee7f8ded124cb9c444052a4c4eafcf5 Mon Sep 17 00:00:00 2001 From: "Yoshiaki Ueda (bootjp)" Date: Wed, 4 Feb 2026 02:52:00 +0900 Subject: [PATCH 1/6] Fix DoQ response parsing and add partial read regression test --- upstream/doq.go | 36 +++++++++++--- upstream/doq_internal_test.go | 92 +++++++++++++++++++++++++++++++++++ 2 files changed, 122 insertions(+), 6 deletions(-) diff --git a/upstream/doq.go b/upstream/doq.go index f5ffbde0..a936dfa1 100644 --- a/upstream/doq.go +++ b/upstream/doq.go @@ -3,7 +3,9 @@ package upstream import ( "context" "crypto/tls" + "encoding/binary" "fmt" + "io" "log/slog" "net" "net/url" @@ -95,6 +97,12 @@ type dnsOverQUIC struct { timeout time.Duration } +// quicStream is the subset of quic.Stream used by readMsg. +type quicStream interface { + Read(p []byte) (n int, err error) + CancelRead(code quic.StreamErrorCode) +} + // newDoQ returns the DNS-over-QUIC Upstream. func newDoQ(addr *url.URL, opts *Options) (u Upstream, err error) { addPort(addr, defaultPortDoQ) @@ -397,15 +405,33 @@ func (p *dnsOverQUIC) closeConnWithError(conn *quic.Conn, err error) { } // readMsg reads the incoming DNS message from the QUIC stream. -func (p *dnsOverQUIC) readMsg(stream *quic.Stream) (m *dns.Msg, err error) { +func (p *dnsOverQUIC) readMsg(stream quicStream) (m *dns.Msg, err error) { pool := p.getBytesPool() bufPtr := pool.Get().(*[]byte) defer pool.Put(bufPtr) + var lenBuf [2]byte + _, err = io.ReadFull(stream, lenBuf[:]) + if err != nil { + return nil, fmt.Errorf("reading response length from %s: %w", p.addr, err) + } + + msgLen := int(binary.BigEndian.Uint16(lenBuf[:])) + if msgLen == 0 { + return nil, fmt.Errorf("empty response from %s", p.addr) + } else if msgLen > dns.MaxMsgSize { + return nil, fmt.Errorf("response size %d exceeds max %d from %s", msgLen, dns.MaxMsgSize, p.addr) + } + respBuf := *bufPtr - n, err := stream.Read(respBuf) - if err != nil && n == 0 { + if cap(respBuf) < msgLen { + respBuf = make([]byte, msgLen) + } + respBuf = respBuf[:msgLen] + + _, err = io.ReadFull(stream, respBuf) + if err != nil { return nil, fmt.Errorf("reading response from %s: %w", p.addr, err) } @@ -414,10 +440,8 @@ func (p *dnsOverQUIC) readMsg(stream *quic.Stream) (m *dns.Msg, err error) { // All DNS messages (queries and responses) sent over DoQ connections MUST // be encoded as a 2-octet length field followed by the message content as // specified in [RFC1035]. - // IMPORTANT: Note, that we ignore this prefix here as this implementation - // does not support receiving multiple messages over a single connection. m = new(dns.Msg) - err = m.Unpack(respBuf[2:]) + err = m.Unpack(respBuf) if err != nil { return nil, fmt.Errorf("unpacking response from %s: %w", p.addr, err) } diff --git a/upstream/doq_internal_test.go b/upstream/doq_internal_test.go index 6aaf6778..32d46b63 100644 --- a/upstream/doq_internal_test.go +++ b/upstream/doq_internal_test.go @@ -232,6 +232,98 @@ func TestUpstreamDoQ_0RTT(t *testing.T) { require.True(t, conns[1].is0RTT()) } +func TestUpstreamDoQ_readMsg_partialRead(t *testing.T) { + oldReq := createHostTestMessage("old.example") + oldResp := (&dns.Msg{}).SetReply(oldReq) + oldResp.Answer = []dns.RR{&dns.A{ + Hdr: dns.RR_Header{ + Name: dns.Fqdn("old.example"), + Rrtype: dns.TypeA, + Class: dns.ClassINET, + Ttl: 60, + }, + A: net.IPv4(1, 1, 1, 1), + }} + + oldPacked, err := oldResp.Pack() + require.NoError(t, err) + oldBuf := make([]byte, 2+len(oldPacked)) + binary.BigEndian.PutUint16(oldBuf, uint16(len(oldPacked))) + copy(oldBuf[2:], oldPacked) + + pool := &sync.Pool{ + New: func() any { + b := make([]byte, dns.MaxMsgSize) + copy(b, oldBuf) + return &b + }, + } + + newReq := createHostTestMessage("new.example") + newResp := (&dns.Msg{}).SetReply(newReq) + newResp.Answer = []dns.RR{&dns.A{ + Hdr: dns.RR_Header{ + Name: dns.Fqdn("new.example"), + Rrtype: dns.TypeA, + Class: dns.ClassINET, + Ttl: 60, + }, + A: net.IPv4(2, 2, 2, 2), + }} + + newPacked, err := newResp.Pack() + require.NoError(t, err) + newBuf := make([]byte, 2+len(newPacked)) + binary.BigEndian.PutUint16(newBuf, uint16(len(newPacked))) + copy(newBuf[2:], newPacked) + + stream := &testQUICStream{ + data: newBuf, + chunk: 1, + } + + p := &dnsOverQUIC{ + addr: &url.URL{Host: "example:853"}, + bytesPool: pool, + bytesPoolMu: &sync.Mutex{}, + } + + got, err := p.readMsg(stream) + require.NoError(t, err) + require.Len(t, got.Answer, 1) + require.Equal(t, dns.Fqdn("new.example"), got.Question[0].Name) + + a, ok := got.Answer[0].(*dns.A) + require.True(t, ok, "wrong answer type: %T", got.Answer[0]) + require.Equal(t, net.IPv4(2, 2, 2, 2).To4(), a.A) +} + +type testQUICStream struct { + data []byte + chunk int +} + +func (s *testQUICStream) Read(p []byte) (n int, err error) { + if len(s.data) == 0 { + return 0, io.EOF + } + + n = s.chunk + if n <= 0 || n > len(s.data) { + n = len(s.data) + } + if n > len(p) { + n = len(p) + } + + copy(p, s.data[:n]) + s.data = s.data[n:] + + return n, nil +} + +func (s *testQUICStream) CancelRead(_ quic.StreamErrorCode) {} + // testDoHServer is an instance of a test DNS-over-QUIC server. type testDoQServer struct { // listener is the QUIC connections listener. From 78639eba591851e66bf3a301ee111482c08608c2 Mon Sep 17 00:00:00 2001 From: "Yoshiaki Ueda (bootjp)" Date: Thu, 5 Feb 2026 22:12:09 +0900 Subject: [PATCH 2/6] Fix DoQ server parsing to avoid stale pooled bytes When the QUIC server parsed requests it passed the entire pooled buffer to dns.Msg.Unpack, even though only n bytes were read. This could let leftover bytes from previous requests influence parsing. Tighten the slice to the actual read length and add a regression test that simulates a truncated request with a prefilled pool buffer. --- proxy/serverquic.go | 4 +- proxy/serverquic_internal_test.go | 92 +++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 2 deletions(-) diff --git a/proxy/serverquic.go b/proxy/serverquic.go index ae974547..8bc04e5c 100644 --- a/proxy/serverquic.go +++ b/proxy/serverquic.go @@ -248,9 +248,9 @@ func (p *Proxy) handleQUICStream(ctx context.Context, stream *quic.Stream, conn // draft DNS messages were not prefixed with the message length. packetLen := binary.BigEndian.Uint16(buf[:2]) if packetLen == uint16(n-2) { - err = req.Unpack(buf[2:]) + err = req.Unpack(buf[2:n]) } else { - err = req.Unpack(buf) + err = req.Unpack(buf[:n]) doqVersion = DoQv1Draft } diff --git a/proxy/serverquic_internal_test.go b/proxy/serverquic_internal_test.go index a840bb26..5fabf9e2 100644 --- a/proxy/serverquic_internal_test.go +++ b/proxy/serverquic_internal_test.go @@ -6,11 +6,13 @@ import ( "crypto/x509" "io" "net" + "sync/atomic" "testing" "time" "github.com/AdguardTeam/dnsproxy/proxyutil" "github.com/AdguardTeam/golibs/logutil/slogutil" + "github.com/AdguardTeam/golibs/syncutil" "github.com/AdguardTeam/golibs/testutil" "github.com/AdguardTeam/golibs/testutil/servicetest" "github.com/miekg/dns" @@ -148,6 +150,96 @@ func TestQuicProxy_largePackets(t *testing.T) { requireResponse(t, msg, resp) } +func TestQuicProxy_truncatedRequestUsesOnlyReadBytes(t *testing.T) { + serverConfig, caPem := newTLSConfig(t) + + var handled atomic.Bool + conf := &Config{ + Logger: slogutil.NewDiscardLogger(), + QUICListenAddr: []*net.UDPAddr{net.UDPAddrFromAddrPort(localhostAnyPort)}, + TLSConfig: serverConfig, + UpstreamConfig: newTestUpstreamConfig(t, defaultTimeout, testDefaultUpstreamAddr), + TrustedProxies: defaultTrustedProxies, + RatelimitSubnetLenIPv4: 24, + RatelimitSubnetLenIPv6: 64, + RequestHandler: func(_ *Proxy, d *DNSContext) (err error) { + handled.Store(true) + resp := (&dns.Msg{}).SetReply(d.Req) + resp.Answer = []dns.RR{&dns.A{ + Hdr: dns.RR_Header{ + Name: d.Req.Question[0].Name, + Rrtype: dns.TypeA, + Class: dns.ClassINET, + }, + A: net.IP{8, 8, 8, 8}, + }} + d.Res = resp + + return nil + }, + } + + dnsProxy := mustNew(t, conf) + + req := &dns.Msg{ + MsgHdr: dns.MsgHdr{ + Id: 0, + RecursionDesired: true, + }, + Question: []dns.Question{{ + Name: "example.org.", + Qtype: dns.TypeA, + Qclass: dns.ClassINET, + }}, + } + req.SetEdns0(4096, false) + + packed, err := req.Pack() + require.NoError(t, err) + + fullBuf := proxyutil.AddPrefix(packed) + + dnsProxy.bytesPool = syncutil.NewPool(func() (v *[]byte) { + b := make([]byte, 2+dns.MaxMsgSize) + copy(b, fullBuf) + + return &b + }) + + servicetest.RequireRun(t, dnsProxy, testTimeout) + + roots := x509.NewCertPool() + roots.AppendCertsFromPEM(caPem) + tlsConfig := &tls.Config{ + ServerName: tlsServerName, + RootCAs: roots, + NextProtos: append([]string{NextProtoDQ}, compatProtoDQ...), + } + + addr := dnsProxy.Addr(ProtoQUIC) + conn, err := quic.DialAddrEarly(context.Background(), addr.String(), tlsConfig, nil) + require.NoError(t, err) + testutil.CleanupAndRequireSuccess(t, func() (err error) { + return conn.CloseWithError(DoQCodeNoError, "") + }) + + truncLen := len(packed) - 5 + require.Greater(t, truncLen, 0) + truncated := packed[:truncLen] + reqBuf := proxyutil.AddPrefix(truncated) + require.Greater(t, len(reqBuf), minDNSPacketSize) + + stream, err := conn.OpenStreamSync(context.Background()) + require.NoError(t, err) + _, err = stream.Write(reqBuf) + require.NoError(t, err) + _ = stream.Close() + + require.Never(t, func() bool { + return handled.Load() + }, testTimeout, 10*time.Millisecond) +} + // sendQUICMessage sends msg to the specified QUIC connection. func sendQUICMessage( t *testing.T, From a236f13e46a2d5ee7f5488d5337e689f1a7a2b86 Mon Sep 17 00:00:00 2001 From: Eugene Burkov Date: Fri, 6 Feb 2026 17:40:54 +0300 Subject: [PATCH 3/6] all: imp test code --- proxy/serverquic_internal_test.go | 51 ++++++++++++------------------- upstream/doq_internal_test.go | 36 +++++++++++++--------- 2 files changed, 41 insertions(+), 46 deletions(-) diff --git a/proxy/serverquic_internal_test.go b/proxy/serverquic_internal_test.go index cf480a9a..866de6d2 100644 --- a/proxy/serverquic_internal_test.go +++ b/proxy/serverquic_internal_test.go @@ -6,7 +6,6 @@ import ( "crypto/x509" "io" "net" - "sync/atomic" "testing" "time" @@ -17,6 +16,7 @@ import ( "github.com/AdguardTeam/golibs/testutil/servicetest" "github.com/miekg/dns" "github.com/quic-go/quic-go" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -143,10 +143,9 @@ func TestQuicProxy_largePackets(t *testing.T) { requireResponse(t, msg, resp) } -func TestQuicProxy_truncatedRequestUsesOnlyReadBytes(t *testing.T) { +func TestQuicProxy_truncatedRequest(t *testing.T) { serverConfig, caPem := newTLSConfig(t) - var handled atomic.Bool conf := &Config{ Logger: slogutil.NewDiscardLogger(), QUICListenAddr: []*net.UDPAddr{net.UDPAddrFromAddrPort(localhostAnyPort)}, @@ -156,27 +155,15 @@ func TestQuicProxy_truncatedRequestUsesOnlyReadBytes(t *testing.T) { RatelimitSubnetLenIPv4: 24, RatelimitSubnetLenIPv6: 64, RequestHandler: &TestRequestHandler{ - OnHandle: func(p *Proxy, dctx *DNSContext) (err error) { - handled.Store(true) - resp := (&dns.Msg{}).SetReply(dctx.Req) - resp.Answer = []dns.RR{&dns.A{ - Hdr: dns.RR_Header{ - Name: dctx.Req.Question[0].Name, - Rrtype: dns.TypeA, - Class: dns.ClassINET, - }, - A: net.IP{8, 8, 8, 8}, - }} - dctx.Res = resp - - return nil + OnHandle: func(p *Proxy, d *DNSContext) (_ error) { + panic(testutil.UnexpectedCall(p, d)) }, }, } dnsProxy := mustNew(t, conf) - req := &dns.Msg{ + req := (&dns.Msg{ MsgHdr: dns.MsgHdr{ Id: 0, RecursionDesired: true, @@ -186,8 +173,7 @@ func TestQuicProxy_truncatedRequestUsesOnlyReadBytes(t *testing.T) { Qtype: dns.TypeA, Qclass: dns.ClassINET, }}, - } - req.SetEdns0(4096, false) + }).SetEdns0(4096, false) packed, err := req.Pack() require.NoError(t, err) @@ -203,36 +189,39 @@ func TestQuicProxy_truncatedRequestUsesOnlyReadBytes(t *testing.T) { servicetest.RequireRun(t, dnsProxy, testTimeout) + addr := dnsProxy.Addr(ProtoQUIC) + roots := x509.NewCertPool() - roots.AppendCertsFromPEM(caPem) + require.True(t, roots.AppendCertsFromPEM(caPem)) + tlsConfig := &tls.Config{ ServerName: tlsServerName, RootCAs: roots, NextProtos: append([]string{NextProtoDQ}, compatProtoDQ...), } - addr := dnsProxy.Addr(ProtoQUIC) - conn, err := quic.DialAddrEarly(context.Background(), addr.String(), tlsConfig, nil) + ctx := testutil.ContextWithTimeout(t, testTimeout) + + conn, err := quic.DialAddrEarly(ctx, addr.String(), tlsConfig, nil) require.NoError(t, err) testutil.CleanupAndRequireSuccess(t, func() (err error) { return conn.CloseWithError(DoQCodeNoError, "") }) - truncLen := len(packed) - 5 + truncLen := len(packed) - len(packed)/2 require.Greater(t, truncLen, 0) + truncated := packed[:truncLen] reqBuf := proxyutil.AddPrefix(truncated) require.Greater(t, len(reqBuf), minDNSPacketSize) - stream, err := conn.OpenStreamSync(context.Background()) + stream, err := conn.OpenStreamSync(ctx) require.NoError(t, err) - _, err = stream.Write(reqBuf) - require.NoError(t, err) - _ = stream.Close() + testutil.CleanupAndRequireSuccess(t, stream.Close) - require.Never(t, func() bool { - return handled.Load() - }, testTimeout, 10*time.Millisecond) + n, err := stream.Write(reqBuf) + require.NoError(t, err) + assert.Greater(t, n, minDNSPacketSize) } // newTestResponse creates a test response for the specified request. diff --git a/upstream/doq_internal_test.go b/upstream/doq_internal_test.go index 32d46b63..01a41504 100644 --- a/upstream/doq_internal_test.go +++ b/upstream/doq_internal_test.go @@ -232,7 +232,7 @@ func TestUpstreamDoQ_0RTT(t *testing.T) { require.True(t, conns[1].is0RTT()) } -func TestUpstreamDoQ_readMsg_partialRead(t *testing.T) { +func TestUpstreamDoQ_ReadMsg_partialRead(t *testing.T) { oldReq := createHostTestMessage("old.example") oldResp := (&dns.Msg{}).SetReply(oldReq) oldResp.Answer = []dns.RR{&dns.A{ @@ -242,23 +242,34 @@ func TestUpstreamDoQ_readMsg_partialRead(t *testing.T) { Class: dns.ClassINET, Ttl: 60, }, - A: net.IPv4(1, 1, 1, 1), + A: net.IP{192, 0, 2, 1}, }} oldPacked, err := oldResp.Pack() require.NoError(t, err) + oldBuf := make([]byte, 2+len(oldPacked)) binary.BigEndian.PutUint16(oldBuf, uint16(len(oldPacked))) copy(oldBuf[2:], oldPacked) pool := &sync.Pool{ - New: func() any { + New: func() (buf any) { b := make([]byte, dns.MaxMsgSize) copy(b, oldBuf) + return &b }, } + ups, err := newDoQ(&url.URL{Host: "example:853"}, &Options{}) + require.NoError(t, err) + testutil.CleanupAndRequireSuccess(t, ups.Close) + + doq := testutil.RequireTypeAssert[*dnsOverQUIC](t, ups) + doq.bytesPool = pool + + newAnsIP := net.IP{192, 0, 2, 2} + newReq := createHostTestMessage("new.example") newResp := (&dns.Msg{}).SetReply(newReq) newResp.Answer = []dns.RR{&dns.A{ @@ -268,11 +279,12 @@ func TestUpstreamDoQ_readMsg_partialRead(t *testing.T) { Class: dns.ClassINET, Ttl: 60, }, - A: net.IPv4(2, 2, 2, 2), + A: newAnsIP, }} newPacked, err := newResp.Pack() require.NoError(t, err) + newBuf := make([]byte, 2+len(newPacked)) binary.BigEndian.PutUint16(newBuf, uint16(len(newPacked))) copy(newBuf[2:], newPacked) @@ -282,20 +294,14 @@ func TestUpstreamDoQ_readMsg_partialRead(t *testing.T) { chunk: 1, } - p := &dnsOverQUIC{ - addr: &url.URL{Host: "example:853"}, - bytesPool: pool, - bytesPoolMu: &sync.Mutex{}, - } - - got, err := p.readMsg(stream) + got, err := doq.readMsg(stream) require.NoError(t, err) require.Len(t, got.Answer, 1) - require.Equal(t, dns.Fqdn("new.example"), got.Question[0].Name) - a, ok := got.Answer[0].(*dns.A) - require.True(t, ok, "wrong answer type: %T", got.Answer[0]) - require.Equal(t, net.IPv4(2, 2, 2, 2).To4(), a.A) + assert.Equal(t, dns.Fqdn("new.example"), got.Question[0].Name) + + a := testutil.RequireTypeAssert[*dns.A](t, got.Answer[0]) + assert.Equal(t, newAnsIP, a.A) } type testQUICStream struct { From 1d84f3eacfc3fa2a7df4dca5d075df7ae50458fd Mon Sep 17 00:00:00 2001 From: Eugene Burkov Date: Mon, 9 Feb 2026 20:30:27 +0300 Subject: [PATCH 4/6] all: imp code, docs --- proxy/proxy_internal_test.go | 2 + proxy/serverquic_internal_test.go | 7 ++-- upstream/doq.go | 40 +++++++++++-------- upstream/doq_internal_test.go | 64 ++++++++++++++++++------------- 4 files changed, 69 insertions(+), 44 deletions(-) diff --git a/proxy/proxy_internal_test.go b/proxy/proxy_internal_test.go index 5d216a72..ff5d0591 100644 --- a/proxy/proxy_internal_test.go +++ b/proxy/proxy_internal_test.go @@ -63,6 +63,8 @@ var defaultTrustedProxies netutil.SubnetSet = netutil.SliceSubnetSet{ // mustNew wraps [New] function failing the test on error. // // TODO(e.burkov): Move into the proxytest package. +// +// TODO(e.burkov): Put default values into configuration. func mustNew(tb testing.TB, conf *Config) (p *Proxy) { tb.Helper() diff --git a/proxy/serverquic_internal_test.go b/proxy/serverquic_internal_test.go index 866de6d2..1331c01e 100644 --- a/proxy/serverquic_internal_test.go +++ b/proxy/serverquic_internal_test.go @@ -20,7 +20,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestQuicProxy(t *testing.T) { +func TestProxy_quic(t *testing.T) { serverConfig, caPem := newTLSConfig(t) roots := x509.NewCertPool() @@ -85,7 +85,7 @@ func TestQuicProxy(t *testing.T) { }) } -func TestQuicProxy_largePackets(t *testing.T) { +func TestProxy_quicLargePackets(t *testing.T) { reqHandler := &TestRequestHandler{ OnHandle: func(_ *Proxy, d *DNSContext) (err error) { d.Res = newTestResponse(d) @@ -143,7 +143,7 @@ func TestQuicProxy_largePackets(t *testing.T) { requireResponse(t, msg, resp) } -func TestQuicProxy_truncatedRequest(t *testing.T) { +func TestProxy_quicTruncatedRequest(t *testing.T) { serverConfig, caPem := newTLSConfig(t) conf := &Config{ @@ -221,6 +221,7 @@ func TestQuicProxy_truncatedRequest(t *testing.T) { n, err := stream.Write(reqBuf) require.NoError(t, err) + assert.Greater(t, n, minDNSPacketSize) } diff --git a/upstream/doq.go b/upstream/doq.go index a936dfa1..8dd03ef1 100644 --- a/upstream/doq.go +++ b/upstream/doq.go @@ -11,12 +11,14 @@ import ( "net/url" "os" "runtime" + "slices" "sync" "time" "github.com/AdguardTeam/dnsproxy/proxyutil" "github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/logutil/slogutil" + "github.com/AdguardTeam/golibs/validate" "github.com/miekg/dns" "github.com/quic-go/quic-go" "github.com/quic-go/quic-go/http3" @@ -97,12 +99,23 @@ type dnsOverQUIC struct { timeout time.Duration } -// quicStream is the subset of quic.Stream used by readMsg. +// quicStream is the interface of QUIC stream used by readMsg to simplify +// testing. +// +// Note, that this interface is implemented by [*quic.Stream]. type quicStream interface { + // Read reads data from the stream. If the stream was canceled, the error + // is a [quic.StreamError]. Read(p []byte) (n int, err error) + + // CancelRead aborts receiving on this stream. See + // [quic.ReceiveStream.CancelRead] for more details. CancelRead(code quic.StreamErrorCode) } +// type check +var _ quicStream = (*quic.Stream)(nil) + // newDoQ returns the DNS-over-QUIC Upstream. func newDoQ(addr *url.URL, opts *Options) (u Upstream, err error) { addPort(addr, defaultPortDoQ) @@ -406,33 +419,30 @@ func (p *dnsOverQUIC) closeConnWithError(conn *quic.Conn, err error) { // readMsg reads the incoming DNS message from the QUIC stream. func (p *dnsOverQUIC) readMsg(stream quicStream) (m *dns.Msg, err error) { + defer func() { err = errors.Annotate(err, "from %s: %w", p.addr) }() + pool := p.getBytesPool() bufPtr := pool.Get().(*[]byte) - defer pool.Put(bufPtr) var lenBuf [2]byte _, err = io.ReadFull(stream, lenBuf[:]) if err != nil { - return nil, fmt.Errorf("reading response length from %s: %w", p.addr, err) + return nil, fmt.Errorf("reading response length: %w", err) } - msgLen := int(binary.BigEndian.Uint16(lenBuf[:])) - if msgLen == 0 { - return nil, fmt.Errorf("empty response from %s", p.addr) - } else if msgLen > dns.MaxMsgSize { - return nil, fmt.Errorf("response size %d exceeds max %d from %s", msgLen, dns.MaxMsgSize, p.addr) + msgLen := binary.BigEndian.Uint16(lenBuf[:]) + err = validate.NotEmpty("response", msgLen) + if err != nil { + return nil, err } - respBuf := *bufPtr - if cap(respBuf) < msgLen { - respBuf = make([]byte, msgLen) - } - respBuf = respBuf[:msgLen] + *bufPtr = slices.Grow(*bufPtr, int(msgLen)) + respBuf := (*bufPtr)[:msgLen] _, err = io.ReadFull(stream, respBuf) if err != nil { - return nil, fmt.Errorf("reading response from %s: %w", p.addr, err) + return nil, err } stream.CancelRead(0) @@ -443,7 +453,7 @@ func (p *dnsOverQUIC) readMsg(stream quicStream) (m *dns.Msg, err error) { m = new(dns.Msg) err = m.Unpack(respBuf) if err != nil { - return nil, fmt.Errorf("unpacking response from %s: %w", p.addr, err) + return nil, fmt.Errorf("from %s: unpacking response: %w", p.addr, err) } return m, nil diff --git a/upstream/doq_internal_test.go b/upstream/doq_internal_test.go index 01a41504..cc6157d4 100644 --- a/upstream/doq_internal_test.go +++ b/upstream/doq_internal_test.go @@ -24,7 +24,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestUpstreamDoQ(t *testing.T) { +func TestDNSOverQUIC(t *testing.T) { tlsConf, rootCAs := createServerTLSConfig(t, "127.0.0.1") srv := startDoQServer(t, tlsConf, 0) @@ -77,7 +77,7 @@ func TestUpstreamDoQ(t *testing.T) { checkRaceCondition(u) } -func TestUpstream_Exchange_quicServerCloseConn(t *testing.T) { +func TestDNSOverQUIC_Exchange_quicCloseConn(t *testing.T) { // Use the same tlsConf for all servers to preserve the data necessary for // 0-RTT connections. tlsConf, rootCAs := createServerTLSConfig(t, "127.0.0.1") @@ -129,7 +129,7 @@ func TestUpstream_Exchange_quicServerCloseConn(t *testing.T) { wg.Wait() } -func TestUpstreamDoQ_serverRestart(t *testing.T) { +func TestDNSOverQUIC_serverRestart(t *testing.T) { t.Parallel() // Use the same tlsConf for all servers to preserve the data necessary for @@ -182,7 +182,7 @@ func TestUpstreamDoQ_serverRestart(t *testing.T) { }) } -func TestUpstreamDoQ_0RTT(t *testing.T) { +func TestDNSOverQUIC_0RTT(t *testing.T) { tlsConf, rootCAs := createServerTLSConfig(t, "127.0.0.1") srv := startDoQServer(t, tlsConf, 0) @@ -232,7 +232,7 @@ func TestUpstreamDoQ_0RTT(t *testing.T) { require.True(t, conns[1].is0RTT()) } -func TestUpstreamDoQ_ReadMsg_partialRead(t *testing.T) { +func TestDNSOverQUIC_ReadMsg_partialRead(t *testing.T) { oldReq := createHostTestMessage("old.example") oldResp := (&dns.Msg{}).SetReply(oldReq) oldResp.Answer = []dns.RR{&dns.A{ @@ -290,8 +290,25 @@ func TestUpstreamDoQ_ReadMsg_partialRead(t *testing.T) { copy(newBuf[2:], newPacked) stream := &testQUICStream{ - data: newBuf, - chunk: 1, + onRead: func(p []byte) (n int, err error) { + if len(newBuf) == 0 { + return 0, io.EOF + } + + n = 1 + if n <= 0 || n > len(newBuf) { + n = len(newBuf) + } + n = min(n, len(p)) + + copy(p, newBuf[:n]) + newBuf = newBuf[n:] + + return n, nil + }, + onCancelRead: func(code quic.StreamErrorCode) { + assert.Equal(t, quic.StreamErrorCode(0), code) + }, } got, err := doq.readMsg(stream) @@ -301,34 +318,29 @@ func TestUpstreamDoQ_ReadMsg_partialRead(t *testing.T) { assert.Equal(t, dns.Fqdn("new.example"), got.Question[0].Name) a := testutil.RequireTypeAssert[*dns.A](t, got.Answer[0]) + assert.Equal(t, newAnsIP, a.A) } +// testQUICStream is a mock implementation of the [quicStream] interface for +// tests. type testQUICStream struct { - data []byte - chunk int + onRead func(p []byte) (n int, err error) + onCancelRead func(code quic.StreamErrorCode) } +// type check +var _ quicStream = (*testQUICStream)(nil) + +// Read implements the [quicStream] interface for *testQUICStream. func (s *testQUICStream) Read(p []byte) (n int, err error) { - if len(s.data) == 0 { - return 0, io.EOF - } - - n = s.chunk - if n <= 0 || n > len(s.data) { - n = len(s.data) - } - if n > len(p) { - n = len(p) - } - - copy(p, s.data[:n]) - s.data = s.data[n:] - - return n, nil + return s.onRead(p) } -func (s *testQUICStream) CancelRead(_ quic.StreamErrorCode) {} +// CancelRead implements the [quicStream] interface for *testQUICStream. +func (s *testQUICStream) CancelRead(code quic.StreamErrorCode) { + s.onCancelRead(code) +} // testDoHServer is an instance of a test DNS-over-QUIC server. type testDoQServer struct { From 7b3f563fa95e9474bfac7f766aabe5d412dcaa1a Mon Sep 17 00:00:00 2001 From: Eugene Burkov Date: Tue, 10 Feb 2026 20:17:11 +0300 Subject: [PATCH 5/6] upstream: imp code --- upstream/doq.go | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/upstream/doq.go b/upstream/doq.go index 8dd03ef1..f0ec7bbc 100644 --- a/upstream/doq.go +++ b/upstream/doq.go @@ -11,7 +11,6 @@ import ( "net/url" "os" "runtime" - "slices" "sync" "time" @@ -421,10 +420,6 @@ func (p *dnsOverQUIC) closeConnWithError(conn *quic.Conn, err error) { func (p *dnsOverQUIC) readMsg(stream quicStream) (m *dns.Msg, err error) { defer func() { err = errors.Annotate(err, "from %s: %w", p.addr) }() - pool := p.getBytesPool() - bufPtr := pool.Get().(*[]byte) - defer pool.Put(bufPtr) - var lenBuf [2]byte _, err = io.ReadFull(stream, lenBuf[:]) if err != nil { @@ -432,12 +427,16 @@ func (p *dnsOverQUIC) readMsg(stream quicStream) (m *dns.Msg, err error) { } msgLen := binary.BigEndian.Uint16(lenBuf[:]) - err = validate.NotEmpty("response", msgLen) + err = validate.Positive("response length", msgLen) if err != nil { + // Don't wrap the error, since it's informative enough as is. return nil, err } - *bufPtr = slices.Grow(*bufPtr, int(msgLen)) + pool := p.getBytesPool() + bufPtr := pool.Get().(*[]byte) + defer pool.Put(bufPtr) + respBuf := (*bufPtr)[:msgLen] _, err = io.ReadFull(stream, respBuf) @@ -450,7 +449,7 @@ func (p *dnsOverQUIC) readMsg(stream quicStream) (m *dns.Msg, err error) { // All DNS messages (queries and responses) sent over DoQ connections MUST // be encoded as a 2-octet length field followed by the message content as // specified in [RFC1035]. - m = new(dns.Msg) + m = &dns.Msg{} err = m.Unpack(respBuf) if err != nil { return nil, fmt.Errorf("from %s: unpacking response: %w", p.addr, err) From 59ca8a85542ca79ad1d81bc0fecfc99f07699e23 Mon Sep 17 00:00:00 2001 From: Eugene Burkov Date: Wed, 11 Feb 2026 20:36:49 +0300 Subject: [PATCH 6/6] upstream: imp test --- upstream/doq_internal_test.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/upstream/doq_internal_test.go b/upstream/doq_internal_test.go index cc6157d4..2add53b2 100644 --- a/upstream/doq_internal_test.go +++ b/upstream/doq_internal_test.go @@ -295,11 +295,7 @@ func TestDNSOverQUIC_ReadMsg_partialRead(t *testing.T) { return 0, io.EOF } - n = 1 - if n <= 0 || n > len(newBuf) { - n = len(newBuf) - } - n = min(n, len(p)) + n = min(1, len(newBuf), len(p)) copy(p, newBuf[:n]) newBuf = newBuf[n:]