From e8fa8c9649b16bd8d0f7acdfd92d4762d07e004e Mon Sep 17 00:00:00 2001 From: dan Date: Sat, 27 Sep 2014 16:33:09 +0000 Subject: [PATCH] Fix a segfault in the streaming API functions triggered by a very long table name. FossilOrigin-Name: d2642543eed54da1ac0f757d43dd4d72482eb752 --- ext/session/session1.test | 26 ++++++++++ ext/session/sessionfault.test | 38 ++++++++++++++- ext/session/sqlite3session.c | 92 ++++++++++++++++------------------- ext/session/test_session.c | 17 +++++-- manifest | 18 +++---- manifest.uuid | 2 +- 6 files changed, 127 insertions(+), 66 deletions(-) diff --git a/ext/session/session1.test b/ext/session/session1.test index e6f9c8fd7f..1c853a97a1 100644 --- a/ext/session/session1.test +++ b/ext/session/session1.test @@ -524,4 +524,30 @@ do_test 9.2 { do_changeset_test 9.2 S {{UPDATE t7 0 ....X.. {{} {} i 1 {} {} i 1 i 1 {} {} {} {}} {{} {} i 2 {} {} i 2 {} {} {} {} {} {}}}} S delete catch { db2 close } + +#------------------------------------------------------------------------- +# Test a really long table name. +# +reset_db +set tblname [string repeat tblname123 100] +do_test 10.1.1 { + execsql " + CREATE TABLE $tblname (a PRIMARY KEY, b); + INSERT INTO $tblname VALUES('xyz', 'def'); + " + sqlite3session S db main + S attach $tblname + execsql " + INSERT INTO $tblname VALUES('uvw', 'abc'); + DELETE FROM $tblname WHERE a = 'xyz'; + " +} {} +breakpoint +do_changeset_test 10.1.2 S " + {INSERT $tblname 0 X. {} {t uvw t abc}} + {DELETE $tblname 0 X. {t xyz t def} {}} +" +do_test 10.1.4 { S delete } {} + + finish_test diff --git a/ext/session/sessionfault.test b/ext/session/sessionfault.test index 4b278098a9..c94589e372 100644 --- a/ext/session/sessionfault.test +++ b/ext/session/sessionfault.test @@ -30,7 +30,6 @@ do_common_sql { faultsim_save_and_close db2 close - #------------------------------------------------------------------------- # Test OOM error handling when collecting and applying a simple changeset. # @@ -234,7 +233,7 @@ set changeset [changeset_from_sql { }] db close -do_faultsim_test 5 -faults oom* -body { +do_faultsim_test 5.1 -faults oom* -body { set ::inverse [sqlite3changeset_invert $::changeset] set {} {} } -test { @@ -251,6 +250,41 @@ do_faultsim_test 5 -faults oom* -body { } } +catch {db close} +catch {db2 close} +forcedelete test.db +sqlite3 db test.db +execsql { + CREATE TABLE t2(a PRIMARY KEY, b); + INSERT INTO t2 VALUES(1, 'abc'); + INSERT INTO t2 VALUES(2, 'def'); +} +set changeset [changeset_from_sql { + UPDATE t2 SET b = (b || b || b || b); + UPDATE t2 SET b = (b || b || b || b); + UPDATE t2 SET b = (b || b || b || b); + UPDATE t2 SET b = (b || b || b || b); +}] +db close +set abc [string repeat abc 256] +set def [string repeat def 256] + +do_faultsim_test 5.2 -faults oom-tra* -body { + set ::inverse [sqlite3changeset_invert $::changeset] + set {} {} +} -test { + faultsim_test_result {0 {}} {1 SQLITE_NOMEM} + if {$testrc==0} { + set x [list] + sqlite3session_foreach c $::inverse { lappend x $c } + foreach c " + {UPDATE t2 0 X. {i 1 t $::abc} {{} {} t abc}} + {UPDATE t2 0 X. {i 2 t $::def} {{} {} t def}} + " { lappend y $c } + if {$x != $y} { error "changeset no good" } + } +} + #------------------------------------------------------------------------- # Test that OOM errors in sqlite3changeset_concat() are handled correctly. # diff --git a/ext/session/sqlite3session.c b/ext/session/sqlite3session.c index eb1ff1e774..7e69be4cc4 100644 --- a/ext/session/sqlite3session.c +++ b/ext/session/sqlite3session.c @@ -18,7 +18,7 @@ typedef struct SessionInput SessionInput; ** Minimum chunk size used by streaming versions of functions. */ #ifdef SQLITE_TEST -#define SESSIONS_STR_CHUNK_SIZE 1 +#define SESSIONS_STR_CHUNK_SIZE 64 #else #define SESSIONS_STR_CHUNK_SIZE 1024 #endif @@ -1347,7 +1347,7 @@ static void sessionAppendValue(SessionBuffer *p, sqlite3_value *pVal, int *pRc){ int rc = *pRc; if( rc==SQLITE_OK ){ int nByte = 0; - sessionSerializeValue(0, pVal, &nByte); + rc = sessionSerializeValue(0, pVal, &nByte); sessionBufferGrow(p, nByte, &rc); if( rc==SQLITE_OK ){ rc = sessionSerializeValue(&p->aBuf[p->nBuf], pVal, 0); @@ -2225,12 +2225,10 @@ static int sessionReadRecord( eType = pIn->aData[pIn->iNext++]; } - assert( !apOut || apOut[i]==0 ); + assert( apOut[i]==0 ); if( eType ){ - if( apOut ){ - apOut[i] = sqlite3ValueNew(0); - if( !apOut[i] ) rc = SQLITE_NOMEM; - } + apOut[i] = sqlite3ValueNew(0); + if( !apOut[i] ) rc = SQLITE_NOMEM; } if( rc==SQLITE_OK ){ @@ -2239,22 +2237,20 @@ static int sessionReadRecord( int nByte; pIn->iNext += sessionVarintGet(aVal, &nByte); rc = sessionInputBuffer(pIn, nByte); - if( apOut && rc==SQLITE_OK ){ + if( rc==SQLITE_OK ){ u8 enc = (eType==SQLITE_TEXT ? SQLITE_UTF8 : 0); rc = sessionValueSetStr(apOut[i],&pIn->aData[pIn->iNext],nByte,enc); } pIn->iNext += nByte; } if( eType==SQLITE_INTEGER || eType==SQLITE_FLOAT ){ - if( apOut ){ - sqlite3_int64 v = sessionGetI64(aVal); - if( eType==SQLITE_INTEGER ){ - sqlite3VdbeMemSetInt64(apOut[i], v); - }else{ - double d; - memcpy(&d, &v, 8); - sqlite3VdbeMemSetDouble(apOut[i], d); - } + sqlite3_int64 v = sessionGetI64(aVal); + if( eType==SQLITE_INTEGER ){ + sqlite3VdbeMemSetInt64(apOut[i], v); + }else{ + double d; + memcpy(&d, &v, 8); + sqlite3VdbeMemSetDouble(apOut[i], d); } pIn->iNext += 8; } @@ -2293,10 +2289,10 @@ static int sessionChangesetBufferTblhdr(SessionInput *pIn, int *pnByte){ while( (pIn->iNext + nRead)nData && pIn->aData[pIn->iNext + nRead] ){ nRead++; } - if( pIn->aData[pIn->iNext + nRead]==0 ) break; + if( (pIn->iNext + nRead)nData ) break; rc = sessionInputBuffer(pIn, nRead + 100); } - if( pnByte ) *pnByte = nRead+1; + *pnByte = nRead+1; return rc; } @@ -2775,7 +2771,7 @@ static int sessionChangesetInvert( /* Write the new old.* record. Consists of the PK columns from the ** original old.* record, and the other values from the original ** new.* record. */ - for(iCol=0; rc==SQLITE_OK && iColpNext){ - int i; - if( pTab->nEntry==0 ) continue; + for(pTab=pList; rc==SQLITE_OK && pTab; pTab=pTab->pNext){ + int i; + if( pTab->nEntry==0 ) continue; - sessionAppendTableHdr(&buf, bPatch, pTab, &rc); - for(i=0; inChange; i++){ - SessionChange *p; - for(p=pTab->apChange[i]; p; p=p->pNext){ - sessionAppendByte(&buf, p->op, &rc); - sessionAppendByte(&buf, p->bIndirect, &rc); - sessionAppendBlob(&buf, p->aRecord, p->nRecord, &rc); - } - } - - if( rc==SQLITE_OK && xOutput && buf.nBuf>=SESSIONS_STR_CHUNK_SIZE ){ - rc = xOutput(pOut, buf.aBuf, buf.nBuf); - buf.nBuf = 0; + sessionAppendTableHdr(&buf, bPatch, pTab, &rc); + for(i=0; inChange; i++){ + SessionChange *p; + for(p=pTab->apChange[i]; p; p=p->pNext){ + sessionAppendByte(&buf, p->op, &rc); + sessionAppendByte(&buf, p->bIndirect, &rc); + sessionAppendBlob(&buf, p->aRecord, p->nRecord, &rc); } } - if( rc==SQLITE_OK ){ - if( xOutput ){ - if( buf.nBuf>0 ) rc = xOutput(pOut, buf.aBuf, buf.nBuf); - }else{ - *ppOut = buf.aBuf; - *pnOut = buf.nBuf; - buf.aBuf = 0; - } + if( rc==SQLITE_OK && xOutput && buf.nBuf>=SESSIONS_STR_CHUNK_SIZE ){ + rc = xOutput(pOut, buf.aBuf, buf.nBuf); + buf.nBuf = 0; } - sqlite3_free(buf.aBuf); } + if( rc==SQLITE_OK ){ + if( xOutput ){ + if( buf.nBuf>0 ) rc = xOutput(pOut, buf.aBuf, buf.nBuf); + }else{ + *ppOut = buf.aBuf; + *pnOut = buf.nBuf; + buf.aBuf = 0; + } + } + sqlite3_free(buf.aBuf); + sessionDeleteTable(pList); return rc; } diff --git a/ext/session/test_session.c b/ext/session/test_session.c index df690d44ad..7e366ef3bb 100644 --- a/ext/session/test_session.c +++ b/ext/session/test_session.c @@ -75,7 +75,7 @@ struct TestSessionsBlob { }; typedef struct TestSessionsBlob TestSessionsBlob; -static int testSessionsOutput( +static int testStreamOutput( void *pCtx, const void *pData, int nData @@ -160,9 +160,9 @@ static int test_session_cmd( if( test_tcl_integer(interp, SESSION_STREAM_TCL_VAR) ){ void *pCtx = (void*)&o; if( iSub==7 ){ - rc = sqlite3session_patchset_str(pSession, testSessionsOutput, pCtx); + rc = sqlite3session_patchset_str(pSession, testStreamOutput, pCtx); }else{ - rc = sqlite3session_changeset_str(pSession, testSessionsOutput, pCtx); + rc = sqlite3session_changeset_str(pSession, testStreamOutput, pCtx); } }else{ if( iSub==7 ){ @@ -562,6 +562,13 @@ static int testStreamInput( int nRem = p->nData - p->iData; /* Bytes of data available */ int nRet = p->nStream; /* Bytes actually returned */ + /* Allocate and free some space. There is no point to this, other than + ** that it allows the regular OOM fault-injection tests to cause an error + ** in this function. */ + void *pAlloc = sqlite3_malloc(10); + if( pAlloc==0 ) return SQLITE_NOMEM; + sqlite3_free(pAlloc); + if( nRet>nReq ) nRet = nReq; if( nRet>nRem ) nRet = nRem; @@ -691,7 +698,7 @@ static int test_sqlite3changeset_invert( if( sIn.nStream ){ rc = sqlite3changeset_invert_str( - testStreamInput, (void*)&sIn, testSessionsOutput, (void*)&sOut + testStreamInput, (void*)&sIn, testStreamOutput, (void*)&sOut ); }else{ rc = sqlite3changeset_invert(sIn.nData, sIn.aData, &sOut.n, &sOut.p); @@ -736,7 +743,7 @@ static int test_sqlite3changeset_concat( rc = sqlite3changeset_concat_str( testStreamInput, (void*)&sLeft, testStreamInput, (void*)&sRight, - testSessionsOutput, (void*)&sOut + testStreamOutput, (void*)&sOut ); }else{ rc = sqlite3changeset_concat( diff --git a/manifest b/manifest index 258fb598ba..4cb1bee75c 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Improve\ssessions\smodule\sdocumentation\sand\scomments.\sFix\ssome\sother\scode\sissues. -D 2014-09-27T12:26:18.848 +C Fix\sa\ssegfault\sin\sthe\sstreaming\sAPI\sfunctions\striggered\sby\sa\svery\slong\stable\sname. +D 2014-09-27T16:33:09.345 F Makefile.arm-wince-mingw32ce-gcc d6df77f1f48d690bd73162294bbba7f59507c72f F Makefile.in dd5f245aa8c741bc65845747203c8ce2f3fb6c83 F Makefile.linux-gcc 91d710bdc4998cb015f39edf3cb314ec4f4d7e23 @@ -146,7 +146,7 @@ F ext/rtree/sqlite3rtree.h 83349d519fe5f518b3ea025d18dd1fe51b1684bd F ext/rtree/tkt3363.test 142ab96eded44a3615ec79fba98c7bde7d0f96de F ext/rtree/viewrtree.tcl eea6224b3553599ae665b239bd827e182b466024 F ext/session/changeset.c 4ccbaa4531944c24584bf6a61ba3a39c62b6267a -F ext/session/session1.test 3733d71eb9a99b14d51fa5219d5b8a82407c3be8 +F ext/session/session1.test 4653867f32a98ce4bbb4a181aac6debe51ca4dfb F ext/session/session2.test 99ca0da7ddb617d42bafd83adccf99f18ae0384b F ext/session/session3.test a7a9ce59b8d1e49e2cc23d81421ac485be0eea01 F ext/session/session4.test a6ed685da7a5293c5d6f99855bcf41dbc352ca84 @@ -157,10 +157,10 @@ F ext/session/session9.test 776e46785c29c11cda01f5205d0f1e8f8f9a46bf F ext/session/sessionA.test eb05c13e4ef1ca8046a3a6dbf2d5f6f5b04a11d4 F ext/session/sessionB.test d4ac901b43d4922a17dff08bbaa2f5354487ce4d F ext/session/session_common.tcl 1539d8973b2aea0025c133eb0cc4c89fcef541a5 -F ext/session/sessionfault.test e7965159a73d385c1a4af12d82c3a039ebdd71a6 -F ext/session/sqlite3session.c 9ce77f4752cd5d8982e7b64f665ae66754dda723 +F ext/session/sessionfault.test cf9758bfb6ccd5db5f09f170667a8cea1ac8afa7 +F ext/session/sqlite3session.c 228cca8fdb6fd07942be097c516f22e39ec39e3c F ext/session/sqlite3session.h b57009fb88835cc4684376bd3eae0d6ab364968a -F ext/session/test_session.c 194083ee1f0f6f38404f662fe9b50849abd3b7ee +F ext/session/test_session.c 56a4ddd443b571c8d365b03197c032bdaed1443a F ext/userauth/sqlite3userauth.h 19cb6f0e31316d0ee4afdfb7a85ef9da3333a220 F ext/userauth/user-auth.txt e6641021a9210364665fe625d067617d03f27b04 F ext/userauth/userauth.c 5fa3bdb492f481bbc1709fc83c91ebd13460c69e @@ -1216,7 +1216,7 @@ F tool/vdbe_profile.tcl 67746953071a9f8f2f668b73fe899074e2c6d8c1 F tool/warnings-clang.sh f6aa929dc20ef1f856af04a730772f59283631d4 F tool/warnings.sh 0abfd78ceb09b7f7c27c688c8e3fe93268a13b32 F tool/win/sqlite.vsix deb315d026cc8400325c5863eef847784a219a2f -P 4d8537eafb40e3687abc057ba26a1f7014f2c2d9 -R c2c5a501f3f294cbca1bcb47eb806551 +P bfc8bd80f8b225cebc66478448510ce84223ae7d +R 9ca53892e63f9c31475266a6b3e34d34 U dan -Z c7ab4f2784fbdfc318b000ddf7671e24 +Z f7759b84adfee5a6c178fa61db492ce5 diff --git a/manifest.uuid b/manifest.uuid index 31da46d10b..903686ec1f 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -bfc8bd80f8b225cebc66478448510ce84223ae7d \ No newline at end of file +d2642543eed54da1ac0f757d43dd4d72482eb752 \ No newline at end of file