From 01a2bbaa1146370a60c325908475455c069291f9 Mon Sep 17 00:00:00 2001 From: ginuerzh Date: Thu, 21 May 2026 22:36:30 +0800 Subject: [PATCH] fix(masque): address review findings for PR #76 - Return errors from MasqueConn.Read/Write instead of silently succeeding (prevents infinite busy-loops and silent data loss if these methods are accidentally called). - Set recorder Network field in protocol dispatch switch rather than hardcoding "udp", so early errors don't misreport type. - Clarify that DatagramConn context-ID handling only supports context ID 0 (sufficient for CONNECT-UDP per RFC 9298). --- dialer/http3/masque/client.go | 13 +++++++------ handler/masque/handler.go | 3 ++- internal/util/masque/conn.go | 10 ++++++---- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/dialer/http3/masque/client.go b/dialer/http3/masque/client.go index 5ae28a54..c482afd6 100644 --- a/dialer/http3/masque/client.go +++ b/dialer/http3/masque/client.go @@ -3,6 +3,7 @@ package masque import ( "context" "crypto/tls" + "errors" "net" "time" @@ -11,6 +12,8 @@ import ( "github.com/quic-go/quic-go/http3" ) +var errReadWriteNotSupported = errors.New("masque: direct Read/Write not supported on MasqueConn, use GetRequestStream") + // Client manages HTTP/3 connections for MASQUE proxying. // It wraps a QUIC connection and HTTP/3 client, following the session pattern // used by other gost dialers (ssh, quic, mws, etc.). @@ -105,17 +108,15 @@ func (c *MasqueConn) GetHost() string { } // Read implements net.Conn but is not used for MASQUE. -// The actual data transfer happens via datagrams. +// The actual data transfer happens via datagrams or the request stream. func (c *MasqueConn) Read(b []byte) (n int, err error) { - // This should not be called - datagrams are used for data transfer - return 0, nil + return 0, errReadWriteNotSupported } // Write implements net.Conn but is not used for MASQUE. -// The actual data transfer happens via datagrams. +// The actual data transfer happens via datagrams or the request stream. func (c *MasqueConn) Write(b []byte) (n int, err error) { - // This should not be called - datagrams are used for data transfer - return len(b), nil + return 0, errReadWriteNotSupported } // Close closes the connection. diff --git a/handler/masque/handler.go b/handler/masque/handler.go index d6e798d4..565d4a88 100644 --- a/handler/masque/handler.go +++ b/handler/masque/handler.go @@ -107,7 +107,7 @@ func (h *masqueHandler) Handle(ctx context.Context, conn net.Conn, opts ...handl start := time.Now() ro := &xrecorder.HandlerRecorderObject{ - Network: "udp", + Network: "", Service: h.options.Service, RemoteAddr: conn.RemoteAddr().String(), LocalAddr: conn.LocalAddr().String(), @@ -183,6 +183,7 @@ func (h *masqueHandler) Handle(ctx context.Context, conn net.Conn, opts ...handl switch r.Proto { case "connect-udp": // Extended CONNECT for UDP (RFC 9298) + ro.Network = "udp" return h.handleConnectUDP(ctx, w, r, conn.LocalAddr(), ro, log) case "HTTP/3.0", "": // Standard CONNECT for TCP (RFC 9114) diff --git a/internal/util/masque/conn.go b/internal/util/masque/conn.go index 3ea66e20..3f6814ea 100644 --- a/internal/util/masque/conn.go +++ b/internal/util/masque/conn.go @@ -83,9 +83,11 @@ func (c *DatagramConn) ReadFrom(b []byte) (n int, addr net.Addr, err error) { return 0, c.remoteAddr, nil } - // Per RFC 9297: datagram format is context-id (varint) + payload - // For CONNECT-UDP with context ID 0, the first byte is 0x00 - // We strip the context ID prefix + // Per RFC 9297: datagram format is context-id (varint) + payload. + // For CONNECT-UDP (RFC 9298), the context ID is always 0. + // QUIC varints encode 0 as a single byte 0x00. + // NOTE: Only context ID 0 is supported. Non-zero context IDs would + // require full QUIC varint decoding. if data[0] == 0x00 { data = data[1:] } @@ -103,7 +105,7 @@ func (c *DatagramConn) WriteTo(b []byte, addr net.Addr) (n int, err error) { default: } - // Prepend context ID (0x00 for context ID 0) + // Prepend context ID (0x00 for CONNECT-UDP context ID 0 per RFC 9298) datagram := make([]byte, 1+len(b)) datagram[0] = 0x00 // Context ID = 0 copy(datagram[1:], b)