From 1de834b0275818dd3758a1220b93c795360e9427 Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Fri, 13 Mar 2026 13:03:04 -0400 Subject: [PATCH 1/2] fix bad behavior in marshal for empty buffers/strings When built with _GLIBCXX_ASSERTIONS (as in Flatpak environments), allMarshal crashes on empty result sets or empty string/blob values due to out-of-bounds vector/string access via operator[]. Fix by using .data() and guarding _writeBytes against zero-length writes. Add tests for empty result sets, empty strings, and empty blobs. Add build:assert script and CI step to catch this class of bug. --- .github/workflows/ci.yml | 4 ++++ package.json | 1 + src/marshal.h | 5 +++-- test/allMarshal.test.js | 29 +++++++++++++++++++++++++++++ test/marshal-test.js | 3 +++ 5 files changed, 40 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 673a1207a..867686089 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -99,6 +99,10 @@ jobs: - name: Package prebuilt binaries run: yarn node-pre-gyp package --target_arch=${{ env.TARGET }} + - name: Rebuild with assertions and re-test + if: contains(matrix.os, 'ubuntu') + run: yarn build:assert && yarn test + - name: Prepare for saving artifact run: | ARTIFACT_NAME=$(echo prebuilt-binaries-${{ matrix.os }}-${{ matrix.target }} | sed 's/[^-a-zA-Z0-9]/_/g') diff --git a/package.json b/package.json index 34002162e..4c94f972f 100644 --- a/package.json +++ b/package.json @@ -72,6 +72,7 @@ "build": "node-pre-gyp build", "build:debug": "node-pre-gyp build --debug", "install": "node-pre-gyp install --fallback-to-build", + "build:assert": "CXXFLAGS=\"${CXXFLAGS:-} -D_GLIBCXX_ASSERTIONS\" node-pre-gyp rebuild --build-from-source", "pretest": "node test/support/createdb.js", "test": "mocha -R spec --timeout 480000", "test:memory": "node test/support/memory_check.js", diff --git a/src/marshal.h b/src/marshal.h index a1a00ae87..5aa52ef69 100644 --- a/src/marshal.h +++ b/src/marshal.h @@ -38,6 +38,7 @@ class Marshaller { } void _writeBytes(const void *bytes, size_t nbytes) { + if (nbytes == 0) return; size_t offset = buffer.size(); buffer.resize(buffer.size() + nbytes); memcpy(&buffer[offset], bytes, nbytes); @@ -57,7 +58,7 @@ class Marshaller { void append(const Marshaller &marshaller) { const std::vector &buf = marshaller.getBuffer(); - _writeBytes(&buf[0], buf.size()); + _writeBytes(buf.data(), buf.size()); } // Marshal the given value depending on its type. @@ -68,7 +69,7 @@ class Marshaller { } void marshalString(const std::string &value) { - marshalString(&value[0], value.size()); + marshalString(value.data(), value.size()); } void marshalString(const char *value, int32_t size) { diff --git a/test/allMarshal.test.js b/test/allMarshal.test.js index 0970a1e86..230a6c4e4 100644 --- a/test/allMarshal.test.js +++ b/test/allMarshal.test.js @@ -60,5 +60,34 @@ describe('Database#allMarshal', function() { } }); + it('should handle empty result set', function(done) { + db.allMarshal("SELECT * FROM foo WHERE 1=0", function(err, result) { + if (err) throw err; + // Should be a dict with column names mapping to empty lists: + // {s[\x00\x00\x00\x00 ...} for each column, terminated by '0' + assert.ok(Buffer.isBuffer(result)); + var parsed = sqlite3.parse(result); + assert.deepEqual(parsed, {row: [], num: [], flt: [], blb: []}); + done(); + }); + }); + + it('should handle empty string and blob values', function(done) { + db.run("CREATE TABLE bar (txt text, blb blob)", function(err) { + if (err) throw err; + db.run("INSERT INTO bar VALUES('', X'')", function(err) { + if (err) throw err; + db.allMarshal("SELECT * FROM bar", function(err, result) { + if (err) throw err; + assert.ok(Buffer.isBuffer(result)); + var parsed = sqlite3.parse(result); + assert.deepEqual(parsed.txt, ['']); + assert.deepEqual(parsed.blb, [new Uint8Array(0)]); + done(); + }); + }); + }); + }); + after(function(done) { db.close(done); }); }); diff --git a/test/marshal-test.js b/test/marshal-test.js index 62c8bd358..0b92489f6 100644 --- a/test/marshal-test.js +++ b/test/marshal-test.js @@ -10,6 +10,9 @@ describe('marshal', function() { return new Uint8Array(Buffer.from(str)); } const samples = [ + // Empty string and empty buffer exercise _writeBytes with 0 bytes. + ['', 'u\x00\x00\x00\x00'], + [new Uint8Array(0), 's\x00\x00\x00\x00'], [null, 'N'], [1, 'i\x01\x00\x00\x00'], [1000000, 'i@B\x0f\x00'], From aade29694ce036cb17a54544a9a231331cc6c3ff Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Mon, 16 Mar 2026 09:27:38 -0400 Subject: [PATCH 2/2] update old self-hosted mac runner to native mac runner --- .github/workflows/ci.yml | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 867686089..04906b8da 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -35,8 +35,7 @@ jobs: node: 22 host: x86 target: x86 - # This is a self-hosted runner, github doesn't have this architecture yet. - - os: macos-m1 + - os: macos-latest node: 22 host: arm64 target: arm64 @@ -48,10 +47,6 @@ jobs: node-version: ${{ matrix.node }} architecture: ${{ matrix.host }} - - name: Add yarn (self-hosted) - if: matrix.os == 'macos-m1' - run: npm install -g yarn - - name: Add msbuild to PATH uses: microsoft/setup-msbuild@v1.1 if: contains(matrix.os, 'windows')