From 4bebb2207d0f50ff9b59129c6b0255ee8909568e Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Fri, 3 Jul 2026 19:09:57 +0800 Subject: [PATCH] idb: on abort, rollback created stores/indexes Also, better query key validation --- build.zig.zon | 2 +- src/browser/webapi/storage/idb/IDBDatabase.zig | 1 + src/browser/webapi/storage/idb/IDBIndex.zig | 18 ++++++++++++++++-- src/browser/webapi/storage/idb/IDBKeyRange.zig | 17 ++++++++++++++++- .../webapi/storage/idb/IDBObjectStore.zig | 11 ++++++++--- .../webapi/storage/idb/IDBTransaction.zig | 15 +++++++++++++++ src/browser/webapi/storage/idb/Key.zig | 2 +- 7 files changed, 58 insertions(+), 8 deletions(-) diff --git a/build.zig.zon b/build.zig.zon index 0b0d49118..ce222f12f 100644 --- a/build.zig.zon +++ b/build.zig.zon @@ -8,7 +8,7 @@ .url = "https://github.com/lightpanda-io/zig-v8-fork/archive/refs/tags/v0.4.9.tar.gz", .hash = "v8-0.0.0-xddH63jnAgB8guO5jPzJD4pnX-CI5od-g4v-fIIjmQXr", }, - //.v8 = .{ .path = "../zig-v8-fork" }, + // .v8 = .{ .path = "../zig-v8-fork" }, .brotli = .{ // v1.2.0 .url = "https://github.com/google/brotli/archive/028fb5a23661f123017c060daa546b55cf4bde29.tar.gz", diff --git a/src/browser/webapi/storage/idb/IDBDatabase.zig b/src/browser/webapi/storage/idb/IDBDatabase.zig index 2da1b397b..677ba78a6 100644 --- a/src/browser/webapi/storage/idb/IDBDatabase.zig +++ b/src/browser/webapi/storage/idb/IDBDatabase.zig @@ -105,6 +105,7 @@ pub fn createObjectStore( const owned_name = try txn.dupe(name); const store = try IDBObjectStore.init(txn, store_id, owned_name, key_path, opts.autoIncrement); + store._created = true; try txn.cacheStore(store); return store; } diff --git a/src/browser/webapi/storage/idb/IDBIndex.zig b/src/browser/webapi/storage/idb/IDBIndex.zig index 85d7fa35c..9aa342120 100644 --- a/src/browser/webapi/storage/idb/IDBIndex.zig +++ b/src/browser/webapi/storage/idb/IDBIndex.zig @@ -44,6 +44,9 @@ _name: []const u8, _key_path: Key.KeyPath, _unique: bool, _multi_entry: bool, +_deleted: bool = false, +// Created by this (versionchange) transaction — so an abort must delete it. +_created: bool = false, // not just for efficiency, we must return the same v8::Array every time the // compound key is accessed. _key_path_js: ?*js.Value.BareGlobal = null, @@ -70,7 +73,16 @@ pub fn releaseRef(self: *IDBIndex, page: *Page) void { self._store._txn.releaseRef(page); } +// The index (or its object store) may have been deleted — including by an +// aborted upgrade. That check precedes the transaction-active check per spec. +fn assertLive(self: *const IDBIndex) !void { + if (self._deleted or self._store._deleted) { + return error.InvalidStateError; + } +} + fn txn(self: *IDBIndex) !*IDBTransaction { + try self.assertLive(); const t = self._store._txn; try t.assertActive(); return t; @@ -78,7 +90,7 @@ fn txn(self: *IDBIndex) !*IDBTransaction { pub fn get(self: *IDBIndex, query: js.Value, exec: *Execution) !*IDBRequest { const t = try self.txn(); - const bounds = try IDBKeyRange.resolveQuery(t._arena, query, exec); + const bounds = try IDBKeyRange.resolveKey(t._arena, query, exec); const request = try t.newRequest(); return request.submit(.{ .index_get = .{ .index = self, .bounds = bounds } }, exec); } @@ -96,7 +108,7 @@ pub fn runGet(self: *IDBIndex, request: *IDBRequest, bounds: Engine.Bounds, exec pub fn getKey(self: *IDBIndex, query: js.Value, exec: *Execution) !*IDBRequest { const t = try self.txn(); - const bounds = try IDBKeyRange.resolveQuery(t._arena, query, exec); + const bounds = try IDBKeyRange.resolveKey(t._arena, query, exec); const request = try t.newRequest(); return request.submit(.{ .index_get_key = .{ .index = self, .bounds = bounds } }, exec); } @@ -188,11 +200,13 @@ pub fn runCount(self: *IDBIndex, request: *IDBRequest, bounds: Engine.Bounds, ex } pub fn openCursor(self: *IDBIndex, query: ?js.Value, direction: ?IDBCursor.Direction, exec: *Execution) !*IDBRequest { + try self.assertLive(); const bounds = try IDBKeyRange.resolveQuery(self._store._txn._arena, query, exec); return IDBCursor.initIndex(self, bounds, direction orelse .next, false, exec); } pub fn openKeyCursor(self: *IDBIndex, query: ?js.Value, direction: ?IDBCursor.Direction, exec: *Execution) !*IDBRequest { + try self.assertLive(); const bounds = try IDBKeyRange.resolveQuery(self._store._txn._arena, query, exec); return IDBCursor.initIndex(self, bounds, direction orelse .next, true, exec); } diff --git a/src/browser/webapi/storage/idb/IDBKeyRange.zig b/src/browser/webapi/storage/idb/IDBKeyRange.zig index 75690685a..0adfe795d 100644 --- a/src/browser/webapi/storage/idb/IDBKeyRange.zig +++ b/src/browser/webapi/storage/idb/IDBKeyRange.zig @@ -123,9 +123,24 @@ pub fn toBounds(self: *const IDBKeyRange) Engine.Bounds { }; } +// query/count/openCursor: a missing or null/undefined query means "all records". pub fn resolveQuery(arena: Allocator, query: ?js.Value, exec: *Execution) !Engine.Bounds { - const q = query orelse return Engine.Bounds.unbounded(); + return resolveQueryInner(arena, query, false, exec); +} + +// get/getKey/delete: the query must be a valid key or key range. null/undefined +// is a DataError +pub fn resolveKey(arena: Allocator, query: ?js.Value, exec: *Execution) !Engine.Bounds { + return resolveQueryInner(arena, query, true, exec); +} + +fn resolveQueryInner(arena: Allocator, query: ?js.Value, null_disallowed: bool, exec: *Execution) !Engine.Bounds { + const q = query orelse { + if (null_disallowed) return error.DataError; + return Engine.Bounds.unbounded(); + }; if (q.isNullOrUndefined()) { + if (null_disallowed) return error.DataError; return Engine.Bounds.unbounded(); } diff --git a/src/browser/webapi/storage/idb/IDBObjectStore.zig b/src/browser/webapi/storage/idb/IDBObjectStore.zig index 7ad58e5e4..4b8cafe02 100644 --- a/src/browser/webapi/storage/idb/IDBObjectStore.zig +++ b/src/browser/webapi/storage/idb/IDBObjectStore.zig @@ -46,6 +46,8 @@ _key_path: ?Key.KeyPath, _auto_increment: bool, _txn: *IDBTransaction, _deleted: bool = false, +// Created by this (versionchange) transaction — so an abort must delete it. +_created: bool = false, // identity map, store.indexes('a') === store.index('a') _indexes: std.ArrayList(*IDBIndex) = .empty, // not just for efficiency, we must return the same v8::Array every time the @@ -91,7 +93,7 @@ pub fn get(self: *IDBObjectStore, query: js.Value, exec: *Execution) !*IDBReques try self.assertLive(); const txn = self._txn; try txn.assertActive(); - const bounds = try IDBKeyRange.resolveQuery(txn._arena, query, exec); + const bounds = try IDBKeyRange.resolveKey(txn._arena, query, exec); const request = try txn.newRequest(); return request.submit(.{ .store_get = .{ .store = self, .bounds = bounds } }, exec); } @@ -114,7 +116,7 @@ pub fn delete(self: *IDBObjectStore, query: js.Value, exec: *Execution) !*IDBReq return error.ReadOnlyError; } try txn.assertActive(); - const bounds = try IDBKeyRange.resolveQuery(txn._arena, query, exec); + const bounds = try IDBKeyRange.resolveKey(txn._arena, query, exec); const request = try txn.newRequest(); return request.submit(.{ .store_delete = .{ .store = self, .bounds = bounds } }, exec); } @@ -239,7 +241,7 @@ pub fn getKey(self: *IDBObjectStore, query: js.Value, exec: *Execution) !*IDBReq try self.assertLive(); const txn = self._txn; try txn.assertActive(); - const bounds = try IDBKeyRange.resolveQuery(txn._arena, query, exec); + const bounds = try IDBKeyRange.resolveKey(txn._arena, query, exec); const request = try txn.newRequest(); return request.submit(.{ .store_get_key = .{ .store = self, .bounds = bounds } }, exec); } @@ -529,6 +531,7 @@ pub fn createIndex(self: *IDBObjectStore, name: []const u8, key_path: Key.KeyPat .unique = opts.unique, .multi_entry = opts.multiEntry, }, owned_name); + idb_index._created = true; try self._engine.releaseSavepoint(); try self._indexes.append(txn._arena, idb_index); return idb_index; @@ -549,6 +552,8 @@ pub fn deleteIndex(self: *IDBObjectStore, name: []const u8, _: *Execution) !void }; for (self._indexes.items, 0..) |idx, i| { if (std.mem.eql(u8, idx._name, name)) { + // A handle the caller still holds must report itself deleted. + idx._deleted = true; _ = self._indexes.swapRemove(i); break; } diff --git a/src/browser/webapi/storage/idb/IDBTransaction.zig b/src/browser/webapi/storage/idb/IDBTransaction.zig index aa65f7515..08b3738ed 100644 --- a/src/browser/webapi/storage/idb/IDBTransaction.zig +++ b/src/browser/webapi/storage/idb/IDBTransaction.zig @@ -247,6 +247,21 @@ pub fn abortWith(self: *IDBTransaction, exec: *Execution, reason: ?anyerror) err self._settled = true; self._error = reason; + // An aborted upgrade reverts the schema: stores and indexes created during + // it no longer exist, so handles the caller still holds must report deleted. + if (self._mode == .versionchange) { + for (self._stores.items) |store| { + if (store._created) { + store._deleted = true; + } + for (store._indexes.items) |idx| { + if (idx._created) { + idx._deleted = true; + } + } + } + } + if (self._begun) { self._engine.rollback(); self._begun = false; diff --git a/src/browser/webapi/storage/idb/Key.zig b/src/browser/webapi/storage/idb/Key.zig index 1bca8ccb6..087f45f81 100644 --- a/src/browser/webapi/storage/idb/Key.zig +++ b/src/browser/webapi/storage/idb/Key.zig @@ -260,7 +260,7 @@ fn isIdentifier(s: []const u8) bool { for (s[1..]) |c| { const ok = switch (c) { - 'a'...'z', 'A'...'Z', '_', '$', '0' ... '9' => true, + 'a'...'z', 'A'...'Z', '_', '$', '0'...'9' => true, else => c >= 0x80, }; if (ok == false) {