diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a7f1c0ee..c51eacea2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,11 @@ unreleased ### Fixes +- The limit on error responses added in v1.11.0 ([#1248]) should only apply on + errors during the connection phase ([#1326]). + +- Limit the size of some server responses, identical to libpq ([#1326]). + - Add Redshift-specific OID mappings ([#1291], [#1317]). - Use correct environment variable name for `PGSSLMINPROTOCOLVERSION` and @@ -21,6 +26,7 @@ unreleased [#1291]: https://github.com/lib/pq/pull/1291 [#1310]: https://github.com/lib/pq/pull/1310 [#1317]: https://github.com/lib/pq/pull/1317 +[#1326]: https://github.com/lib/pq/pull/1326 v1.12.3 (2026-04-03) diff --git a/conn.go b/conn.go index 3ed7ecfa4..18048ac65 100644 --- a/conn.go +++ b/conn.go @@ -476,8 +476,7 @@ func dial(ctx context.Context, d Dialer, cfg Config) (net.Conn, error) { } func (cn *conn) isInTransaction() bool { - return cn.txnStatus == txnStatusIdleInTransaction || - cn.txnStatus == txnStatusInFailedTransaction + return cn.txnStatus == txnStatusIdleInTransaction || cn.txnStatus == txnStatusInFailedTransaction } func (cn *conn) checkIsInTransaction(intxn bool) error { @@ -1131,10 +1130,15 @@ func (cn *conn) recvMessage(r *readBuf) (proto.ResponseCode, error) { // // libpq checks "if ErrorResponse && (msgLength < 8 || msgLength > MAX_ERRLEN)", // but check < 4 since n represents bytes remaining to be read after length. - if t == proto.ErrorResponse && (n < 4 || n > proto.MaxErrlen) { + // + // Use txnStatus to check if we're in the startup phase. + if cn.txnStatus == 0 && t == proto.ErrorResponse && (n < 4 || n > proto.MaxMsgLen) { msg, _ := cn.buf.ReadString('\x00') return 0, fmt.Errorf("pq: server error: %s%s", string(x[1:]), strings.TrimSuffix(msg, "\x00")) } + if !proto.ValidLongMessageType(t) && n > proto.MaxMsgLen { + return 0, fmt.Errorf("pq: lost synchronization with server: got message type %q, length %d", t, n) + } var y []byte if n <= len(cn.scratch) { @@ -1153,11 +1157,11 @@ func (cn *conn) recvMessage(r *readBuf) (proto.ResponseCode, error) { return t, nil } -// recv receives a message from the backend, returning an error if an error -// happened while reading the message or the received message an ErrorResponse. -// NoticeResponses are ignored. This function should generally be used only -// during the startup sequence. -func (cn *conn) recv() (proto.ResponseCode, *readBuf, error) { +// recvError receives a message from the backend, returning an error if an error +// happened while reading the message or the received message is an +// ErrorResponse. NoticeResponses are ignored. This function should generally be +// used only during the startup sequence. +func (cn *conn) recvError() (proto.ResponseCode, *readBuf, error) { for { r := new(readBuf) t, err := cn.recvMessage(r) @@ -1370,7 +1374,7 @@ func (cn *conn) startup(cfg Config) error { var didauth bool for { - t, r, err := cn.recv() + t, r, err := cn.recvError() if err != nil { return err } @@ -1515,7 +1519,7 @@ func (cn *conn) auth(code proto.AuthCode, r *readBuf, cfg Config) error { return err } - t, r, err := cn.recv() + t, r, err := cn.recvError() if err != nil { return err } @@ -1541,7 +1545,7 @@ func (cn *conn) auth(code proto.AuthCode, r *readBuf, cfg Config) error { return err } - t, r, err = cn.recv() + t, r, err = cn.recvError() if err != nil { return err } diff --git a/conn_test.go b/conn_test.go index 1a39ecfc3..ba8e00081 100644 --- a/conn_test.go +++ b/conn_test.go @@ -1784,6 +1784,83 @@ func TestPreProtocolError(t *testing.T) { } } +func TestLargeMessage(t *testing.T) { + type resp struct { + c proto.ResponseCode + r string + } + long := strings.Repeat("Y", 35_000) + tests := []struct { + responses []resp + want string + wantErr string + }{ + { // DataRow can be unlimited length + []resp{ + {proto.RowDescription, "\x00\x01col\x00\x00\x00A\xc6\x00\x01\x00\x00\x00\x19\xff\xff\xff\xff\xff\xff\x00\x00"}, + {proto.DataRow, "\x00\x01\x00\x00\x88\xb8" + long}, + {proto.CommandComplete, "SELECT 1\x00"}, + }, + long, "", + }, + { // ErrorResponse as well (after startup) + []resp{ + {proto.ErrorResponse, "SERROR\x00C58030\x00M" + long + "\x00\x00"}, + }, + "", "pq: " + long + " (58030)", + }, + { // But e.g. Empty Query can't + []resp{ + {proto.EmptyQueryResponse, long}, + {proto.CommandComplete, "SELECT 1\x00"}, + }, + "", `pq: lost synchronization with server: got message type "(I) EmptyQueryResponse", length 35000`, + }, + } + + for _, tt := range tests { + t.Run("", func(t *testing.T) { + t.Parallel() + f := pqtest.NewFake(t, func(f pqtest.Fake, cn net.Conn) { + f.Startup(cn, nil) + for { + code, q, ok := f.ReadMsg(cn) + if !ok { + return + } + switch code { + case proto.Query: + switch q := string(q[:bytes.IndexByte(q, 0)]); { + case q == ";": // Ping() + f.WriteMsg(cn, proto.EmptyQueryResponse, "") + f.WriteMsg(cn, proto.ReadyForQuery, "I") + default: + for _, r := range tt.responses { + f.WriteMsg(cn, r.c, r.r) + } + f.WriteMsg(cn, proto.ReadyForQuery, "I") + } + case proto.Terminate: + cn.Close() + return + } + } + }) + defer f.Close() + + db := pqtest.MustDB(t, f.DSN()) + var have string + err := db.QueryRow(`select t from tbl`).Scan(&have) + if !pqtest.ErrorContains(err, tt.wantErr) { + t.Fatalf("wrong error:\nhave: %s\nwant: %s", err, tt.wantErr) + } + if have != tt.want { + t.Fatal("rows not equal") // Don't output content as this deals with lots of text. + } + }) + } +} + // reading from circularConn yields content[:prefixLen] once, followed by // content[prefixLen:] over and over again. It never returns EOF. type circularConn struct { diff --git a/internal/proto/proto.go b/internal/proto/proto.go index e8b4bc593..5aaa51634 100644 --- a/internal/proto/proto.go +++ b/internal/proto/proto.go @@ -16,9 +16,20 @@ const ( NegotiateGSSCode = (1234 << 16) | 5680 ) +// Constants from fe-protocol3.c + +func ValidLongMessageType(c ResponseCode) bool { + switch c { + case CopyDataResponse, DataRow, ErrorResponse, FunctionCallResponse, + NoticeResponse, NotificationResponse, RowDescription: + return true + } + return false +} + // Constants from fe-connect.c const ( - MaxErrlen = 30_000 // https://github.com/postgres/postgres/blob/c6a10a89f/src/interfaces/libpq/fe-connect.c#L4067 + MaxMsgLen = 30_000 // https://github.com/postgres/postgres/blob/c6a10a89f/src/interfaces/libpq/fe-connect.c#L4067 ) // RequestCode is a request codes sent by the frontend.