Skip to content

Commit 39de389

Browse files
committed
fixup! sqlite: re-validate database state after reading options
Add a test that options getters do not run at all when the database is already closed. This is the invariant that justifies keeping the early state check rather than moving it, and it is the only coverage for the early check added to createSession(). Signed-off-by: Trevor Burnham <trevorburnham@gmail.com>
1 parent 1e71b2a commit 39de389

1 file changed

Lines changed: 52 additions & 0 deletions

File tree

test/parallel/test-sqlite-options-getter-reentry.js

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,58 @@ suite('closing the database from an options getter', () => {
141141
});
142142
});
143143

144+
// The state check runs before the options bag is read, so a call that is
145+
// already doomed must not execute any of the caller's getters.
146+
test('options getters do not run on an already-closed database', (t) => {
147+
const source = new DatabaseSync(':memory:');
148+
source.exec('CREATE TABLE data(key INTEGER PRIMARY KEY)');
149+
const image = source.serialize();
150+
const session = source.createSession();
151+
source.exec('INSERT INTO data (key) VALUES (1)');
152+
const changeset = session.changeset();
153+
154+
const cases = {
155+
prepare: (db, options) => db.prepare('SELECT 1', options),
156+
function: (db, options) => db.function('fn', options, () => 1),
157+
aggregate: (db, options) => db.aggregate('agg', options),
158+
deserialize: (db, options) => db.deserialize(image, options),
159+
createSession: (db, options) => db.createSession(options),
160+
applyChangeset: (db, options) => db.applyChangeset(changeset, options),
161+
backup: (db, options) =>
162+
backup(db, join(tmpdir.path, 'closed-backup.db'), options),
163+
};
164+
165+
// The property each method reads first, and a valid value for it, so that a
166+
// getter which does run leaves the "did not run" assertion as the failure
167+
// rather than a type error from the returned value.
168+
const probes = {
169+
prepare: ['returnArrays', false],
170+
function: ['useBigIntArguments', false],
171+
aggregate: ['start', 0],
172+
deserialize: ['dbName', 'main'],
173+
createSession: ['db', 'main'],
174+
applyChangeset: ['onConflict', undefined],
175+
backup: ['rate', 1],
176+
};
177+
178+
for (const [name, invoke] of Object.entries(cases)) {
179+
const db = new DatabaseSync(':memory:');
180+
db.close();
181+
182+
const [key, value] = probes[name];
183+
let ran = false;
184+
const options = {
185+
get [key]() {
186+
ran = true;
187+
return value;
188+
},
189+
};
190+
191+
t.assert.throws(() => invoke(db, options), invalidState, name);
192+
t.assert.strictEqual(ran, false, `${name} ran an options getter`);
193+
}
194+
});
195+
144196
suite('resizing a deserialize() buffer from an options getter', () => {
145197
test('throws rather than handing uninitialized memory to SQLite', (t) => {
146198
const source = new DatabaseSync(':memory:');

0 commit comments

Comments
 (0)