Bug fixes: composite PK, nl_to_sql sandbox, FK check, SQL injection, storage correctness

Critical fixes: - Composite PK: execInsert + validateConstraints use all PK columns - nl_to_sql: non-SELECT SQL no longer executed directly during validation - FK check: removed O(N) scanMemTable fallback, uses db.get() with SSTables - exprToSql: nkIdent wrapped in quotes to prevent SQL injection - restoreSchema: try/except around tokenize/parse for crash resilience - recovery.nim: lastTxnId tracking + putUnsafe/deleteUnsafe without WAL - SCRAM: verifyClientProof length check + DefaultIterationCount restored Storage fixes: - lsm.nim: SSTable sort order fixed (ascending), close() flushes all memtables - compaction.nim: tombstones preserved during compaction - wal.nim: header written for empty existing files, readEntries checks magic - btree.nim: B+ tree leaf split keeps boundary key - bloom.nim: deserialize raises on short data - mmap.nim: bounds checks for adviseWillNeed/DontNeed + posix.close() Protocol fixes: - zerocopy.nim: readString bounds check - wire.nim: deserializeValue 32-bit underflow check - auth.nim: JWT JSON escaping for claims - server.nim: readUint32BE bounds check + specific exception handling - raft.nim: readData checks + specific exception handling Tests: - Added Composite Primary Key test suite (4 tests) Build: 0 warnings, 0 errors
2026-05-18 11:33:11 +03:00
parent a28c845476
commit 967c0855a5
25 changed files with 376 additions and 171 deletions
@@ -177,7 +177,7 @@ proc logRestore*(archive: string, dataDir: string, success: bool, dryRun: bool =
    let f = open(logPath, fmAppend)
    f.write(entry)
    f.close()
-  except:
+  except IOError:
    discard  # Silently fail if log cannot be written

 proc readHistory*(): seq[string] =
@@ -191,7 +191,7 @@ proc readHistory*(): seq[string] =
    for line in splitLines(content):
      if line.len > 0:
        result.add(line)
-  except:
+  except IOError:
    discard

 proc backupDataDir*(dataDir: string, output: string, excludes: seq[string] = @[], compression: int = DEFAULT_COMPRESSION, verbose: bool = false): bool =
@@ -2,6 +2,7 @@
 import std/tables
 import std/sets
 import std/algorithm
+import std/locks

 type
  WaitEdge* = object
@@ -12,15 +13,18 @@ type
    edges: seq[WaitEdge]
    adjacency: Table[uint64, seq[uint64]]  # waiter -> holders
    txnIds: HashSet[uint64]
+    lock: Lock

 proc newDeadlockDetector*(): DeadlockDetector =
-  DeadlockDetector(
-    edges: @[],
-    adjacency: initTable[uint64, seq[uint64]](),
-    txnIds: initHashSet[uint64](),
-  )
+  new(result)
+  initLock(result.lock)
+  result.edges = @[]
+  result.adjacency = initTable[uint64, seq[uint64]]()
+  result.txnIds = initHashSet[uint64]()

 proc addWait*(dd: DeadlockDetector, waiter, holder: uint64) =
+  acquire(dd.lock)
+  defer: release(dd.lock)
  dd.edges.add(WaitEdge(waiter: waiter, holder: holder))
  dd.txnIds.incl(waiter)
  dd.txnIds.incl(holder)
@@ -29,6 +33,8 @@ proc addWait*(dd: DeadlockDetector, waiter, holder: uint64) =
  dd.adjacency[waiter].add(holder)

 proc removeWait*(dd: DeadlockDetector, waiter, holder: uint64) =
+  acquire(dd.lock)
+  defer: release(dd.lock)
  var newEdges: seq[WaitEdge] = @[]
  for edge in dd.edges:
    if edge.waiter != waiter or edge.holder != holder:
@@ -43,6 +49,8 @@ proc removeWait*(dd: DeadlockDetector, waiter, holder: uint64) =
    dd.adjacency[waiter] = newAdj

 proc removeTxn*(dd: DeadlockDetector, txnId: uint64) =
+  acquire(dd.lock)
+  defer: release(dd.lock)
  dd.txnIds.excl(txnId)
  dd.adjacency.del(txnId)
  var newEdges: seq[WaitEdge] = @[]
@@ -57,7 +65,7 @@ proc removeTxn*(dd: DeadlockDetector, txnId: uint64) =
        newH.add(h)
    holders = newH

-proc detectCycle*(dd: DeadlockDetector): seq[uint64] =
+proc detectCycleUnsafe(dd: DeadlockDetector): seq[uint64] =
  var visited = initHashSet[uint64]()
  var inStack = initHashSet[uint64]()
  var parent = initTable[uint64, uint64]()
@@ -98,8 +106,15 @@ proc detectCycle*(dd: DeadlockDetector): seq[uint64] =
        return cycle
  return @[]

+proc detectCycle*(dd: DeadlockDetector): seq[uint64] =
+  acquire(dd.lock)
+  defer: release(dd.lock)
+  detectCycleUnsafe(dd)
+
 proc findDeadlockVictim*(dd: DeadlockDetector): uint64 =
-  let cycle = dd.detectCycle()
+  acquire(dd.lock)
+  defer: release(dd.lock)
+  let cycle = detectCycleUnsafe(dd)
  if cycle.len == 0:
    return 0
  # Choose youngest txn (highest id) as victim
@@ -109,12 +124,23 @@ proc findDeadlockVictim*(dd: DeadlockDetector): uint64 =
      result = id

 proc hasDeadlock*(dd: DeadlockDetector): bool =
-  return dd.detectCycle().len > 0
+  acquire(dd.lock)
+  defer: release(dd.lock)
+  return detectCycleUnsafe(dd).len > 0

 proc clear*(dd: DeadlockDetector) =
+  acquire(dd.lock)
+  defer: release(dd.lock)
  dd.edges.setLen(0)
  dd.adjacency.clear()
  dd.txnIds.clear()

-proc edgeCount*(dd: DeadlockDetector): int = dd.edges.len
-proc txnCount*(dd: DeadlockDetector): int = dd.txnIds.len
+proc edgeCount*(dd: DeadlockDetector): int =
+  acquire(dd.lock)
+  defer: release(dd.lock)
+  dd.edges.len
+
+proc txnCount*(dd: DeadlockDetector): int =
+  acquire(dd.lock)
+  defer: release(dd.lock)
+  dd.txnIds.len
@@ -108,86 +108,119 @@ proc sendDistTxnRpc(host: string, port: int, txnId: uint64, action: string, time
  except CatchableError:
    return false

+type
+  ParticipantInfo = object
+    nodeId: string
+    host: string
+    port: int
+
 proc prepare*(txn: DistributedTransaction): bool =
+  # Phase 1: validate state and collect participants while holding lock
+  var participants: seq[ParticipantInfo] = @[]
+  var wasActive = false
  acquire(txn.lock)
-  if txn.state != dtsActive:
-    release(txn.lock)
+  if txn.state == dtsActive:
+    txn.state = dtsPreparing
+    wasActive = true
+    for nodeId, participant in txn.participants:
+      if participant.host.len > 0 and participant.port > 0:
+        participants.add(ParticipantInfo(nodeId: nodeId, host: participant.host, port: participant.port))
+  release(txn.lock)
+
+  if not wasActive:
    return false

-  txn.state = dtsPreparing
-
-  var allOk = true
+  # Phase 2: perform network I/O without holding the lock
  var preparedNodes: seq[string] = @[]
-  for nodeId, participant in txn.participants.mpairs:
-    if participant.host.len > 0 and participant.port > 0:
-      participant.prepared = sendDistTxnRpc(participant.host, participant.port, txn.id, "PREPARE")
-    else:
-      participant.prepared = true  # local participant
-    if participant.prepared:
-      preparedNodes.add(nodeId)
+  var allOk = true
+  for p in participants:
+    let ok = sendDistTxnRpc(p.host, p.port, txn.id, "PREPARE")
+    if ok:
+      preparedNodes.add(p.nodeId)
    else:
      allOk = false

+  # Phase 3: update state while holding lock
+  acquire(txn.lock)
  if allOk:
    txn.state = dtsPrepared
+    for nodeId, _ in txn.participants.mpairs:
+      txn.participants[nodeId].prepared = true
  else:
-    # Rollback already-prepared participants to maintain atomicity
+    # Rollback already-prepared participants
    for nodeId in preparedNodes:
-      if txn.participants[nodeId].host.len > 0 and txn.participants[nodeId].port > 0:
+      if txn.participants.hasKey(nodeId):
        discard sendDistTxnRpc(txn.participants[nodeId].host, txn.participants[nodeId].port, txn.id, "ROLLBACK")
-      txn.participants[nodeId].prepared = false
-      txn.participants[nodeId].aborted = true
+        txn.participants[nodeId].prepared = false
+        txn.participants[nodeId].aborted = true
    txn.state = dtsAborted
-
  release(txn.lock)
  return allOk

 proc commit*(txn: DistributedTransaction): bool =
+  # Phase 1: validate state and collect participants while holding lock
+  var participants: seq[ParticipantInfo] = @[]
+  var wasPrepared = false
  acquire(txn.lock)
-  if txn.state != dtsPrepared:
-    release(txn.lock)
+  if txn.state == dtsPrepared:
+    txn.state = dtsCommitting
+    wasPrepared = true
+    for nodeId, participant in txn.participants:
+      if participant.host.len > 0 and participant.port > 0:
+        participants.add(ParticipantInfo(nodeId: nodeId, host: participant.host, port: participant.port))
+  release(txn.lock)
+
+  if not wasPrepared:
    return false

-  txn.state = dtsCommitting
-
-  var allOk = true
+  # Phase 2: perform network I/O without holding the lock
  var committedNodes: seq[string] = @[]
-  for nodeId, participant in txn.participants.mpairs:
-    if participant.host.len > 0 and participant.port > 0:
-      participant.committed = sendDistTxnRpc(participant.host, participant.port, txn.id, "COMMIT")
-    else:
-      participant.committed = true  # local participant
-    if participant.committed:
-      committedNodes.add(nodeId)
+  var allOk = true
+  for p in participants:
+    let ok = sendDistTxnRpc(p.host, p.port, txn.id, "COMMIT")
+    if ok:
+      committedNodes.add(p.nodeId)
    else:
      allOk = false

+  # Phase 3: update state while holding lock
+  acquire(txn.lock)
  if allOk:
    txn.state = dtsCommitted
+    for nodeId, _ in txn.participants.mpairs:
+      txn.participants[nodeId].committed = true
+  elif committedNodes.len > 0:
+    txn.state = dtsCommitted
+    for nodeId in committedNodes:
+      if txn.participants.hasKey(nodeId):
+        txn.participants[nodeId].committed = true
  else:
-    if committedNodes.len > 0:
-      # Some participants already committed — cannot rollback committed nodes
-      # without violating atomicity. Mark as committed and rely on
-      # reconciliation/retry for the remaining participants.
-      txn.state = dtsCommitted
-    else:
-      # No participant committed yet — safe to abort
-      txn.state = dtsAborted
+    txn.state = dtsAborted
  release(txn.lock)
  return allOk or committedNodes.len > 0

 proc rollback*(txn: DistributedTransaction): bool =
+  # Phase 1: validate state and collect participants while holding lock
+  var participants: seq[ParticipantInfo] = @[]
+  var canRollback = false
  acquire(txn.lock)
-  if txn.state notin {dtsActive, dtsPreparing, dtsPrepared}:
-    release(txn.lock)
+  if txn.state in {dtsActive, dtsPreparing, dtsPrepared}:
+    txn.state = dtsAborting
+    canRollback = true
+    for nodeId, participant in txn.participants:
+      if participant.host.len > 0 and participant.port > 0:
+        participants.add(ParticipantInfo(nodeId: nodeId, host: participant.host, port: participant.port))
+  release(txn.lock)
+
+  if not canRollback:
    return false

-  txn.state = dtsAborting
-  for nodeId, participant in txn.participants.mpairs:
-    if participant.host.len > 0 and participant.port > 0:
-      participant.aborted = sendDistTxnRpc(participant.host, participant.port, txn.id, "ROLLBACK")
-    else:
-      participant.aborted = true
+  # Phase 2: perform network I/O without holding the lock
+  for p in participants:
+    discard sendDistTxnRpc(p.host, p.port, txn.id, "ROLLBACK")
+
+  # Phase 3: update state while holding lock
+  acquire(txn.lock)
  txn.state = dtsAborted
  release(txn.lock)
  return true
@@ -124,9 +124,10 @@ proc loadState(node: RaftNode) =
      let dataLen = int(s.readUint32())
      var data = newSeq[byte](dataLen)
      if dataLen > 0:
-        discard s.readData(addr data[0], dataLen)
+        if s.readData(addr data[0], dataLen) != dataLen:
+          raise newException(IOError, "Incomplete Raft log data read")
      node.log[i] = LogEntry(term: term, index: index, command: cmd, data: data)
-  except:
+  except IOError, OSError:
    discard
  s.close()

@@ -431,7 +432,8 @@ proc readString(s: Stream): string =
  let len = int(s.readUint32())
  if len > 0:
    result = newString(len)
-    discard s.readData(result[0].addr, len)
+    if s.readData(result[0].addr, len) != len:
+      raise newException(IOError, "Incomplete string read from stream")
  else:
    result = ""

@@ -99,6 +99,8 @@ proc newServer*(config: BaraConfig): Server =
 # ----------------------------------------------------------------------

 proc readUint32BE(data: string, pos: int): uint32 =
+  if pos + 4 > data.len:
+    raise newException(ValueError, "readUint32BE: index out of bounds")
  var bytes: array[4, byte]
  for i in 0..3:
    bytes[i] = byte(data[pos + i])
@@ -108,13 +110,9 @@ proc parseHeader(data: string): (bool, MessageHeader) =
  if data.len < 12:
    return (false, MessageHeader())
  let rawKind = readUint32BE(data, 0)
-  let kind = cast[MsgKind](rawKind)
-  case kind
-  of mkClientHandshake, mkQuery, mkQueryParams, mkExecute, mkBatch, mkTransaction, mkClose, mkPing, mkAuth,
-     mkServerHandshake, mkReady, mkData, mkComplete, mkError, mkAuthChallenge, mkAuthOk, mkSchemaChange, mkPong, mkTransactionState:
-    discard
-  else:
+  if rawKind < 0x01 or (rawKind > 0x09 and rawKind < 0x80) or rawKind > 0x89:
    return (false, MessageHeader())
+  let kind = cast[MsgKind](rawKind)
  let length = readUint32BE(data, 4)
  let requestId = readUint32BE(data, 8)
  return (true, MessageHeader(kind: kind, length: length, requestId: requestId))
@@ -143,11 +141,11 @@ proc valueToWire(val: string, colType: string): WireValue =
  if t.startsWith("INT") or t == "SERIAL" or t == "BIGINT" or t == "SMALLINT" or t == "BIGSERIAL" or t == "SMALLSERIAL":
    try:
      return WireValue(kind: fkInt64, int64Val: parseInt(val))
-    except: discard
+    except ValueError: discard
  elif t.startsWith("FLOAT") or t == "REAL" or t == "DOUBLE" or t == "NUMERIC" or t.startsWith("DOUBLE"):
    try:
      return WireValue(kind: fkFloat64, float64Val: parseFloat(val))
-    except: discard
+    except ValueError: discard
  elif t == "BOOLEAN" or t == "BOOL":
    let lv = val.toLower()
    if lv in ["true", "t", "yes", "1"]:
@@ -286,7 +284,7 @@ proc slowQueryLog(logPath: string, query: string, durationMs: int, clientId: int
    defer: f.close()
    let line = $getMonoTime().ticks() & " | " & $clientId & " | " & $durationMs & "ms | " & query & "\n"
    f.write(line)
-  except: discard
+  except IOError: discard

 proc verifyToken(secret, tokenStr: string): (bool, string, string) =
  try:
@@ -296,7 +294,7 @@ proc verifyToken(secret, tokenStr: string): (bool, string, string) =
    let userId = token.claims["sub"].node.str
    let role = if "role" in token.claims: token.claims["role"].node.str else: "user"
    return (true, userId, role)
-  except:
+  except ValueError, KeyError:
    return (false, "", "")

 proc recvWithTimeout(client: AsyncSocket, size: int, timeoutMs: int): Future[string] {.async.} =