fix reviews

Author: mertcanaltin

doc/api/sqlite.md

@@ -157,19 +157,31 @@ changes:
     **Default:** `false`.
   * `limits` {Object} Configuration for various SQLite limits. These limits
     can be used to prevent excessive resource consumption when handling
-    potentially malicious input. See [Run-Time Limits][] in the SQLite
-    documentation for details. The following properties are supported:
+    potentially malicious input. See [Run-Time Limits][] and [Limit Constants][]
+    in the SQLite documentation for details. The following properties are
+    supported:
     * `length` {number} Maximum length of a string or BLOB.
+      **Default:** `1000000000`.
     * `sqlLength` {number} Maximum length of an SQL statement.
+      **Default:** `1000000000`.
     * `column` {number} Maximum number of columns.
+      **Default:** `2000`.
     * `exprDepth` {number} Maximum depth of expression tree.
+      **Default:** `1000`.
     * `compoundSelect` {number} Maximum number of terms in compound SELECT.
+      **Default:** `500`.
     * `vdbeOp` {number} Maximum number of VDBE instructions.
+      **Default:** `250000000`.
     * `functionArg` {number} Maximum number of function arguments.
+      **Default:** `1000`.
     * `attach` {number} Maximum number of attached databases.
+      **Default:** `10`.
     * `likePatternLength` {number} Maximum length of LIKE pattern.
+      **Default:** `50000`.
     * `variableNumber` {number} Maximum number of SQL variables.
+      **Default:** `32766`.
     * `triggerDepth` {number} Maximum trigger recursion depth.
+      **Default:** `1000`.
 
 Constructs a new `DatabaseSync` instance.
 
@@ -1490,11 +1502,12 @@ callback function to indicate what type of operation is being authorized.
 [Changesets and Patchsets]: https://www.sqlite.org/sessionintro.html#changesets_and_patchsets
 [Constants Passed To The Conflict Handler]: https://www.sqlite.org/session/c_changeset_conflict.html
 [Constants Returned From The Conflict Handler]: https://www.sqlite.org/session/c_changeset_abort.html
+[Limit Constants]: https://www.sqlite.org/c3ref/c_limit_attached.html
+[Run-Time Limits]: https://www.sqlite.org/c3ref/limit.html
 [SQL injection]: https://en.wikipedia.org/wiki/SQL_injection
 [Type conversion between JavaScript and SQLite]: #type-conversion-between-javascript-and-sqlite
 [`ATTACH DATABASE`]: https://www.sqlite.org/lang_attach.html
 [`PRAGMA foreign_keys`]: https://www.sqlite.org/pragma.html#pragma_foreign_keys
-[Run-Time Limits]: https://www.sqlite.org/c3ref/limit.html
 [`SQLITE_DBCONFIG_DEFENSIVE`]: https://www.sqlite.org/c3ref/c_dbconfig_defensive.html#sqlitedbconfigdefensive
 [`SQLITE_DETERMINISTIC`]: https://www.sqlite.org/c3ref/c_deterministic.html
 [`SQLITE_DIRECTONLY`]: https://www.sqlite.org/c3ref/c_deterministic.html

src/node_sqlite.cc

@@ -12,6 +12,7 @@
 #include "threadpoolwork-inl.h"
 #include "util-inl.h"
 
+#include <array>
 #include <cinttypes>
 
 namespace node {
@@ -144,7 +145,7 @@ struct LimitInfo {
   int sqlite_limit_id;
 };
 
-static constexpr LimitInfo kLimitMapping[] = {
+static constexpr std::array<LimitInfo, 11> kLimitMapping = {{
     {"length", SQLITE_LIMIT_LENGTH},
     {"sqlLength", SQLITE_LIMIT_SQL_LENGTH},
     {"column", SQLITE_LIMIT_COLUMN},
@@ -156,13 +157,11 @@ static constexpr LimitInfo kLimitMapping[] = {
     {"likePatternLength", SQLITE_LIMIT_LIKE_PATTERN_LENGTH},
     {"variableNumber", SQLITE_LIMIT_VARIABLE_NUMBER},
     {"triggerDepth", SQLITE_LIMIT_TRIGGER_DEPTH},
-};
-
-static constexpr size_t kLimitMappingCount = arraysize(kLimitMapping);
+}};
 
 // Helper function to find limit ID from JS property name
 static int GetLimitIdFromName(const std::string& name) {
-  for (size_t i = 0; i < kLimitMappingCount; ++i) {
+  for (size_t i = 0; i < kLimitMapping.size(); ++i) {
     if (name == kLimitMapping[i].js_name) {
       return kLimitMapping[i].sqlite_limit_id;
     }
@@ -835,7 +834,7 @@ void DatabaseSyncLimits::LimitsEnumerator(
   Isolate* isolate = info.GetIsolate();
   LocalVector<Value> names(isolate);
 
-  for (size_t i = 0; i < kLimitMappingCount; ++i) {
+  for (size_t i = 0; i < kLimitMapping.size(); ++i) {
     Local<String> name;
     if (String::NewFromUtf8(isolate, kLimitMapping[i].js_name).ToLocal(&name)) {
       names.push_back(name);
@@ -1301,7 +1300,7 @@ void DatabaseSync::New(const FunctionCallbackInfo<Value>& args) {
       Local<Object> limits_obj = limits_v.As<Object>();
 
       // Iterate through known limit names and extract values
-      for (size_t i = 0; i < kLimitMappingCount; ++i) {
+      for (size_t i = 0; i < kLimitMapping.size(); ++i) {
         Local<String> key;
         if (!String::NewFromUtf8(env->isolate(), kLimitMapping[i].js_name)
                  .ToLocal(&key)) {

test/parallel/test-sqlite-limits.js

@@ -5,6 +5,23 @@ const { DatabaseSync } = require('node:sqlite');
 const { suite, test } = require('node:test');
 
 suite('DatabaseSync limits', () => {
+  test('default limits match expected SQLite defaults', (t) => {
+    const db = new DatabaseSync(':memory:');
+    t.assert.deepStrictEqual({ ...db.limits }, {
+      length: 1000000000,
+      sqlLength: 1000000000,
+      column: 2000,
+      exprDepth: 1000,
+      compoundSelect: 500,
+      vdbeOp: 250000000,
+      functionArg: 1000,
+      attach: 10,
+      likePatternLength: 50000,
+      variableNumber: 32766,
+      triggerDepth: 1000,
+    });
+  });
+
   test('constructor accepts limits option', (t) => {
     const db = new DatabaseSync(':memory:', {
       limits: {
@@ -33,7 +50,6 @@ suite('DatabaseSync limits', () => {
     t.assert.strictEqual(db.limits.likePatternLength, 1000);
     t.assert.strictEqual(db.limits.variableNumber, 100);
     t.assert.strictEqual(db.limits.triggerDepth, 5);
-    db.close();
   });
 
   test('getter returns current limit value', (t) => {
@@ -42,7 +58,6 @@ suite('DatabaseSync limits', () => {
     t.assert.ok(db.limits.length > 0);
     t.assert.strictEqual(typeof db.limits.sqlLength, 'number');
     t.assert.ok(db.limits.sqlLength > 0);
-    db.close();
   });
 
   test('setter modifies limit value', (t) => {
@@ -56,8 +71,6 @@ suite('DatabaseSync limits', () => {
 
     db.limits.column = 50;
     t.assert.strictEqual(db.limits.column, 50);
-
-    db.close();
   });
 
   test('throws on invalid argument type', (t) => {
@@ -68,7 +81,6 @@ suite('DatabaseSync limits', () => {
       name: 'TypeError',
       message: /Limit value must be an integer/,
     });
-    db.close();
   });
 
   test('throws on negative value', (t) => {
@@ -79,7 +91,6 @@ suite('DatabaseSync limits', () => {
       name: 'RangeError',
       message: /Limit value must be non-negative/,
     });
-    db.close();
   });
 
   test('throws on getter access after close', (t) => {
@@ -118,7 +129,6 @@ suite('DatabaseSync limits', () => {
     t.assert.ok(keys.includes('likePatternLength'));
     t.assert.ok(keys.includes('variableNumber'));
     t.assert.ok(keys.includes('triggerDepth'));
-    db.close();
   });
 
   test('throws on invalid limits option type', (t) => {
@@ -156,6 +166,21 @@ suite('DatabaseSync limits', () => {
     });
     t.assert.strictEqual(db.limits.length, 100000);
     t.assert.strictEqual(typeof db.limits.sqlLength, 'number');
-    db.close();
+  });
+
+  test('throws when exceeding column limit', (t) => {
+    const db = new DatabaseSync(':memory:', {
+      limits: {
+        column: 10,
+      }
+    });
+
+    db.exec('CREATE TABLE t1 (c1, c2, c3, c4, c5, c6, c7, c8, c9, c10)');
+
+    t.assert.throws(() => {
+      db.exec('CREATE TABLE t2 (c1, c2, c3, c4, c5, c6, c7, c8, c9, c10, c11)');
+    }, {
+      message: /too many columns/,
+    });
   });
 });