diff --git a/async_postgres/pg_connection/lifecycle.nim b/async_postgres/pg_connection/lifecycle.nim index 65efbe1..60e84eb 100644 --- a/async_postgres/pg_connection/lifecycle.nim +++ b/async_postgres/pg_connection/lifecycle.nim @@ -587,6 +587,7 @@ proc orderedHosts*(config: ConnConfig): seq[HostEntry] = proc connect*(config: ConnConfig): Future[PgConnection] = ## Connect with multi-host failover, ``targetSessionAttrs``, per-host ``connectTimeout``. ## Per-host failures fold into one ``PgConnectionError``; a ``PgConfigError`` escapes the fold. + ## Single-host ``connectTimeout`` raises ``AsyncTimeoutError`` (not folded). # Local mutable copy: ``validateConnConfig`` may normalize ``connectTimeout``. var config = config proc perform(hosts: seq[HostEntry]): Future[PgConnection] {.async.} = diff --git a/async_postgres/pg_connection/simple_query.nim b/async_postgres/pg_connection/simple_query.nim index 42784db..84e7614 100644 --- a/async_postgres/pg_connection/simple_query.nim +++ b/async_postgres/pg_connection/simple_query.nim @@ -84,6 +84,9 @@ proc checkTxIdle*(conn: PgConnection) = proc quoteIdentifier*(s: string): string = ## Quote a SQL identifier (e.g. table/channel name) with double quotes, escaping embedded quotes. + ## Raises ``ValueError`` for an embedded NUL byte (same as ``quoteLiteral``). + if '\0' in s: + raise newException(ValueError, "SQL identifier contains a NUL byte") "\"" & s.replace("\"", "\"\"") & "\"" proc quoteLiteral*(s: string): string = diff --git a/async_postgres/pg_connection/ssl.nim b/async_postgres/pg_connection/ssl.nim index 4ebad4a..23d2668 100644 --- a/async_postgres/pg_connection/ssl.nim +++ b/async_postgres/pg_connection/ssl.nim @@ -621,6 +621,7 @@ proc establishTls(conn: PgConnection, config: ConnConfig, sslHost: string) {.asy proc negotiateSSL*(conn: PgConnection, config: ConnConfig, sslHost: string) {.async.} = ## Negotiate TLS (SSLRequest or Direct). ``sslHost`` is cert verification name. + ## Raises ``PgConfigError`` when ``sslMode == sslDisable``. # Defensive: connectToHost / perform already validate, but this proc is # exported and may be called directly; the checks are idempotent. # `validateClientCertConfig` also runs at the connect-time chokepoint in @@ -629,6 +630,9 @@ proc negotiateSSL*(conn: PgConnection, config: ConnConfig, sslHost: string) {.as # would silently drop a lone `sslCert` while asyncdispatch errors out. Its # `PgConfigError` is left as is, so a direct caller sees the type `connect` # raises. + if config.sslMode == sslDisable: + raise + newException(PgConfigError, "negotiateSSL requires sslmode other than disable") validateClientCertConfig(config) validateDirectSslCompatible(config) if config.sslMode in {sslVerifyCa, sslVerifyFull} and config.sslRootCert.len == 0: diff --git a/async_postgres/pg_largeobject.nim b/async_postgres/pg_largeobject.nim index e373511..c70ef2f 100644 --- a/async_postgres/pg_largeobject.nim +++ b/async_postgres/pg_largeobject.nim @@ -201,6 +201,8 @@ proc loRead*( ): Future[seq[byte]] {.async.} = ## Read up to ``length`` bytes from the current position. ## Returns the bytes read (may be fewer than ``length`` at EOF). + if length < 0: + raise newException(ValueError, "loRead: length must be non-negative") let qr = await lo.conn.query( "SELECT loread($1, $2)", @[toPgParam(lo.fd), toPgParam(length)], @@ -232,6 +234,9 @@ proc loSeek*( timeout: Duration = ZeroDuration, ): Future[int64] {.async.} = ## Seek to a position. Returns the new absolute position. + if whence != SEEK_SET and whence != SEEK_CUR and whence != SEEK_END: + raise + newException(ValueError, "loSeek: whence must be SEEK_SET, SEEK_CUR, or SEEK_END") let s = await lo.conn.queryValue( "SELECT lo_lseek64($1, $2, $3)", @[toPgParam(lo.fd), toPgParam(offset), toPgParam(whence)], diff --git a/async_postgres/pg_types/accessors.nim b/async_postgres/pg_types/accessors.nim index 763d1bc..9528127 100644 --- a/async_postgres/pg_types/accessors.nim +++ b/async_postgres/pg_types/accessors.nim @@ -54,8 +54,10 @@ proc `[]`*(row: Row, col: int): Option[seq[byte]] = else: some(@(row.data.buf.toOpenArray(off, off + clen - 1))) -converter toRow*(cells: seq[Option[seq[byte]]]): Row = +converter toRow*(cells: seq[Option[seq[byte]]]): Row {.raises: [PgTypeError].} = ## Backward-compatible converter: build a Row from ``seq[Option[seq[byte]]]``. + if cells.len > high(int16): + raise newException(PgTypeError, "toRow: too many columns (" & $cells.len & ")") let rd = RowData( numCols: int16(cells.len), buf: @[], cellIndex: newSeq[int32](cells.len * 2) ) @@ -65,6 +67,15 @@ converter toRow*(cells: seq[Option[seq[byte]]]): Row = rd.cellIndex[i * 2 + 1] = -1'i32 else: let data = cell.get + if data.len > high(int32): + raise newException( + PgTypeError, "toRow: cell value too large (len=" & $data.len & ")" + ) + if rd.buf.len > high(int32) - data.len: + raise newException( + PgTypeError, + "toRow: accumulated cell bytes exceed int32 (len=" & $rd.buf.len & ")", + ) rd.cellIndex[i * 2] = int32(rd.buf.len) rd.cellIndex[i * 2 + 1] = int32(data.len) rd.buf.add(data) diff --git a/async_postgres/pg_types/encoding.nim b/async_postgres/pg_types/encoding.nim index f143b2f..979426a 100644 --- a/async_postgres/pg_types/encoding.nim +++ b/async_postgres/pg_types/encoding.nim @@ -773,8 +773,13 @@ proc hexNibble*(c: char): int = else: -1 -proc decodeHexPair*(s: string, i: int, errCtx: string): byte = +proc decodeHexPair*(s: string, i: int, errCtx: string): byte {.raises: [PgTypeError].} = ## Failures report the position and input length only (see `PgTypeError`). + if i < 0 or i + 1 >= s.len: + raise newException( + PgTypeError, + errCtx & ": hex pair out of range at position " & $i & " (len=" & $s.len & ")", + ) let hi = hexNibble(s[i]) let lo = hexNibble(s[i + 1]) if hi < 0 or lo < 0: @@ -784,14 +789,27 @@ proc decodeHexPair*(s: string, i: int, errCtx: string): byte = ) byte((hi shl 4) or lo) -proc decodeHexPair*(buf: openArray[byte], i: int, errCtx: string): byte = +proc decodeHexPair*( + buf: openArray[byte], i: int, errCtx: string +): byte {.raises: [PgTypeError].} = + ## Failures report the position and input length only (see `PgTypeError`). + if i < 0 or i + 1 >= buf.len: + raise newException( + PgTypeError, + errCtx & ": hex pair out of range at position " & $i & " (len=" & $buf.len & ")", + ) let hi = hexNibble(char(buf[i])) let lo = hexNibble(char(buf[i + 1])) if hi < 0 or lo < 0: - raise newException(PgTypeError, errCtx & ": non-hex character at position " & $i) + raise newException( + PgTypeError, + errCtx & ": non-hex character at position " & $i & " (len=" & $buf.len & ")", + ) byte((hi shl 4) or lo) -proc decodeByteaEscape*(s: openArray[char], errCtx: string): seq[byte] = +proc decodeByteaEscape*( + s: openArray[char], errCtx: string +): seq[byte] {.raises: [PgTypeError].} = ## Decode bytea text in the legacy `bytea_output = escape` format. ## Server output only ever produces `\\` and `\NNN` (3 octal digits); ## other bytes pass through verbatim. @@ -1832,7 +1850,9 @@ proc toPgMoneyArrayNDParam*( buf.writeMoneyAt(pos, v.elements[i].get) PgParam(oid: OidMoneyArray, format: 1, value: some(buf)) -proc coerceBinaryParam*(param: PgParam, serverOid: int32): PgParam = +proc coerceBinaryParam*( + param: PgParam, serverOid: int32 +): PgParam {.raises: [PgTypeError].} = ## Return a copy of `param` whose binary payload matches `serverOid`. ## Text-format parameters (format == 0) and matching OIDs are returned ## unchanged. For binary-format parameters with a type mismatch, safe diff --git a/async_postgres/pg_types/user_types.nim b/async_postgres/pg_types/user_types.nim index 770125e..23258de 100644 --- a/async_postgres/pg_types/user_types.nim +++ b/async_postgres/pg_types/user_types.nim @@ -1,4 +1,4 @@ -import std/[options, macros, strutils, typetraits] +import std/[options, macros, strutils, typetraits, times] import ../pg_protocol import core, decoding, encoding @@ -390,6 +390,13 @@ proc encodeCompositeText*(fields: seq[Option[string]]): string {.raises: [].} = result.add(compositeFieldToText(f.get)) result.add(')') +proc compositeDateTimeToText(dt: DateTime): string = + ## Text form of a composite DateTime field. The UTC offset is mandatory: + ## without it a timestamptz field is reinterpreted in the session TimeZone. + if not dt.isInitialized: + raise newException(PgTypeError, "Uninitialized DateTime in composite field") + dt.utc.format("yyyy-MM-dd HH:mm:ss'.'ffffffzzz") + macro pgComposite*(T: typedesc, oid: int32 = 0'i32): untyped = ## Generate ``toPgParam`` for a Nim object as a PostgreSQL composite type. ## Each field is sent as text inside the composite text format. @@ -403,9 +410,14 @@ macro pgComposite*(T: typedesc, oid: int32 = 0'i32): untyped = for _, val in v.fieldPairs: when typeof(val) is Option: if val.isSome: - fields.add(some($val.get)) + when typeof(val.get) is DateTime: + fields.add(some(compositeDateTimeToText(val.get))) + else: + fields.add(some($val.get)) else: fields.add(none(string)) + elif typeof(val) is DateTime: + fields.add(some(compositeDateTimeToText(val))) else: fields.add(some($val)) PgParam( @@ -432,6 +444,8 @@ proc compositeFieldFromText[T](s: string): T = parsePgBoolText(s) elif T is PgNumeric: parsePgNumeric(s) + elif T is DateTime: + parseTimestampText(s) else: raise newException(PgTypeError, "Unsupported composite field type") @@ -457,7 +471,7 @@ template checkFieldOid(actual: int32, allowed: openArray[int32], typeName: strin template decodeBinaryField(val, buf: untyped, fOid: int32, fOff, fEnd, fLen: int) = when typeof(val) is string: - checkFieldOid(fOid, [OidText, OidVarchar], "string") + checkFieldOid(fOid, LabelBearingOids, "string") val = readString(buf, fOff, fLen) elif typeof(val) is int16: checkFieldOid(fOid, [OidInt2], "int16") @@ -483,6 +497,10 @@ template decodeBinaryField(val, buf: untyped, fOid: int32, fOff, fEnd, fLen: int checkFieldOid(fOid, [OidBool], "bool") checkFieldLen(fLen, 1, "bool") val = buf[fOff] != 0 + elif typeof(val) is DateTime: + checkFieldOid(fOid, [OidTimestamp, OidTimestampTz], "DateTime") + checkFieldLen(fLen, 8, "DateTime") + val = decodeBinaryTimestamp(buf.toOpenArray(fOff, fEnd)) else: val = compositeFieldFromText[typeof(val)](readString(buf, fOff, fLen)) @@ -575,12 +593,16 @@ proc getDomain*[T: distinct](row: Row, col: int): T = T(row.getInt64(col)) elif distinctBase(T) is float64: T(row.getFloat(col)) + elif distinctBase(T) is float32: + T(row.getFloat32(col)) elif distinctBase(T) is bool: T(row.getBool(col)) + elif distinctBase(T) is DateTime: + T(row.getTimestamp(col)) else: {. error: - "Unsupported domain base type: use string, int16, int32, int64, float64, or bool" + "Unsupported domain base type: use string, int16, int32, int64, float32, float64, bool, or DateTime" .} proc getDomainOpt*[T: distinct](row: Row, col: int): Option[T] = diff --git a/tests/test_e2e_misc.nim b/tests/test_e2e_misc.nim index 2265156..8f71bf3 100644 --- a/tests/test_e2e_misc.nim +++ b/tests/test_e2e_misc.nim @@ -186,6 +186,14 @@ suite "E2E: quoteIdentifier": test "identifier with spaces": doAssert quoteIdentifier("my table") == "\"my table\"" + test "NUL byte raises ValueError": + var raised = false + try: + discard quoteIdentifier("a\0b") + except ValueError: + raised = true + doAssert raised, "NUL byte should raise ValueError" + suite "E2E: quoteLiteral": test "simple literal": doAssert quoteLiteral("foo") == "'foo'" diff --git a/tests/test_largeobject_parse.nim b/tests/test_largeobject_parse.nim index d1a01ff..eae07c3 100644 --- a/tests/test_largeobject_parse.nim +++ b/tests/test_largeobject_parse.nim @@ -9,7 +9,7 @@ ## `loCreate`) against a scripted mock server, so the call sites that consume ## these parsers are covered too — not just the private helpers. -import std/[unittest, strutils] +import std/[unittest, strutils, importutils] import ../async_postgres/[async_backend, pg_connection] import ../async_postgres/pg_errors @@ -17,6 +17,8 @@ import ../async_postgres/pg_largeobject {.all.} import mock_pg_server +privateAccess(LargeObject) + proc mockConfig(port: int): ConnConfig = ConnConfig( host: "127.0.0.1", port: port, user: "test", database: "test", sslMode: sslDisable @@ -181,3 +183,14 @@ suite "Large Object parsers: hostile server text via the public API": await closeServer(ms) waitFor t() + +suite "Large Object precondition errors (no server)": + test "loRead rejects negative length with ValueError": + let lo = LargeObject(conn: nil, fd: 1, oid: Oid(1)) + expect ValueError: + discard waitFor lo.loRead(-1) + + test "loSeek rejects invalid whence with ValueError": + let lo = LargeObject(conn: nil, fd: 1, oid: Oid(1)) + expect ValueError: + discard waitFor lo.loSeek(0, whence = 99'i32) diff --git a/tests/test_ssl.nim b/tests/test_ssl.nim index a6d3f55..7029d43 100644 --- a/tests/test_ssl.nim +++ b/tests/test_ssl.nim @@ -1137,6 +1137,23 @@ suite "SSL negotiation - sslAllow": check raised suite "SSL negotiation - sslDisable": + test "negotiateSSL with sslDisable raises PgConfigError": + var raised = false + + proc t() {.async.} = + var conn = PgConnection() + conn.state = csReady + let config = ConnConfig( + host: "127.0.0.1", port: 1, user: "test", database: "test", sslMode: sslDisable + ) + try: + await negotiateSSL(conn, config, "localhost") + except PgConfigError: + raised = true + + waitFor t() + check raised + test "sslDisable sends StartupMessage directly without SSLRequest": var firstMsgVersion: int32 = 0 var connState: PgConnState diff --git a/tests/test_types.nim b/tests/test_types.nim index 0072033..70037ff 100644 --- a/tests/test_types.nim +++ b/tests/test_types.nim @@ -20,6 +20,8 @@ type ProbabilityF = distinct float64 BigCount = distinct int64 IsActive = distinct bool + RatioF32 = distinct float32 + EventAt = distinct DateTime # Test-local shims for the legacy 1-D ``encodeBinaryArray`` and # ``encodeBinaryArrayEmpty`` shapes that were removed when the encoder was @@ -42,6 +44,8 @@ proc `==`(a, b: SmallCount): bool {.borrow.} proc `==`(a, b: PositiveInt): bool {.borrow.} proc `==`(a, b: BigCount): bool {.borrow.} proc `==`(a, b: IsActive): bool {.borrow.} +proc `==`(a, b: RatioF32): bool {.borrow.} +proc `==`(a, b: EventAt): bool {.borrow.} proc `$`(v: UsPostalCode): string {.borrow.} proc `$`(v: SmallCount): string {.borrow.} proc `$`(v: PositiveInt): string {.borrow.} @@ -52,6 +56,8 @@ pgDomain(PositiveInt, int32) pgDomain(ProbabilityF, float64, 90001) pgDomain(BigCount, int64) pgDomain(IsActive, bool) +pgDomain(RatioF32, float32) +pgDomain(EventAt, DateTime) proc toString(data: seq[byte]): string = result = newString(data.len) @@ -1941,6 +1947,13 @@ suite "Row type alias": check row.getStr(0) == "hello" check row.isNull(1) + test "toRow raises PgTypeError for too many columns": + var cells = newSeq[Option[seq[byte]]](int(high(int16)) + 1) + for i in 0 ..< cells.len: + cells[i] = none(seq[byte]) + expect PgTypeError: + discard toRow(cells) + suite "parseAffectedRows": test "UPDATE tag": check parseAffectedRows("UPDATE 3") == 3 @@ -4374,6 +4387,30 @@ suite "coerceBinaryParam": raised = true check raised +suite "decodeHexPair / decodeByteaEscape error contract": + test "decodeHexPair string rejects out-of-range index with PgTypeError": + expect PgTypeError: + discard decodeHexPair("ab", 1, "hex") + expect PgTypeError: + discard decodeHexPair("ab", -1, "hex") + expect PgTypeError: + discard decodeHexPair("", 0, "hex") + + test "decodeHexPair openArray rejects out-of-range index with PgTypeError": + let buf = @[byte('a'), byte('b')] + expect PgTypeError: + discard decodeHexPair(buf, 1, "hex") + expect PgTypeError: + discard decodeHexPair(buf, -1, "hex") + + test "decodeHexPair accepts valid pair": + check decodeHexPair("ff", 0, "hex") == 255'u8 + check decodeHexPair(@[byte('0'), byte('a')], 0, "hex") == 10'u8 + + test "decodeByteaEscape trailing backslash raises PgTypeError": + expect PgTypeError: + discard decodeByteaEscape(['a', '\\'], "bytea") + suite "PgInet": test "$ IPv4": let v = PgInet(address: parseIpAddress("192.168.1.1"), mask: 24) @@ -4971,10 +5008,19 @@ type name: string n: int64 + TimestampRecord = object + label: string + at: DateTime + + NameFieldRecord = object + name: string + pgComposite(PointRecord) pgComposite(PersonRecord, 50000'i32) pgComposite(NullableRecord) pgComposite(WideIntRecord) +pgComposite(TimestampRecord) +pgComposite(NameFieldRecord) suite "Composite text parser": test "parseCompositeText simple": @@ -5466,6 +5512,45 @@ suite "User-defined composite": check abs(pt.x - 3.14) < 1e-10 check abs(pt.y - 2.72) < 1e-10 + test "pgComposite DateTime text round-trip": + let dt = dateTime(2024, mMar, 15, 10, 30, 0, 123456000, utc()) + let p = toPgParam(TimestampRecord(label: "evt", at: dt)) + check p.format == 0'i16 + let encoded = toString(p.value.get) + # The offset must be kept: a zoneless literal would be reinterpreted in the + # session TimeZone when the server-side field is timestamptz. + check "2024-03-15 10:30:00.123456Z" in encoded + let row: Row = @[some(p.value.get)] + let got = getComposite[TimestampRecord](row, 0) + check got.label == "evt" + check got.at == dt + + test "pgComposite rejects an uninitialized DateTime field": + expect PgTypeError: + discard toPgParam(TimestampRecord(label: "evt")) + + test "getComposite DateTime binary format": + let dt = dateTime(2024, mMar, 15, 10, 30, 0, 0, utc()) + let tsBytes = toPgBinaryParam(dt).value.get + let fields_data = @[ + (oid: OidText, data: some(toBytes("evt"))), + (oid: OidTimestamp, data: some(tsBytes)), + ] + let data = encodeBinaryComposite(fields_data) + let fields = @[mkField(50000'i32, 1'i16)] + let row = mkRow(@[some(data)], fields) + let got = getComposite[TimestampRecord](row, 0) + check got.label == "evt" + check got.at == dt + + test "getComposite binary name OID accepted for string field": + let fields_data = @[(oid: OidName, data: some(toBytes("alice")))] + let data = encodeBinaryComposite(fields_data) + let fields = @[mkField(50000'i32, 1'i16)] + let row = mkRow(@[some(data)], fields) + let got = getComposite[NameFieldRecord](row, 0) + check got.name == "alice" + suite "User-defined domain": test "pgDomain generates toPgParam with base type OID": let p = toPgParam(UsPostalCode("12345")) @@ -5525,6 +5610,17 @@ suite "User-defined domain": let rowF: Row = @[some(toBytes("f"))] check getDomain[IsActive](rowF, 0) == IsActive(false) + test "getDomain text format float32": + let row: Row = @[some(toBytes("0.5"))] + check getDomain[RatioF32](row, 0) == RatioF32(0.5'f32) + + test "getDomain text format DateTime": + let row: Row = @[some(toBytes("2024-01-15 10:30:00.000000"))] + let got = getDomain[EventAt](row, 0) + check DateTime(got).year == 2024 + check DateTime(got).month == mJan + check DateTime(got).monthday == 15 + test "getDomain binary format int16": let fields = @[mkField(OidInt2, 1'i16)] let row = mkRow(@[some(@(toBE16(3'i16)))], fields)