fix tests

Author: aheev

.github/workflows/ci.yml

@@ -109,6 +109,8 @@ jobs:
       - name: Build Node.js native module (Windows)
         if: ${{ matrix.platform == 'win32' }}
         shell: cmd
+        env:
+          EXTRA_CMAKE_FLAGS: -DLBUG_NODEJS_ENABLE_TEST_EXPORTS=ON
         run: |
           powershell.exe -Command "Add-MpPreference -ExclusionPath '${{ github.workspace }}'"
           call "C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Auxiliary\Build\vcvars64.bat"
@@ -124,6 +126,7 @@ jobs:
           CMAKE_OSX_ARCHITECTURES: ${{ matrix.mac_env && matrix.arch || '' }}
           CMAKE_C_COMPILER_LAUNCHER: ccache
           CMAKE_CXX_COMPILER_LAUNCHER: ccache
+          EXTRA_CMAKE_FLAGS: -DLBUG_NODEJS_ENABLE_TEST_EXPORTS=ON
 
       - name: Run Node.js tests
         working-directory: tools/nodejs_api

CMakeLists.txt

@@ -104,7 +104,16 @@ get_filename_component(NODE_ADDON_API_INCLUDE_PATH ./node_modules/node-addon-api
 include_directories(${CMAKE_JS_INC})
 include_directories(${NODE_ADDON_API_INCLUDE_PATH})
 
-file(GLOB CPP_SOURCE_FILES ./src_cpp/*)
+option(LBUG_NODEJS_ENABLE_TEST_EXPORTS
+    "Export test-only native helpers (e.g. createArrowCSRTestData). Enable for dev/test builds only; never ship in production."
+    OFF)
+
+file(GLOB CPP_SOURCE_FILES ./src_cpp/*.cpp)
+
+if(NOT LBUG_NODEJS_ENABLE_TEST_EXPORTS)
+    list(REMOVE_ITEM CPP_SOURCE_FILES "${CMAKE_CURRENT_SOURCE_DIR}/src_cpp/node_arrow_test_utils.cpp")
+endif()
+
 file(GLOB JS_SOURCE_FILES 
     ./src_js/*.js
     ./src_js/*.mjs
@@ -178,3 +187,7 @@ target_link_libraries(lbugjs PRIVATE lbug ${NODEJS_LBUG_LINK_DEPS} ${CMAKE_JS_LI
 if(LBUG_NODEJS_EXTRA_LINK_LIBS)
   target_link_libraries(lbugjs PRIVATE ${LBUG_NODEJS_EXTRA_LINK_LIBS})
 endif()
+if(LBUG_NODEJS_ENABLE_TEST_EXPORTS)
+  target_compile_definitions(lbugjs PRIVATE LBUG_ENABLE_TEST_EXPORTS)
+  message(STATUS "lbugjs: test-only exports enabled (LBUG_NODEJS_ENABLE_TEST_EXPORTS=ON)")
+endif()

src_cpp/main.cpp

@@ -1,15 +1,20 @@
-#include "include/node_arrow_test_utils.h"
 #include "include/node_connection.h"
 #include "include/node_database.h"
 #include "include/node_query_result.h"
 #include <napi.h>
 
+#ifdef LBUG_ENABLE_TEST_EXPORTS
+#include "include/node_arrow_test_utils.h"
+#endif
+
 Napi::Object InitAll(Napi::Env env, Napi::Object exports) {
     NodeConnection::Init(env, exports);
     NodeDatabase::Init(env, exports);
     NodePreparedStatement::Init(env, exports);
     NodeQueryResult::Init(env, exports);
+#ifdef LBUG_ENABLE_TEST_EXPORTS
     exports.Set("createArrowCSRTestData", Napi::Function::New(env, CreateArrowCSRTestData));
+#endif
     return exports;
 }
 

src_cpp/node_connection.cpp

@@ -10,6 +10,25 @@
 
 namespace {
 
+// Maximum number of Arrow arrays accepted in a single batch call.
+// Guards against integer-wrap DoS when a raw pointer + count is used.
+static constexpr int64_t kMaxArrowBatchCount = 65536;
+
+// Reads a batch-count argument, throwing if it is not a number, not a positive
+// integer, or exceeds kMaxArrowBatchCount.
+static uint64_t ValidateArrayCount(const Napi::Value& value, const char* name) {
+    if (!value.IsNumber()) {
+        throw std::runtime_error(std::string(name) + " must be a number.");
+    }
+    auto count = value.As<Napi::Number>().Int64Value();
+    if (count <= 0 || count > kMaxArrowBatchCount) {
+        throw std::runtime_error(std::string(name) +
+                                 " must be a positive integer no greater than " +
+                                 std::to_string(kMaxArrowBatchCount) + ".");
+    }
+    return static_cast<uint64_t>(count);
+}
+
 template<typename T>
 T* GetPointerArgument(const Napi::Value& value, const char* name) {
     if (value.IsBigInt()) {
@@ -264,7 +283,7 @@ Napi::Value NodeConnection::CreateArrowTableSync(const Napi::CallbackInfo& info)
     Napi::HandleScope scope(env);
     auto tableName = info[0].As<Napi::String>().Utf8Value();
     auto* schema = GetPointerArgument<ArrowSchema>(info[1], "schema");
-    auto numArrays = info[3].As<Napi::Number>().Int64Value();
+    auto numArrays = ValidateArrayCount(info[3], "numArrays");
     auto nodeQueryResult = Napi::ObjectWrap<NodeQueryResult>::Unwrap(info[4].As<Napi::Object>());
     try {
         auto result = lbug::ArrowTableSupport::createViewFromArrowTable(*connection, tableName,
@@ -290,9 +309,9 @@ Napi::Value NodeConnection::CreateArrowRelTableSync(const Napi::CallbackInfo& in
             //           info[6]=indptrSchemaPtr,  info[7]=indptrArraysPtr, info[8]=numIndptrArrays,
             //           info[9]=dstColName,        info[10]=nodeQueryResult
             auto* indicesSchema = GetPointerArgument<ArrowSchema>(info[3], "indicesSchema");
-            auto numIndicesArrays = info[5].As<Napi::Number>().Uint32Value();
+            auto numIndicesArrays = ValidateArrayCount(info[5], "numIndicesArrays");
             auto* indptrSchema = GetPointerArgument<ArrowSchema>(info[6], "indptrSchema");
-            auto numIndptrArrays = info[8].As<Napi::Number>().Uint32Value();
+            auto numIndptrArrays = ValidateArrayCount(info[8], "numIndptrArrays");
             auto dstColName = info[9].As<Napi::String>().Utf8Value();
             auto nodeQueryResult =
                 Napi::ObjectWrap<NodeQueryResult>::Unwrap(info[10].As<Napi::Object>());
@@ -306,7 +325,7 @@ Napi::Value NodeConnection::CreateArrowRelTableSync(const Napi::CallbackInfo& in
             // Flat mode: info[3]=schemaPtr, info[4]=arraysPtr, info[5]=numArrays,
             // info[6]=nodeQueryResult
             auto* schema = GetPointerArgument<ArrowSchema>(info[3], "schema");
-            auto numArrays = info[5].As<Napi::Number>().Uint32Value();
+            auto numArrays = ValidateArrayCount(info[5], "numArrays");
             auto nodeQueryResult =
                 Napi::ObjectWrap<NodeQueryResult>::Unwrap(info[6].As<Napi::Object>());
             auto result = lbug::ArrowTableSupport::createRelTableFromArrowTable(*connection,

src_js/connection.js

@@ -426,6 +426,12 @@ class Connection {
     const connection = this._getConnectionSync();
     const nodeQueryResult = new LbugNative.NodeQueryResult();
     if (indptrSchemaPtr != null) {
+      if (indptrArraysPtr == null) {
+        throw new Error("indptrArraysPtr must be provided in CSR mode.");
+      }
+      if (!Array.isArray(indptrArraysPtr) && (!Number.isSafeInteger(numIndptrArrays) || numIndptrArrays <= 0)) {
+        throw new Error("numIndptrArrays must be a positive integer.");
+      }
       if (typeof dstColName !== "string") {
         throw new Error("dstColName must be a string.");
       }

test/test_arrow_memory_backed_table.js

@@ -6,6 +6,10 @@ const fs = require("fs");
 // Access the native module directly for test helpers
 const LbugNative = require("../build/lbug_native.js");
 
+// createArrowCSRTestData is only compiled in when the addon is built with
+// LBUG_NODEJS_ENABLE_TEST_EXPORTS=ON. Skip the whole suite otherwise.
+const hasTestHelpers = typeof LbugNative.createArrowCSRTestData === "function";
+
 let arrowDb, arrowConn;
 
 before(function () {
@@ -15,12 +19,23 @@ before(function () {
   arrowConn = new lbug.Connection(arrowDb, 2);
 });
 
+after(function () {
+  if (arrowConn) arrowConn.closeSync();
+  if (arrowDb) arrowDb.closeSync();
+});
+
 describe("createArrowRelTableSync CSR", function () {
+  before(function () {
+    if (!hasTestHelpers) {
+      this.skip();
+    }
+  });
   it("should throw when dstColName is not a string in CSR mode", function () {
     const { nodeSchemaPtr, nodeArrayPtr, indicesSchemaPtr, indicesArrayPtr,
       indptrSchemaPtr, indptrArrayPtr } = LbugNative.createArrowCSRTestData("to");
 
-    arrowConn.createArrowTableSync("person_err", nodeSchemaPtr, nodeArrayPtr, 1);
+    const createPersonResult = arrowConn.createArrowTableSync("person_err", nodeSchemaPtr, nodeArrayPtr, 1);
+    createPersonResult.close();
 
     assert.throws(() => {
       arrowConn.createArrowRelTableSync(
@@ -36,20 +51,24 @@ describe("createArrowRelTableSync CSR", function () {
     const { nodeSchemaPtr, nodeArrayPtr, indicesSchemaPtr, indicesArrayPtr,
       indptrSchemaPtr, indptrArrayPtr } = LbugNative.createArrowCSRTestData("to");
 
-    arrowConn.createArrowTableSync("person_default", nodeSchemaPtr, nodeArrayPtr, 1);
+    const createPersonResult = arrowConn.createArrowTableSync("person_default", nodeSchemaPtr, nodeArrayPtr, 1);
+    createPersonResult.close();
 
     // Use default dstColName "to" (omit last optional param)
-    arrowConn.createArrowRelTableSync(
+    const createRelResult = arrowConn.createArrowRelTableSync(
       "knows_default", "person_default", "person_default",
       indicesSchemaPtr, indicesArrayPtr, 1,
       indptrSchemaPtr, indptrArrayPtr, 1
     );
+    createRelResult.close();
 
     // Verify edges: person0→person1 (w=10), person0→person2 (w=20), person1→person2 (w=30)
-    const rows = arrowConn.querySync(
+    const qr = arrowConn.querySync(
       "MATCH (a:person_default)-[r:knows_default]->(b:person_default) " +
       "RETURN a.id, r.weight, b.id ORDER BY a.id, b.id"
-    ).getAllSync();
+    );
+    const rows = qr.getAllSync();
+    qr.close();
 
     assert.deepEqual(
       rows.map(r => [Number(r["a.id"]), Number(r["r.weight"]), Number(r["b.id"])]),
@@ -62,19 +81,23 @@ describe("createArrowRelTableSync CSR", function () {
     const { nodeSchemaPtr, nodeArrayPtr, indicesSchemaPtr, indicesArrayPtr,
       indptrSchemaPtr, indptrArrayPtr } = LbugNative.createArrowCSRTestData(dstColName);
 
-    arrowConn.createArrowTableSync("person_custom", nodeSchemaPtr, nodeArrayPtr, 1);
+    const createPersonResult = arrowConn.createArrowTableSync("person_custom", nodeSchemaPtr, nodeArrayPtr, 1);
+    createPersonResult.close();
 
-    arrowConn.createArrowRelTableSync(
+    const createRelResult = arrowConn.createArrowRelTableSync(
       "knows_custom", "person_custom", "person_custom",
       indicesSchemaPtr, indicesArrayPtr, 1,
       indptrSchemaPtr, indptrArrayPtr, 1,
       dstColName
     );
+    createRelResult.close();
 
-    const rows = arrowConn.querySync(
+    const qr = arrowConn.querySync(
       "MATCH (a:person_custom)-[r:knows_custom]->(b:person_custom) " +
       "RETURN a.id, r.weight, b.id ORDER BY a.id, b.id"
-    ).getAllSync();
+    );
+    const rows = qr.getAllSync();
+    qr.close();
 
     assert.deepEqual(
       rows.map(r => [Number(r["a.id"]), Number(r["r.weight"]), Number(r["b.id"])]),
@@ -86,7 +109,8 @@ describe("createArrowRelTableSync CSR", function () {
     // Fresh data since each call to createArrowTableSync transfers ownership
     const d = LbugNative.createArrowCSRTestData("to");
 
-    arrowConn.createArrowTableSync("person_flat_test", d.nodeSchemaPtr, d.nodeArrayPtr, 1);
+    const createPersonResult = arrowConn.createArrowTableSync("person_flat_test", d.nodeSchemaPtr, d.nodeArrayPtr, 1);
+    createPersonResult.close();
 
     // Flat path requires "from"/"to" columns in the Arrow batch.
     // Passing a batch with only "id" should trigger a Ladybug error (confirming flat path is used).