diff --git a/AUTHORS b/AUTHORS index 05e71df48..42c7f02c0 100644 --- a/AUTHORS +++ b/AUTHORS @@ -92,6 +92,7 @@ Lunny Xiao Maciej Zimnoch Michael Woolnough Minh Quang +Morgan Tocker Nao Yokotsuka Nathanial Murphy Nicola Peduzzi @@ -136,6 +137,7 @@ Ziheng Lyu # Organizations Barracuda Networks, Inc. +Block, Inc. Counting Ltd. Defined Networking Inc. DigitalOcean Inc. diff --git a/connection.go b/connection.go index 5648e47d8..b1660a508 100644 --- a/connection.go +++ b/connection.go @@ -453,12 +453,11 @@ func (mc *mysqlConn) query(query string, args []driver.Value) (*textRows, error) } // Gets the value of the given MySQL System Variable -// The returned byte slice is only valid until the next read -func (mc *mysqlConn) getSystemVar(name string) ([]byte, error) { +func (mc *mysqlConn) getSystemVar(name string) (string, error) { // Send command handleOk := mc.clearResult() if err := mc.writeCommandPacketStr(comQuery, "SELECT @@"+name); err != nil { - return nil, err + return "", err } // Read Result @@ -471,16 +470,19 @@ func (mc *mysqlConn) getSystemVar(name string) ([]byte, error) { if resLen > 0 { // Columns if err := mc.skipColumns(resLen); err != nil { - return nil, err + return "", err } } dest := make([]driver.Value, resLen) if err = rows.readRow(dest); err == nil { - return dest[0].([]byte), mc.skipRows() + // Convert to string before skipRows, which may + // overwrite the read buffer that dest[0] points into. + val := string(dest[0].([]byte)) + return val, mc.skipRows() } } - return nil, err + return "", err } // cancel is called when the query has canceled. diff --git a/connection_test.go b/connection_test.go index 440ecbff7..d489c1e3e 100644 --- a/connection_test.go +++ b/connection_test.go @@ -15,6 +15,7 @@ import ( "errors" "net" "testing" + "time" ) func TestInterpolateParams(t *testing.T) { @@ -204,3 +205,118 @@ func (bc badConnection) Write(b []byte) (n int, err error) { func (bc badConnection) Close() error { return nil } + +// chunkedConn is a net.Conn that serves pre-built data chunks, one per Read +// call. This simulates the behavior seen with TLS connections, where the +// server's TLS library typically produces a separate TLS record per write +// and Go's crypto/tls.Read returns one record at a time. +type chunkedConn struct { + chunks [][]byte + idx int // current chunk index + off int // offset within current chunk +} + +func (c *chunkedConn) Read(b []byte) (int, error) { + if c.idx >= len(c.chunks) { + return 0, errors.New("no more data") + } + n := copy(b, c.chunks[c.idx][c.off:]) + c.off += n + if c.off >= len(c.chunks[c.idx]) { + c.idx++ + c.off = 0 + } + return n, nil +} + +func (c *chunkedConn) Write(b []byte) (int, error) { return len(b), nil } // swallow writes (e.g. COM_QUERY) +func (c *chunkedConn) Close() error { return nil } +func (c *chunkedConn) LocalAddr() net.Addr { return nil } +func (c *chunkedConn) RemoteAddr() net.Addr { return nil } +func (c *chunkedConn) SetDeadline(_ time.Time) error { return nil } +func (c *chunkedConn) SetReadDeadline(_ time.Time) error { return nil } +func (c *chunkedConn) SetWriteDeadline(_ time.Time) error { return nil } + +var _ net.Conn = (*chunkedConn)(nil) + +// makePacket wraps a payload in a MySQL protocol packet header. +func makePacket(seq byte, payload []byte) []byte { + pkt := make([]byte, 4+len(payload)) + pkt[0] = byte(len(payload)) + pkt[1] = byte(len(payload) >> 8) + pkt[2] = byte(len(payload) >> 16) + pkt[3] = seq + copy(pkt[4:], payload) + return pkt +} + +// TestGetSystemVarBufferReuse verifies that getSystemVar returns a value that +// is not corrupted by the subsequent skipRows call. +// +// The row value returned by readRow points into the read buffer. skipRows may +// call fill(), which overwrites that memory. The test feeds each protocol +// packet as a separate Read call via chunkedConn (mimicking TLS record +// boundaries), guaranteeing that fill() is called for the trailing EOF. +func TestGetSystemVarBufferReuse(t *testing.T) { + // Protocol response for: SELECT @@max_allowed_packet → "67108864" + // + // Sequence numbers start at 1 (client sent COM_QUERY as seq 0). + // + // seq 1: column count = 1 + // seq 2: column definition (minimal valid) + // seq 3: EOF (end of column defs) + // seq 4: row data — length-encoded string "67108864" + // seq 5: EOF (end of rows) + + colCountPkt := makePacket(1, []byte{0x01}) + + colDef := []byte{ + 0x03, 'd', 'e', 'f', // catalog = "def" + 0x00, // schema = "" + 0x00, // table = "" + 0x00, // org_table = "" + 0x14, // name length = 20 + '@', '@', 'm', 'a', 'x', '_', 'a', 'l', 'l', 'o', + 'w', 'e', 'd', '_', 'p', 'a', 'c', 'k', 'e', 't', + 0x00, // org_name = "" + 0x0c, // length of fixed fields + 0x3f, 0x00, // charset = 63 (binary) + 0x14, 0x00, 0x00, 0x00, // column_length = 20 + 0x0f, // type = FIELD_TYPE_VARCHAR + 0x00, 0x00, // flags + 0x00, // decimals + 0x00, 0x00, // filler + } + colDefPkt := makePacket(2, colDef) + + eof1 := makePacket(3, []byte{0xfe, 0x00, 0x00, 0x02, 0x00}) + + // Row: length-encoded string "67108864" (8 bytes → length prefix 0x08) + rowPkt := makePacket(4, []byte{0x08, '6', '7', '1', '0', '8', '8', '6', '4'}) + + eof2 := makePacket(5, []byte{0xfe, 0x00, 0x00, 0x02, 0x00}) + + // Each packet arrives in its own Read call, simulating TLS record + // boundaries where each server Write becomes a separate TLS record + // and each client Read returns exactly one record. + conn := &chunkedConn{chunks: [][]byte{colCountPkt, colDefPkt, eof1, rowPkt, eof2}} + + mc := &mysqlConn{ + netConn: conn, + buf: newBuffer(), + cfg: NewConfig(), + closech: make(chan struct{}), + maxAllowedPacket: defaultMaxAllowedPacket, + sequence: 1, // after COM_QUERY (seq 0) + } + + val, err := mc.getSystemVar("max_allowed_packet") + if err != nil { + t.Fatalf("getSystemVar failed: %v", err) + } + + const expected = "67108864" + if val != expected { + t.Fatalf("getSystemVar(max_allowed_packet) = %q, want %q", val, expected) + } +} diff --git a/connector.go b/connector.go index db2bd7cf9..978fda415 100644 --- a/connector.go +++ b/connector.go @@ -182,7 +182,7 @@ func (c *connector) Connect(ctx context.Context) (driver.Conn, error) { mc.Close() return nil, err } - n, err := strconv.Atoi(string(maxap)) + n, err := strconv.Atoi(maxap) if err != nil { mc.Close() return nil, fmt.Errorf("invalid max_allowed_packet value (%q): %w", maxap, err)