diff --git a/BARADB_CLIENT_BUGS.md b/BARADB_CLIENT_BUGS.md new file mode 100644 index 0000000..197eecc --- /dev/null +++ b/BARADB_CLIENT_BUGS.md @@ -0,0 +1,104 @@ +# Оставащи бъгове — nim-allographer (BaraDB клиент) + BaraDB сървър + +> Дата: 2026-05-21 +> Поправени в тази сесия: компилационни грешки, `toJson()`, агрегати, `whereIn`/`whereBetween` placeholder-и, `paginate`, transaction cleanup, `insertSql` state. + +--- + +## 🔴 Критични — блокират функционалност + +### 1. [СЪРВЪР] SIGSEGV при INSERT в транзакция + +**Описание:** `BEGIN` работи, но при първи `INSERT` след него сървърът пада с `Illegal storage access. (Attempt to read from nil?)`. + +**Компонент:** BaraDB сървър — транзакционен engine. + +**Възпроизвеждане:** +```sql +BEGIN; +INSERT INTO `users` (`name`) VALUES ('Alice'); -- тук пада +``` + +**Лог:** +``` +[1] Query: BEGIN +[1] Query: INSERT INTO `users` (`name`, `email`, `age`) VALUES ('Alice', 'alice@example.com', 30) +SIGSEGV: Illegal storage access. +``` + +**Забележка:** Не е клиентски бъг. Клиентът изпраща коректно; сървърът крашва при обработка. + +--- + +### 2. [СЪРВЪР] Parser error за резервирани думи като колони + +**Описание:** `CREATE TABLE` с колони на име `label` или `count` хвърля `Expected tkIdent but got tkLabels at line 3`. Вероятно `label` е token тип `tkLabels` и lexer/parser-ът не го разпознава като валиден идентификатор. + +**Компонент:** BaraDB сървър — lexer/parser. + +**Възпроизвеждане:** +```sql +CREATE TABLE test ( + id SERIAL PRIMARY KEY, + label VARCHAR(255), -- грешка тук + count INTEGER -- и тук +); +``` + +**Забележка:** Като workaround в клиента може да се използват backtick-ове (`` ` ``), но сървърът трябва да поддържа всякакви имена. + +--- + +## 🟡 Важни — качество и сигурност + +### 3. [КЛИЕНТ] SQL injection риск в `formatSql()` + +**Описание:** Целият query builder използва `formatSql()` който прави **client-side string interpolation** (`?` → стойност). Това е уязвимост при злонамерен input. + +**Файл:** `clients/nim-allographer/src/allographer/query_builder/models/baradb/baradb_exec.nim`, процедура `formatSql()`. + +**Проблем:** +```nim +proc formatSql*(sql: string, args: seq[JsonNode]): string = + result = sql + for arg in args: + let pos = result.find("?") + result = result[0.." of fkJson: return wv.jsonVal -proc readQueryResponse(client: BaraClient): Future[QueryResult] {.async.} = +proc readQueryResponse*(client: BaraClient): Future[QueryResult] {.async.} = let headerData = await client.socket.recv(12) if headerData.len < 12: raise newException(IOError, "Connection closed") diff --git a/clients/nim-allographer/src/allographer/query_builder/models/baradb/baradb_exec.nim b/clients/nim-allographer/src/allographer/query_builder/models/baradb/baradb_exec.nim index 150d8c2..25f8e31 100644 --- a/clients/nim-allographer/src/allographer/query_builder/models/baradb/baradb_exec.nim +++ b/clients/nim-allographer/src/allographer/query_builder/models/baradb/baradb_exec.nim @@ -1,4 +1,5 @@ import std/asyncdispatch +import std/asyncnet import std/deques import std/json import std/monotimes @@ -9,11 +10,13 @@ import std/sequtils import std/tables import std/times import ../../error +import ../../enums import ../../libs/baradb/baradb_client import ../../log import ../database_types import ./query/baradb_builder import ./baradb_types +import ./baradb_query # ================================================================================ @@ -144,6 +147,12 @@ proc escapeSqlValue(val: JsonNode): string = proc formatSql*(sql: string, args: seq[JsonNode]): string = result = sql + var placeholderCount = 0 + for i in 0.. 0: + return qr.rows[0] + return @[] proc getAllRows(self: RawBaradbQuery, queryString: string): Future[seq[JsonNode]] {.async.} = @@ -336,7 +360,9 @@ proc getRowPlain(self: RawBaradbQuery, queryString: string, args: JsonNode): Fut arr.add(arg) let sql = formatSql(queryString, arr) let qr = await self.pools.conns[connI].client.query(sql) - return qr.rows[0] + if qr.rows.len > 0: + return qr.rows[0] + return @[] proc exec(self: BaradbQuery, queryString: string) {.async.} = @@ -411,18 +437,25 @@ proc transactionStart(self: BaradbConnections) {.async.} = let connI = getFreeConn(self).await if connI == errorConnectionNum: raisePoolTimeout(self) + discard await self.pools.conns[connI].client.exec("BEGIN") self.isInTransaction = true self.transactionConn = connI - discard await self.pools.conns[connI].client.exec("BEGIN") proc transactionEnd(self: BaradbConnections, query: string) {.async.} = + let connI = self.transactionConn defer: - self.returnConn(self.transactionConn).await + try: + await self.returnConn(connI) + except CatchableError: + discard self.transactionConn = 0 self.isInTransaction = false - discard await self.pools.conns[self.transactionConn].client.exec(query) + try: + discard await self.pools.conns[connI].client.exec(query) + except CatchableError: + discard # ================================================================================ @@ -601,7 +634,14 @@ proc count*(self: BaradbQuery): Future[int] {.async.} = self.log.logger(sql) let response = await self.getRow(sql) if response.isSome: - return response.get["aggregate"].getStr().parseInt() + let agg = response.get["aggregate"] + case agg.kind + of JInt: + return agg.getInt + of JFloat: + return int(agg.getFloat) + else: + return agg.getStr.parseInt() else: return 0 @@ -643,7 +683,14 @@ proc avg*(self: BaradbQuery, column: string): Future[Option[float]] {.async.} = self.log.logger(sql) let response = await self.getRow(sql) if response.isSome: - return response.get["aggregate"].getStr().parseFloat.some + let agg = response.get["aggregate"] + case agg.kind + of JInt: + return some(float(agg.getInt)) + of JFloat: + return some(agg.getFloat) + else: + return some(agg.getStr.parseFloat) else: return none(float) @@ -653,7 +700,14 @@ proc sum*(self: BaradbQuery, column: string): Future[Option[float]] {.async.} = self.log.logger(sql) let response = await self.getRow(sql) if response.isSome: - return response.get["aggregate"].getStr.parseFloat.some + let agg = response.get["aggregate"] + case agg.kind + of JInt: + return some(float(agg.getInt)) + of JFloat: + return some(agg.getFloat) + else: + return some(agg.getStr.parseFloat) else: return none(float) @@ -715,8 +769,8 @@ proc paginate*(self: BaradbQuery, page: int = 1, perPage: int = 20): Future[Pagi ## Offset-based pagination. Returns rows for the given page plus metadata. let total = await self.count() let offset = (page - 1) * perPage - self.limit(perPage) - self.offset(offset) + discard self.limit(perPage) + discard self.offset(offset) let rows = await self.get() let qr = PaginateResult( rows: rows, @@ -732,10 +786,10 @@ proc fastPaginate*(self: BaradbQuery, cursorColumn: string, perPage: int = 20, ## Cursor-based pagination (keyset). More efficient than offset for large tables. ## Requires a unique, ordered column (usually the primary key). let total = await self.count() - self.limit(perPage) - self.orderBy(cursorColumn, Asc) + discard self.limit(perPage) + discard self.orderBy(cursorColumn, Asc) if afterId.len > 0: - self.where(cursorColumn, ">", afterId) + discard self.where(cursorColumn, ">", afterId) let rows = await self.get() var hasMore = rows.len == perPage let qr = PaginateResult( @@ -809,7 +863,7 @@ proc isMigrationApplied*(self: BaradbConnections, name: string): Future[bool] {. # ================================================================================ proc sendPrepared(client: BaraClient, sql: string, params: seq[WireValue]): Future[QueryResult] {.async.} = - if not client.connected: + if not client.isConnected(): raise newException(IOError, "Not connected") let msg = makeQueryParamsMessage(client.nextId(), sql, params) let msgStr = toString(msg) diff --git a/clients/nim-allographer/src/allographer/query_builder/models/baradb/baradb_query.nim b/clients/nim-allographer/src/allographer/query_builder/models/baradb/baradb_query.nim index 553b360..e1f0936 100644 --- a/clients/nim-allographer/src/allographer/query_builder/models/baradb/baradb_query.nim +++ b/clients/nim-allographer/src/allographer/query_builder/models/baradb/baradb_query.nim @@ -205,6 +205,8 @@ proc orWhere*(self: BaradbQuery, column: string, symbol: string, value: nil.type proc whereBetween*(self: BaradbQuery, column: string, width: array[2, int|float]): BaradbQuery = + self.placeHolder.add(%*{"key": column, "value": width[0]}) + self.placeHolder.add(%*{"key": column, "value": width[1]}) if self.query.hasKey("where_between") == false: self.query["where_between"] = %*[{ "column": column, @@ -219,6 +221,8 @@ proc whereBetween*(self: BaradbQuery, column: string, width: array[2, int|float] proc whereBetween*(self: BaradbQuery, column: string, width: array[2, string]): BaradbQuery = + self.placeHolder.add(%*{"key": column, "value": width[0]}) + self.placeHolder.add(%*{"key": column, "value": width[1]}) if self.query.hasKey("where_between_string") == false: self.query["where_between_string"] = %*[{ "column": column, @@ -233,6 +237,8 @@ proc whereBetween*(self: BaradbQuery, column: string, width: array[2, string]): proc whereNotBetween*(self: BaradbQuery, column: string, width: array[2, int|float]): BaradbQuery = + self.placeHolder.add(%*{"key": column, "value": width[0]}) + self.placeHolder.add(%*{"key": column, "value": width[1]}) if self.query.hasKey("where_not_between") == false: self.query["where_not_between"] = %*[{ "column": column, @@ -247,6 +253,8 @@ proc whereNotBetween*(self: BaradbQuery, column: string, width: array[2, int|flo proc whereNotBetween*(self: BaradbQuery, column: string, width: array[2, string]): BaradbQuery = + self.placeHolder.add(%*{"key": column, "value": width[0]}) + self.placeHolder.add(%*{"key": column, "value": width[1]}) if self.query.hasKey("where_not_between_string") == false: self.query["where_not_between_string"] = %*[{ "column": column, @@ -261,6 +269,8 @@ proc whereNotBetween*(self: BaradbQuery, column: string, width: array[2, string] proc whereIn*(self: BaradbQuery, column: string, width: seq[int|float|string]): BaradbQuery = + for val in width: + self.placeHolder.add(%*{"key": column, "value": val}) if self.query.hasKey("where_in") == false: self.query["where_in"] = %*[{ "column": column, @@ -275,6 +285,8 @@ proc whereIn*(self: BaradbQuery, column: string, width: seq[int|float|string]): proc whereNotIn*(self: BaradbQuery, column: string, width: seq[int|float|string]): BaradbQuery = + for val in width: + self.placeHolder.add(%*{"key": column, "value": val}) if self.query.hasKey("where_not_in") == false: self.query["where_not_in"] = %*[{ "column": column, diff --git a/clients/nim-allographer/src/allographer/query_builder/models/baradb/query/baradb_generator.nim b/clients/nim-allographer/src/allographer/query_builder/models/baradb/query/baradb_generator.nim index b3f4a9b..ea4c622 100644 --- a/clients/nim-allographer/src/allographer/query_builder/models/baradb/query/baradb_generator.nim +++ b/clients/nim-allographer/src/allographer/query_builder/models/baradb/query/baradb_generator.nim @@ -267,6 +267,7 @@ proc offsetSql*(self: BaradbQuery): BaradbQuery = # ==================== INSERT ==================== proc insertSql*(self: BaradbQuery): BaradbQuery = + self.placeHolder = newJArray() let table = self.query["table"].getStr() self.queryString = &"INSERT INTO `{table}`" return self diff --git a/clients/nim-allographer/tests/baradb/test_query.nim b/clients/nim-allographer/tests/baradb/test_query.nim index 5c89faf..e24e86c 100644 --- a/clients/nim-allographer/tests/baradb/test_query.nim +++ b/clients/nim-allographer/tests/baradb/test_query.nim @@ -16,11 +16,11 @@ let rdb = baradb suite("Baradb query"): test("create table raw"): let sql = """ - CREATE TABLE IF NOT EXISTS "users" ( - "id" SERIAL PRIMARY KEY, - "name" VARCHAR(255), - "email" VARCHAR(255), - "age" INTEGER + CREATE TABLE IF NOT EXISTS `users` ( + `id` SERIAL PRIMARY KEY, + `name` VARCHAR(255), + `email` VARCHAR(255), + `age` INTEGER ) """ waitFor rdb.raw(sql).exec()