From abb28667f159796be159292b8c0c6a68b56f73d9 Mon Sep 17 00:00:00 2001 From: dan Date: Wed, 7 Sep 2022 16:41:33 +0000 Subject: [PATCH] Ensure that the recover extension properly escapes CR and NL characters in text mode. Also that it holds transactions open on both input and output databases for the duration of a recovery operation. FossilOrigin-Name: 6cca8913e703635ad89415a60fc84000ac188d9df43f45594b8ad87facb91d54 --- ext/recover/sqlite3recover.c | 331 +++++++++++++++++++++++------------ ext/recover/test_recover.c | 5 +- manifest | 14 +- manifest.uuid | 2 +- 4 files changed, 229 insertions(+), 123 deletions(-) diff --git a/ext/recover/sqlite3recover.c b/ext/recover/sqlite3recover.c index df6ab36b98..de3dc6ea1a 100644 --- a/ext/recover/sqlite3recover.c +++ b/ext/recover/sqlite3recover.c @@ -234,15 +234,15 @@ static void recoverFinalize(sqlite3_recover *p, sqlite3_stmt *pStmt){ static int recoverExec(sqlite3_recover *p, sqlite3 *db, const char *zSql){ if( p->errCode==SQLITE_OK ){ - int rc = sqlite3_exec(p->dbOut, zSql, 0, 0, 0); + int rc = sqlite3_exec(db, zSql, 0, 0, 0); if( rc ){ - recoverDbError(p, p->dbOut); + recoverDbError(p, db); } } return p->errCode; } -static char *recoverPrintf(sqlite3_recover *p, const char *zFmt, ...){ +static char *recoverMPrintf(sqlite3_recover *p, const char *zFmt, ...){ va_list ap; char *z; va_start(ap, zFmt); @@ -375,79 +375,187 @@ static void recoverGetPage( } } +/* +** Find a string that is not found anywhere in z[]. Return a pointer +** to that string. +** +** Try to use zA and zB first. If both of those are already found in z[] +** then make up some string and store it in the buffer zBuf. +*/ +static const char *unused_string( + const char *z, /* Result must not appear anywhere in z */ + const char *zA, const char *zB, /* Try these first */ + char *zBuf /* Space to store a generated string */ +){ + unsigned i = 0; + if( strstr(z, zA)==0 ) return zA; + if( strstr(z, zB)==0 ) return zB; + do{ + sqlite3_snprintf(20,zBuf,"(%s%u)", zA, i++); + }while( strstr(z,zBuf)!=0 ); + return zBuf; +} + + +/* +** Scalar function "escape_crnl". The argument passed to this function is the +** output of built-in function quote(). If the first character of the input is +** "'", indicating that the value passed to quote() was a text value, then this +** function searches the input for "\n" and "\r" characters and adds a wrapper +** similar to the following: +** +** replace(replace(, '\n', char(10), '\r', char(13)); +** +** Or, if the first character of the input is not "'", then a copy +** of the input is returned. +*/ +static void recoverEscapeCrnl( + sqlite3_context *context, + int argc, + sqlite3_value **argv +){ + const char *zText = (const char*)sqlite3_value_text(argv[0]); + if( zText && zText[0]=='\'' ){ + int nText = sqlite3_value_bytes(argv[0]); + int i; + char zBuf1[20]; + char zBuf2[20]; + const char *zNL = 0; + const char *zCR = 0; + int nCR = 0; + int nNL = 0; + + for(i=0; zText[i]; i++){ + if( zNL==0 && zText[i]=='\n' ){ + zNL = unused_string(zText, "\\n", "\\012", zBuf1); + nNL = (int)strlen(zNL); + } + if( zCR==0 && zText[i]=='\r' ){ + zCR = unused_string(zText, "\\r", "\\015", zBuf2); + nCR = (int)strlen(zCR); + } + } + + if( zNL || zCR ){ + int iOut = 0; + i64 nMax = (nNL > nCR) ? nNL : nCR; + i64 nAlloc = nMax * nText + (nMax+64)*2; + char *zOut = (char*)sqlite3_malloc64(nAlloc); + if( zOut==0 ){ + sqlite3_result_error_nomem(context); + return; + } + + if( zNL && zCR ){ + memcpy(&zOut[iOut], "replace(replace(", 16); + iOut += 16; + }else{ + memcpy(&zOut[iOut], "replace(", 8); + iOut += 8; + } + for(i=0; zText[i]; i++){ + if( zText[i]=='\n' ){ + memcpy(&zOut[iOut], zNL, nNL); + iOut += nNL; + }else if( zText[i]=='\r' ){ + memcpy(&zOut[iOut], zCR, nCR); + iOut += nCR; + }else{ + zOut[iOut] = zText[i]; + iOut++; + } + } + + if( zNL ){ + memcpy(&zOut[iOut], ",'", 2); iOut += 2; + memcpy(&zOut[iOut], zNL, nNL); iOut += nNL; + memcpy(&zOut[iOut], "', char(10))", 12); iOut += 12; + } + if( zCR ){ + memcpy(&zOut[iOut], ",'", 2); iOut += 2; + memcpy(&zOut[iOut], zCR, nCR); iOut += nCR; + memcpy(&zOut[iOut], "', char(13))", 12); iOut += 12; + } + + sqlite3_result_text(context, zOut, iOut, SQLITE_TRANSIENT); + sqlite3_free(zOut); + return; + } + } + + sqlite3_result_value(context, argv[0]); +} + + #ifdef _WIN32 __declspec(dllexport) #endif int sqlite3_dbdata_init(sqlite3*, char**, const sqlite3_api_routines*); static int recoverOpenOutput(sqlite3_recover *p){ - int rc = SQLITE_OK; - if( p->dbOut==0 ){ - const int flags = SQLITE_OPEN_URI|SQLITE_OPEN_CREATE|SQLITE_OPEN_READWRITE; - sqlite3 *db = 0; + struct Func { + const char *zName; + int nArg; + void (*xFunc)(sqlite3_context*,int,sqlite3_value **); + } aFunc[] = { + { "getpage", 1, recoverGetPage }, + { "page_is_used", 1, recoverPageIsUsed }, + { "read_i32", 2, recoverReadI32 }, + { "escape_crnl", 1, recoverEscapeCrnl }, + }; - assert( p->dbOut==0 ); + const int flags = SQLITE_OPEN_URI|SQLITE_OPEN_CREATE|SQLITE_OPEN_READWRITE; + sqlite3 *db = 0; /* New database handle */ + int ii; /* For iterating through aFunc[] */ - rc = sqlite3_open_v2(p->zUri, &db, flags, 0); - if( rc==SQLITE_OK ){ - rc = sqlite3_exec(db, "PRAGMA writable_schema = 1", 0, 0, 0); - } - if( rc==SQLITE_OK ){ - const char *zPath = p->zStateDb ? p->zStateDb : ":memory:"; - char *zSql = sqlite3_mprintf("ATTACH %Q AS recovery", zPath); - if( zSql==0 ){ - rc = p->errCode = SQLITE_NOMEM; - }else{ - rc = sqlite3_exec(db, zSql, 0, 0, 0); - } - sqlite3_free(zSql); - } + assert( p->dbOut==0 ); - /* Truncate the output database. This is done by opening a new, empty, - ** temp db, then using the backup API to clobber any existing output db - ** with a copy of it. */ - if( rc==SQLITE_OK ){ - sqlite3 *db2 = 0; - rc = sqlite3_open("", &db2); - if( rc==SQLITE_OK ){ - sqlite3_backup *pBackup = sqlite3_backup_init(db, "main", db2, "main"); - if( pBackup ){ - while( sqlite3_backup_step(pBackup, 1000)==SQLITE_OK ); - rc = sqlite3_backup_finish(pBackup); - } - } - sqlite3_close(db2); - } - - if( rc==SQLITE_OK ){ - rc = sqlite3_exec(db, RECOVERY_SCHEMA, 0, 0, 0); - } - - if( rc==SQLITE_OK ){ - sqlite3_dbdata_init(db, 0, 0); - rc = sqlite3_create_function( - db, "getpage", 1, SQLITE_UTF8, (void*)p, recoverGetPage, 0, 0 - ); - } - if( rc==SQLITE_OK ){ - rc = sqlite3_create_function( - db, "page_is_used", 1, SQLITE_UTF8, (void*)p, recoverPageIsUsed, 0, 0 - ); - } - if( rc==SQLITE_OK ){ - rc = sqlite3_create_function( - db, "read_i32", 2, SQLITE_UTF8, (void*)p, recoverReadI32, 0, 0 - ); - } - - if( rc!=SQLITE_OK ){ - if( p->errCode==SQLITE_OK ) rc = recoverDbError(p, db); - sqlite3_close(db); - }else{ - p->dbOut = db; - } + if( sqlite3_open_v2(p->zUri, &db, flags, 0) ){ + recoverDbError(p, db); + }else{ + char *zSql = recoverMPrintf(p, "ATTACH %Q AS recovery;", p->zStateDb); + recoverExec(p, db, zSql); + recoverExec(p, db, + "PRAGMA writable_schema = 1;" + "CREATE TABLE recovery.map(pgno INTEGER PRIMARY KEY, parent INT);" + "CREATE TABLE recovery.schema(type, name, tbl_name, rootpage, sql);" + ); + sqlite3_free(zSql); } - return rc; + + /* Register the sqlite_dbdata and sqlite_dbptr virtual table modules. + ** These two are registered with the output database handle - this + ** module depends on the input handle supporting the sqlite_dbpage + ** virtual table only. */ + if( p->errCode==SQLITE_OK ){ + p->errCode = sqlite3_dbdata_init(db, 0, 0); + } + + /* Register the custom user-functions with the output handle. */ + for(ii=0; p->errCode==SQLITE_OK && iierrCode = sqlite3_create_function(db, aFunc[ii].zName, + aFunc[ii].nArg, SQLITE_UTF8, (void*)p, aFunc[ii].xFunc, 0, 0 + ); + } + + /* Truncate the output database to 0 pages in size. This is done by + ** opening a new, empty, temp db, then using the backup API to clobber + ** any existing output db with a copy of it. */ + if( p->errCode==SQLITE_OK ){ + sqlite3 *db2 = 0; + int rc = sqlite3_open("", &db2); + if( rc==SQLITE_OK ){ + sqlite3_backup *pBackup = sqlite3_backup_init(db, "main", db2, "main"); + if( pBackup ){ + while( sqlite3_backup_step(pBackup, 1000)==SQLITE_OK ); + p->errCode = sqlite3_backup_finish(pBackup); + } + } + sqlite3_close(db2); + } + + p->dbOut = db; + return p->errCode; } static int recoverCacheSchema(sqlite3_recover *p){ @@ -632,22 +740,6 @@ static int recoverWriteSchema2(sqlite3_recover *p){ return p->errCode; } - -static char *recoverMPrintf(sqlite3_recover *p, const char *zFmt, ...){ - char *zRet = 0; - if( p->errCode==SQLITE_OK ){ - va_list ap; - char *z; - va_start(ap, zFmt); - zRet = sqlite3_vmprintf(zFmt, ap); - va_end(ap); - if( zRet==0 ){ - p->errCode = SQLITE_NOMEM; - } - } - return zRet; -} - static sqlite3_stmt *recoverInsertStmt( sqlite3_recover *p, RecoverTable *pTab, @@ -688,8 +780,8 @@ static sqlite3_stmt *recoverInsertStmt( zSql = recoverMPrintf(p, "%z%s%Q", zSql, zSep, pTab->aCol[ii].zCol); if( bSql ){ - zBind = recoverMPrintf( - p, "%z%squote(?%d)", zBind, zSqlSep, pTab->aCol[ii].iBind + zBind = recoverMPrintf(p, + "%z%sescape_crnl(quote(?%d))", zBind, zSqlSep, pTab->aCol[ii].iBind ); zSqlSep = "||', '||"; }else{ @@ -745,9 +837,9 @@ static char *recoverLostAndFoundCreate( for(ii=-1; zTbl==0 && p->errCode==SQLITE_OK && ii<1000; ii++){ int bFail = 0; if( ii<0 ){ - zTbl = recoverPrintf(p, "%s", p->zLostAndFound); + zTbl = recoverMPrintf(p, "%s", p->zLostAndFound); }else{ - zTbl = recoverPrintf(p, "%s_%d", p->zLostAndFound, ii); + zTbl = recoverMPrintf(p, "%s_%d", p->zLostAndFound, ii); } if( p->errCode==SQLITE_OK ){ @@ -777,7 +869,7 @@ static char *recoverLostAndFoundCreate( zSep = ", "; } - zSql = recoverPrintf(p, "CREATE TABLE %s(%s)", zTbl, zField); + zSql = recoverMPrintf(p, "CREATE TABLE %s(%s)", zTbl, zField); sqlite3_free(zField); recoverExec(p, p->dbOut, zSql); @@ -1217,14 +1309,14 @@ int sqlite3_recover_config(sqlite3_recover *p, int op, void *pArg){ switch( op ){ case SQLITE_RECOVER_TESTDB: sqlite3_free(p->zStateDb); - p->zStateDb = sqlite3_mprintf("%s", (char*)pArg); + p->zStateDb = recoverMPrintf(p, "%s", (char*)pArg); break; case SQLITE_RECOVER_LOST_AND_FOUND: const char *zArg = (const char*)pArg; sqlite3_free(p->zLostAndFound); if( zArg ){ - p->zLostAndFound = recoverPrintf(p, "%s", zArg); + p->zLostAndFound = recoverMPrintf(p, "%s", zArg); }else{ p->zLostAndFound = 0; } @@ -1247,21 +1339,47 @@ int sqlite3_recover_config(sqlite3_recover *p, int op, void *pArg){ } static void recoverStep(sqlite3_recover *p){ - + RecoverTable *pTab = 0; + RecoverTable *pNext = 0; + int rc = SQLITE_OK; assert( p->errCode==SQLITE_OK ); + recoverSqlCallback(p, "BEGIN"); recoverSqlCallback(p, "PRAGMA writable_schema = on"); - if( p->dbOut==0 ){ - if( recoverOpenOutput(p) ) return; - if( recoverCacheSchema(p) ) return; - if( recoverWriteSchema1(p) ) return; - if( recoverWriteData(p) ) return; - if( p->zLostAndFound && recoverLostAndFound(p) ) return; - if( recoverWriteSchema2(p) ) return; - } + /* Open the output database. And register required virtual tables and + ** user functions with the new handle. */ + recoverOpenOutput(p); + + /* Open transactions on both the input and output databases. */ + recoverExec(p, p->dbIn, "BEGIN"); + recoverExec(p, p->dbOut, "BEGIN"); + + recoverCacheSchema(p); + recoverWriteSchema1(p); + recoverWriteData(p); + if( p->zLostAndFound ) recoverLostAndFound(p); + recoverWriteSchema2(p); + + /* If no error has occurred, commit the write transaction on the output + ** database. Then end the read transaction on the input database, regardless + ** of whether or not prior errors have occurred. */ + recoverExec(p, p->dbOut, "COMMIT"); + rc = sqlite3_exec(p->dbIn, "END", 0, 0, 0); + if( p->errCode==SQLITE_OK ) p->errCode = rc; recoverSqlCallback(p, "PRAGMA writable_schema = off"); + recoverSqlCallback(p, "COMMIT"); + + for(pTab=p->pTblList; pTab; pTab=pNext){ + pNext = pTab->pNext; + sqlite3_free(pTab); + } + p->pTblList = 0; + + sqlite3_finalize(p->pGetPage); + sqlite3_close(p->dbOut); + p->pGetPage = 0; } int sqlite3_recover_step(sqlite3_recover *p){ @@ -1272,21 +1390,8 @@ int sqlite3_recover_step(sqlite3_recover *p){ } int sqlite3_recover_finish(sqlite3_recover *p){ - RecoverTable *pTab; - RecoverTable *pNext; - int rc; - - for(pTab=p->pTblList; pTab; pTab=pNext){ - pNext = pTab->pNext; - sqlite3_free(pTab); - } - - sqlite3_finalize(p->pGetPage); - rc = sqlite3_close(p->dbOut); - assert( rc==SQLITE_OK ); - p->pGetPage = 0; - rc = p->errCode; - + int rc = p->errCode; + sqlite3_free(p->zErrMsg); sqlite3_free(p->zStateDb); sqlite3_free(p->zLostAndFound); sqlite3_free(p); diff --git a/ext/recover/test_recover.c b/ext/recover/test_recover.c index acfcf8c7a3..1de99f4bd6 100644 --- a/ext/recover/test_recover.c +++ b/ext/recover/test_recover.c @@ -13,6 +13,7 @@ */ #include "sqlite3recover.h" +#include "sqliteInt.h" #include #include @@ -128,7 +129,7 @@ static int testRecoverCmd( int iVal = 0; if( Tcl_GetBooleanFromObj(interp, objv[3], &iVal) ) return TCL_ERROR; res = sqlite3_recover_config(pTest->p, - SQLITE_RECOVER_FREELIST_CORRUPT, (void*)iVal + SQLITE_RECOVER_FREELIST_CORRUPT, SQLITE_INT_TO_PTR(iVal) ); break; } @@ -136,7 +137,7 @@ static int testRecoverCmd( int iVal = 0; if( Tcl_GetBooleanFromObj(interp, objv[3], &iVal) ) return TCL_ERROR; res = sqlite3_recover_config(pTest->p, - SQLITE_RECOVER_ROWIDS, (void*)iVal + SQLITE_RECOVER_ROWIDS, SQLITE_INT_TO_PTR(iVal) ); break; } diff --git a/manifest b/manifest index a19ce57ac1..9e036d2201 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Tests\sand\sa\sfix\sfor\sthe\sSQLITE_RECOVER_ROWIDS\soption. -D 2022-09-06T19:38:06.927 +C Ensure\sthat\sthe\srecover\sextension\sproperly\sescapes\sCR\sand\sNL\scharacters\sin\stext\smode.\sAlso\sthat\sit\sholds\stransactions\sopen\son\sboth\sinput\sand\soutput\sdatabases\sfor\sthe\sduration\sof\sa\srecovery\soperation. +D 2022-09-07T16:41:33.248 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724 @@ -391,9 +391,9 @@ F ext/recover/recover1.test ae8ce9828210aa6c466bf88e23b0933849d5bb43091abe48cf2e F ext/recover/recover_common.tcl 6679af7dffc858e345053a91c9b0a897595b4a13007aceffafca75304ccb137c F ext/recover/recoverold.test f368a6ae2db12b6017257b332a19ab5df527f4061e43f12f5c85d8e2b236f074 F ext/recover/recoverrowid.test ec4436cd69e6cdacb48dd2963ff6dd9dbd5fe648376de5e7c0c2f4f6cbacb417 -F ext/recover/sqlite3recover.c 297fdef9da8523ef7fa3f88adb31356340826dac56c1fb13db2dd3e167806efe +F ext/recover/sqlite3recover.c 9724f913fd457f655e2873552bc6600a6aaff7104b9113ccb38fea18b6a71f03 F ext/recover/sqlite3recover.h 32f89b66f0235c0d94dfee0f1c3e9ed1ad726b3c4f3716ef0262b31cc33e8301 -F ext/recover/test_recover.c be0d74f0eba44fe7964e22d287dba0f3fa2baf197f630d51f0f9066af9b5eb2a +F ext/recover/test_recover.c 7aa268d3431d630eaa82ce14974ae04be50fe7feba660ffaea009cd581916d27 F ext/repair/README.md 92f5e8aae749a4dae14f02eea8e1bb42d4db2b6ce5e83dbcdd6b1446997e0c15 F ext/repair/checkfreelist.c e21f06995ff4efdc1622dcceaea4dcba2caa83ca2f31a1607b98a8509168a996 F ext/repair/checkindex.c 4383e4469c21e5b9ae321d0d63cec53e981af9d7a6564be6374f0eeb93dfc890 @@ -2006,8 +2006,8 @@ F vsixtest/vsixtest.tcl 6a9a6ab600c25a91a7acc6293828957a386a8a93 F vsixtest/vsixtest.vcxproj.data 2ed517e100c66dc455b492e1a33350c1b20fbcdc F vsixtest/vsixtest.vcxproj.filters 37e51ffedcdb064aad6ff33b6148725226cd608e F vsixtest/vsixtest_TemporaryKey.pfx e5b1b036facdb453873e7084e1cae9102ccc67a0 -P 09ec588d2fe24dd321e88318fe90a9ae912cbc73c8a2d59a10c821625dd12d9d -R 2900e793f9b609ad9d5a7b230af64f99 +P 1d5000f5718004110776ff58284d824854a93fe1cd1f6d8a4e8b8b0c2b2c3076 +R 3bf4abb672d881722cf6509f544c5091 U dan -Z a6081cceac978cdb593b5cd251ae6a63 +Z c71f206aa1a37ae4579417497726bb42 # Remove this line to create a well-formed Fossil manifest. diff --git a/manifest.uuid b/manifest.uuid index c0fbb47278..85ba19965b 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -1d5000f5718004110776ff58284d824854a93fe1cd1f6d8a4e8b8b0c2b2c3076 \ No newline at end of file +6cca8913e703635ad89415a60fc84000ac188d9df43f45594b8ad87facb91d54 \ No newline at end of file