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).
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user