Skip to content

Commit f341103

Browse files
authored
fix bad behavior in marshal for empty buffers/strings (#16)
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.
1 parent f302e6f commit f341103

5 files changed

Lines changed: 41 additions & 8 deletions

File tree

.github/workflows/ci.yml

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,7 @@ jobs:
3535
node: 22
3636
host: x86
3737
target: x86
38-
# This is a self-hosted runner, github doesn't have this architecture yet.
39-
- os: macos-m1
38+
- os: macos-latest
4039
node: 22
4140
host: arm64
4241
target: arm64
@@ -48,10 +47,6 @@ jobs:
4847
node-version: ${{ matrix.node }}
4948
architecture: ${{ matrix.host }}
5049

51-
- name: Add yarn (self-hosted)
52-
if: matrix.os == 'macos-m1'
53-
run: npm install -g yarn
54-
5550
- name: Add msbuild to PATH
5651
uses: microsoft/setup-msbuild@v1.1
5752
if: contains(matrix.os, 'windows')
@@ -99,6 +94,10 @@ jobs:
9994
- name: Package prebuilt binaries
10095
run: yarn node-pre-gyp package --target_arch=${{ env.TARGET }}
10196

97+
- name: Rebuild with assertions and re-test
98+
if: contains(matrix.os, 'ubuntu')
99+
run: yarn build:assert && yarn test
100+
102101
- name: Prepare for saving artifact
103102
run: |
104103
ARTIFACT_NAME=$(echo prebuilt-binaries-${{ matrix.os }}-${{ matrix.target }} | sed 's/[^-a-zA-Z0-9]/_/g')

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@
7272
"build": "node-pre-gyp build",
7373
"build:debug": "node-pre-gyp build --debug",
7474
"install": "node-pre-gyp install --fallback-to-build",
75+
"build:assert": "CXXFLAGS=\"${CXXFLAGS:-} -D_GLIBCXX_ASSERTIONS\" node-pre-gyp rebuild --build-from-source",
7576
"pretest": "node test/support/createdb.js",
7677
"test": "mocha -R spec --timeout 480000",
7778
"test:memory": "node test/support/memory_check.js",

src/marshal.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ class Marshaller {
3838
}
3939

4040
void _writeBytes(const void *bytes, size_t nbytes) {
41+
if (nbytes == 0) return;
4142
size_t offset = buffer.size();
4243
buffer.resize(buffer.size() + nbytes);
4344
memcpy(&buffer[offset], bytes, nbytes);
@@ -57,7 +58,7 @@ class Marshaller {
5758

5859
void append(const Marshaller &marshaller) {
5960
const std::vector<char> &buf = marshaller.getBuffer();
60-
_writeBytes(&buf[0], buf.size());
61+
_writeBytes(buf.data(), buf.size());
6162
}
6263

6364
// Marshal the given value depending on its type.
@@ -68,7 +69,7 @@ class Marshaller {
6869
}
6970

7071
void marshalString(const std::string &value) {
71-
marshalString(&value[0], value.size());
72+
marshalString(value.data(), value.size());
7273
}
7374

7475
void marshalString(const char *value, int32_t size) {

test/allMarshal.test.js

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,5 +60,34 @@ describe('Database#allMarshal', function() {
6060
}
6161
});
6262

63+
it('should handle empty result set', function(done) {
64+
db.allMarshal("SELECT * FROM foo WHERE 1=0", function(err, result) {
65+
if (err) throw err;
66+
// Should be a dict with column names mapping to empty lists:
67+
// {s<col>[\x00\x00\x00\x00 ...} for each column, terminated by '0'
68+
assert.ok(Buffer.isBuffer(result));
69+
var parsed = sqlite3.parse(result);
70+
assert.deepEqual(parsed, {row: [], num: [], flt: [], blb: []});
71+
done();
72+
});
73+
});
74+
75+
it('should handle empty string and blob values', function(done) {
76+
db.run("CREATE TABLE bar (txt text, blb blob)", function(err) {
77+
if (err) throw err;
78+
db.run("INSERT INTO bar VALUES('', X'')", function(err) {
79+
if (err) throw err;
80+
db.allMarshal("SELECT * FROM bar", function(err, result) {
81+
if (err) throw err;
82+
assert.ok(Buffer.isBuffer(result));
83+
var parsed = sqlite3.parse(result);
84+
assert.deepEqual(parsed.txt, ['']);
85+
assert.deepEqual(parsed.blb, [new Uint8Array(0)]);
86+
done();
87+
});
88+
});
89+
});
90+
});
91+
6392
after(function(done) { db.close(done); });
6493
});

test/marshal-test.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@ describe('marshal', function() {
1010
return new Uint8Array(Buffer.from(str));
1111
}
1212
const samples = [
13+
// Empty string and empty buffer exercise _writeBytes with 0 bytes.
14+
['', 'u\x00\x00\x00\x00'],
15+
[new Uint8Array(0), 's\x00\x00\x00\x00'],
1316
[null, 'N'],
1417
[1, 'i\x01\x00\x00\x00'],
1518
[1000000, 'i@B\x0f\x00'],

0 commit comments

Comments
 (0)