From: stephan Date: Wed, 26 Nov 2025 20:12:29 +0000 (+0000) Subject: kvvfs: silently reject attempts to change the page size, as changing it corrupts... X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a4cb56e425cebc7cb6f74d4e1535212457bf4813;p=thirdparty%2Fsqlite.git kvvfs: silently reject attempts to change the page size, as changing it corrupts this vfs for reasons not yet understood. Add more kvvfs tests. FossilOrigin-Name: 65272adfab333565719bde4f6c5bbe4f0b5415c17a8de43e6b2d7b0518cca4c7 --- diff --git a/ext/wasm/api/sqlite3-vfs-kvvfs.c-pp.js b/ext/wasm/api/sqlite3-vfs-kvvfs.c-pp.js index 82b960f870..cab96bc373 100644 --- a/ext/wasm/api/sqlite3-vfs-kvvfs.c-pp.js +++ b/ext/wasm/api/sqlite3-vfs-kvvfs.c-pp.js @@ -671,7 +671,7 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ s = newStorageObj(nm); installStorageAndJournal(s); s.files.push(f); - s.deleteAtRefc0 = deleteAt0; + s.deleteAtRefc0 = deleteAt0 || !!(capi.SQLITE_OPEN_DELETEONCLOSE & flags); debug("xOpen installed storage handle [",nm, nm+"-journal","]", s); } pFileHandles.set(pProtoFile, {storage: s, file: f, jzClass}); @@ -789,17 +789,51 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ return capi.SQLITE_ERROR; } }, + + xFileControl: function(pFile, opId, pArg){ + try{ + const h = pFileHandles.get(pFile); + if( h && opId===capi.SQLITE_FCNTL_PRAGMA ){ + /* pArg== length-3 (char**) */ + const zName = wasm.peekPtr(wasm.ptr.add(pArg, wasm.ptr.size)); + //const argv = wasm.cArgvToJs(3, pArg); + if( "page_size"===wasm.cstrToJs(zName) ){ + //debug("xFileControl pragma",wasm.cstrToJs(zName)); + const zVal = wasm.peekPtr(wasm.ptr.add(pArg, 2*wasm.ptr.size)); + if( zVal ){ + /* Without this, pragma page_size=N; followed by a + vacuum breaks the db. With this, it continues + working but does not actually change the page + size. */ + //debug("xFileControl pragma", h, + // "NOT setting page size to", wasm.cstrToJs(zVal)); + h.file.$szPage = -1; + return 0;//corrupts capi.SQLITE_NOTFOUND; + }else if( 0 && h.file.$szPage>0 ){ + // This works but is superfluous + //debug("xFileControl", h, "getting page size",h.file.$szPage); + wasm.pokePtr(pArg, wasm.allocCString(""+h.file.$szPage)); + // memory now owned by sqlite. + return 0;//capi.SQLITE_NOTFOUND; + } + } + } + return originalIoMethods(h.file).xFileControl(pFile, opId, pArg); + }catch(e){ + error("xFileControl",e); + return capi.SQLITE_ERROR; + } + }, + //#if nope xRead: function(pFile,pTgt,n,iOff64){}, xWrite: function(pFile,pSrc,n,iOff64){}, xTruncate: function(pFile,i64){}, xSync: function(pFile,flags){}, - xFileControl: function(pFile, opId, pArg){}, xFileSize: function(pFile,pi64Out){}, xLock: function(pFile,iLock){}, xUnlock: function(pFile,iLock){}, xCheckReservedLock: function(pFile,piOut){}, - xFileControl: function(pFile,iOp,pArg){}, xSectorSize: function(pFile){}, xDeviceCharacteristics: function(pFile){} //#endif @@ -820,7 +854,6 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ xLock: true, xUnlock: true, xCheckReservedLock: true, - xFileControl: function(pFile,iOp,pArg){}, xSectorSize: true, xDeviceCharacteristics: true //#endif @@ -1362,6 +1395,8 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ ){ const opt = DB.dbCtorHelper.normalizeArgs(...arguments); opt.vfs = 'kvvfs'; + if( opt.flags ) opt.flags = 'cw'+opt.flags; + else opt.flags = 'cw'; switch( opt.filename ){ /* sqlite3_open(), in these builds, recognizes the names below and performs some magic which we want to bypass diff --git a/ext/wasm/tester1.c-pp.js b/ext/wasm/tester1.c-pp.js index 06f32d52f1..4056d54780 100644 --- a/ext/wasm/tester1.c-pp.js +++ b/ext/wasm/tester1.c-pp.js @@ -2946,6 +2946,24 @@ globalThis.sqlite3InitModule = sqlite3InitModule; const filename = 'localThread' /* preinstalled instance */; const JDb = sqlite3.oo1.JsStorageDb; const DB = sqlite3.oo1.DB; + + T.mustThrowMatching(()=>{ + new JDb("this is an illegal name too long and spaces"); + /* We don't have a way to get error strings from xOpen() + to this point? xOpen() does not have a handle to the + db and SQLite is not calling xGetLastError() to fetch + the error string. */ + }, capi.SQLITE_RANGE); + T.mustThrowMatching(()=>{ + new JDb("012345678901234567890123"/*too long*/); + }, capi.SQLITE_RANGE); + T.mustThrowMatching(()=>new JDb(""), capi.SQLITE_RANGE); + { + const name = "01234567890123456789012" /* max name length */; + (new JDb(name)).close(); + T.assert( sqlite3.kvvfs.unlink(name) ); + } + sqlite3.kvvfs.clear(filename); let db = new JDb(filename); const sqlSetup = [ @@ -3027,31 +3045,17 @@ globalThis.sqlite3InitModule = sqlite3InitModule; //predicate: ()=>false, test: function(sqlite3){ const filename = 'my'; + const kvvfs = sqlite3.kvvfs; const DB = sqlite3.oo1.DB; const JDb = sqlite3.oo1.JsStorageDb; let db; let duo; + let q1, q2; const sqlSetup = [ 'create table kvvfs(a);', 'insert into kvvfs(a) values(1),(2),(3)' ]; const sqlCount = 'select count(*) from kvvfs'; - T.mustThrowMatching(()=>{ - new JDb("this is an illegal name too long and spaces"); - /* We don't have a way to get error strings from xOpen() - to this point? xOpen() does not have a handle to the - db and SQLite is not calling xGetLastError() to fetch - the error string. */ - }, capi.SQLITE_RANGE); - T.mustThrowMatching(()=>{ - new JDb("012345678901234567890123"/*too long*/); - }, capi.SQLITE_RANGE); - T.mustThrowMatching(()=>new JDb(""), capi.SQLITE_RANGE); - { - const name = "01234567890123456789012" /* max name length */; - (new JDb(name)).close(); - T.assert( sqlite3.kvvfs.unlink(name) ); - } try { const exportDb = sqlite3.kvvfs.export; @@ -3065,9 +3069,10 @@ globalThis.sqlite3InitModule = sqlite3InitModule; duo.exec('insert into kvvfs(a) values(4),(5),(6)'); T.assert(6 === db.selectValue(sqlCount)); let exp = exportDb(filename); + let expectRows = 6; console.debug("exported db",exp); db.close(); - T.assert(6 === duo.selectValue(sqlCount)); + T.assert(expectRows === duo.selectValue(sqlCount)); duo.close(); T.mustThrowMatching(function(){ let ddb = new JDb(filename); @@ -3075,13 +3080,16 @@ globalThis.sqlite3InitModule = sqlite3InitModule; finally{ddb.close()} }, /.*no such table: kvvfs.*/); + T.assert( kvvfs.unlink(filename) ) + .assert( !kvvfs.exists(filename) ); + const importDb = sqlite3.kvvfs.import; - duo = new JDb(filename); + duo = new JDb(dbFileRaw); T.mustThrowMatching(()=>importDb(exp,true), /.*in use.*/); duo.close(); importDb(exp, true); - duo = new JDb(filename); - T.assert(6 === duo.selectValue(sqlCount)); + duo = new JDb(dbFileRaw); + T.assert(expectRows === duo.selectValue(sqlCount)); let newCount; try{ duo.transaction(()=>{ @@ -3091,10 +3099,95 @@ globalThis.sqlite3InitModule = sqlite3InitModule; }); }catch(e){/*ignored*/} T.assert(7===newCount, "Unexpected row count before rollback") - .assert(6 === duo.selectValue(sqlCount), + .assert(expectRows === duo.selectValue(sqlCount), "Unexpected row count after rollback"); duo.close(); + T.assert( kvvfs.unlink(filename) ) + .assert( !kvvfs.exists(filename) ); + + importDb(exp, true); + db = new JDb({ + filename, + flags: 't' + }); + duo = new JDb(filename); + T.assert(expectRows === duo.selectValue(sqlCount)); + const sqlIns1 = "insert into kvvfs(a) values(?)"; + q1 = db.prepare(sqlIns1); + q2 = duo.prepare(sqlIns1); + if( 0 ){ + q1.bind('from q1').stepFinalize(); + ++expectRows; + T.assert(expectRows === duo.selectValue(sqlCount), + "Unexpected record count."); + q2.bind('from q1').stepFinalize(); + ++expectRows; + }else{ + q1.bind('from q1'); + T.assert(capi.SQLITE_DONE===capi.sqlite3_step(q1), + "Unexpected step result"); + ++expectRows; + T.assert(expectRows === duo.selectValue(sqlCount), + "Unexpected record count."); + q2.bind('from q1').step(); + ++expectRows; + } + T.assert(expectRows === db.selectValue(sqlCount), + "Unexpected record count."); + q1.finalize(); + q2.finalize(); + + if( 1 ){ + const pageSize = 1024 * 16; + console.debug("Export before vacuum", exportDb(filename)); + console.debug("page size before vacuum", + db.selectArray( + "select page_size from pragma_page_size()" + )); + db.exec([ + "delete from kvvfs where a=1;", + "pragma page_size="+pageSize+";", + "vacuum;" + ]); + --expectRows; + console.debug("page size after", + db.selectArray( + "select page_size from pragma_page_size()" + )); + T.assert(expectRows === duo.selectValue(sqlCount), + "Unexpected record count."); + T.assert( 8192 == db.selectValue( + /* kvvfs actively resists attempts to change the page size + because _not_ doing so causes a vacuum to corrupt + the db. */ + "select page_size from pragma_page_size()" + ) ); + if( 0 ){ + // It's stuck at 8k + T.assert(pageSize == duo.selectValue("pragma page_size"), + "Unexpected page size."); + } + // It fails at this point for non-8k sizes + T.assert(expectRows === duo.selectValue(sqlCount), + "Unexpected record count."); + exp = exportDb(filename); + console.debug("vacuumed export",exp); + }else{ + expectRows = 6; + } + + db.close(); + duo.close(); + T.assert( kvvfs.unlink(exp.name) ) + .assert( !kvvfs.exists(exp.name) ); + importDb(exp); + T.assert( kvvfs.exists(exp.name) ); + db = new JDb(exp.name); + console.debug("column count after export",db.selectValue(sqlCount)); + T.assert(expectRows === db.selectValue(sqlCount), + "Unexpected record count."); + /* TODO: more advanced concurrent use tests, e.g. looping over a query in one connection while writing from @@ -3104,6 +3197,8 @@ globalThis.sqlite3InitModule = sqlite3InitModule; appropriate. */ }finally{ + q1?.finalize?.(); + q2?.finalize?.(); db?.close?.(); duo?.close?.(); } diff --git a/manifest b/manifest index b381aef70a..9139ab8fc9 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Factor\sout\san\sobsolete\smalloc\sfailure\scheck\sand\san\ssnprintf()\sfor\skeys\sin\stransient\skvvfs\sstorage. -D 2025-11-26T17:19:44.822 +C kvvfs:\ssilently\sreject\sattempts\sto\schange\sthe\spage\ssize,\sas\schanging\sit\scorrupts\sthis\svfs\sfor\sreasons\snot\syet\sunderstood.\sAdd\smore\skvvfs\stests. +D 2025-11-26T20:12:29.088 F .fossil-settings/binary-glob 61195414528fb3ea9693577e1980230d78a1f8b0a54c78cf1b9b24d0a409ed6a x F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea @@ -600,7 +600,7 @@ F ext/wasm/api/sqlite3-api-worker1.c-pp.js 1041dd645e8e821c082b628cd8d9acf70c667 F ext/wasm/api/sqlite3-license-version-header.js 0c807a421f0187e778dc1078f10d2994b915123c1223fe752b60afdcd1263f89 F ext/wasm/api/sqlite3-opfs-async-proxy.js 9654b565b346dc609b75d15337f20acfa7af7d9d558da1afeb9b6d8eaa404966 F ext/wasm/api/sqlite3-vfs-helper.c-pp.js 3f828cc66758acb40e9c5b4dcfd87fd478a14c8fb7f0630264e6c7fa0e57515d -F ext/wasm/api/sqlite3-vfs-kvvfs.c-pp.js ba339fcc1ea253cdfd8c5f03567d3e2bf6a71c178ad6bbc6b0d60aaa6062216c +F ext/wasm/api/sqlite3-vfs-kvvfs.c-pp.js 08b058de61d3a04e98388a179bf622dbfab0d841aa3546ce1541e442880a012a F ext/wasm/api/sqlite3-vfs-opfs-sahpool.c-pp.js a2eea6442556867b589e04107796c6e1d04a472219529eeb45b7cd221d7d048b F ext/wasm/api/sqlite3-vfs-opfs.c-pp.js 88ce2078267a2d1af57525a32d896295f4a8db7664de0e17e82dc9ff006ed8d3 F ext/wasm/api/sqlite3-vtab-helper.c-pp.js 9097074724172e31e56ce20ccd7482259cf72a76124213cbc9469d757676da86 @@ -647,7 +647,7 @@ F ext/wasm/test-opfs-vfs.html 1f2d672f3f3fce810dfd48a8d56914aba22e45c6834e262555 F ext/wasm/test-opfs-vfs.js 1618670e466f424aa289859fe0ec8ded223e42e9e69b5c851f809baaaca1a00c F ext/wasm/tester1-worker.c-pp.html 0e432ec2c0d99cd470484337066e8d27e7aee4641d97115338f7d962bf7b081a F ext/wasm/tester1.c-pp.html 52d88fe2c6f21a046030a36410b4839b632f4424028197a45a3d5669ea724ddb -F ext/wasm/tester1.c-pp.js c58165ce63c1c20c6ff5276443d97a629ef3e04f461b3a265c6a36ff93b12954 +F ext/wasm/tester1.c-pp.js f7a2aa456af7b713772d2750c0b9e7693fb2ddbfa152faf0d41a4ef9121ac57b F ext/wasm/tests/opfs/concurrency/index.html 657578a6e9ce1e9b8be951549ed93a6a471f4520a99e5b545928668f4285fb5e F ext/wasm/tests/opfs/concurrency/test.js d08889a5bb6e61937d0b8cbb78c9efbefbf65ad09f510589c779b7cc6a803a88 F ext/wasm/tests/opfs/concurrency/worker.js 0a8c1a3e6ebb38aabbee24f122693f1fb29d599948915c76906681bb7da1d3d2 @@ -2178,8 +2178,8 @@ F tool/version-info.c 33d0390ef484b3b1cb685d59362be891ea162123cea181cb8e6d2cf6dd F tool/warnings-clang.sh bbf6a1e685e534c92ec2bfba5b1745f34fb6f0bc2a362850723a9ee87c1b31a7 F tool/warnings.sh d924598cf2f55a4ecbc2aeb055c10bd5f48114793e7ba25f9585435da29e7e98 F tool/win/sqlite.vsix deb315d026cc8400325c5863eef847784a219a2f -P 3c1d5eac270e8afe6196ccb11a6d7bb0d1f262c882ce390a16b998bd2f55cb3d -R 559ee0d478a7f3b78449f87dd9293df4 +P e2e94f9094f8bbe93cdf7d2a2e72e92462b94e18319603c5a364f183cb780be1 +R eb733c7a4409e01bf94307709e4f0a90 U stephan -Z 94ea3fa5373b51001d96c5da81f5054a +Z 460ddd5e8bff8ef00b5202225229de31 # Remove this line to create a well-formed Fossil manifest. diff --git a/manifest.uuid b/manifest.uuid index 34e45c9972..29d3bc3ee3 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -e2e94f9094f8bbe93cdf7d2a2e72e92462b94e18319603c5a364f183cb780be1 +65272adfab333565719bde4f6c5bbe4f0b5415c17a8de43e6b2d7b0518cca4c7