From 15daa6b53ed3dd90b772deba903ab27ff790baf9 Mon Sep 17 00:00:00 2001 From: dan Date: Thu, 1 Feb 2018 19:41:23 +0000 Subject: [PATCH] Fix a problem triggered when a zipfile virtual table is created and written to within the same transaction. And add other zipfile test. FossilOrigin-Name: 48f1c556994d7f8f359c649a1da81eec02306106b68946a9a20b276742c4610d --- ext/misc/zipfile.c | 99 +++++++++++++++----------------- manifest | 20 +++---- manifest.uuid | 2 +- test/zipfile.test | 127 +++++++++++++++++++++++++++++++++++++++-- test/zipfile2.test | 38 ++++++++++++ test/zipfilefault.test | 54 +++++++++++++++++- 6 files changed, 270 insertions(+), 70 deletions(-) diff --git a/ext/misc/zipfile.c b/ext/misc/zipfile.c index dc2d831d37..441219565f 100644 --- a/ext/misc/zipfile.c +++ b/ext/misc/zipfile.c @@ -1073,7 +1073,8 @@ static int zipfileColumn( case 6: /* method */ sqlite3_result_int(ctx, pCDS->iCompression); break; - case 7: /* z */ + default: /* z */ + assert( i==7 ); sqlite3_result_int64(ctx, pCsr->iId); break; } @@ -1217,7 +1218,7 @@ static int zipfileFilter( ){ ZipfileTab *pTab = (ZipfileTab*)cur->pVtab; ZipfileCsr *pCsr = (ZipfileCsr*)cur; - const char *zFile; /* Zip file to scan */ + const char *zFile = 0; /* Zip file to scan */ int rc = SQLITE_OK; /* Return Code */ int bInMemory = 0; /* True for an in-memory zipfile */ @@ -1226,7 +1227,8 @@ static int zipfileFilter( if( pTab->zFile ){ zFile = pTab->zFile; }else if( idxNum==0 ){ - bInMemory = 1; + zipfileSetErrmsg(pCsr, "zipfile() function requires an argument"); + return SQLITE_ERROR; }else if( sqlite3_value_type(argv[0])==SQLITE_BLOB ){ const u8 *aBlob = (const u8*)sqlite3_value_blob(argv[0]); int nBlob = sqlite3_value_bytes(argv[0]); @@ -1295,14 +1297,11 @@ static int zipfileBestIndex( return SQLITE_OK; } -static ZipfileEntry *zipfileNewEntry(const char *zPath, int nData){ +static ZipfileEntry *zipfileNewEntry(const char *zPath){ ZipfileEntry *pNew; - pNew = sqlite3_malloc(sizeof(ZipfileEntry) + nData); + pNew = sqlite3_malloc(sizeof(ZipfileEntry)); if( pNew ){ memset(pNew, 0, sizeof(ZipfileEntry)); - if( nData ){ - pNew->aData = (u8*)&pNew[1]; - } pNew->cds.zFile = sqlite3_mprintf("%s", zPath); if( pNew->cds.zFile==0 ){ sqlite3_free(pNew); @@ -1358,6 +1357,7 @@ static int zipfileAppendEntry( nBuf = zipfileSerializeLFH(pEntry, aBuf); rc = zipfileAppendData(pTab, aBuf, nBuf); if( rc==SQLITE_OK ){ + pEntry->iDataOff = pTab->szCurrent; rc = zipfileAppendData(pTab, pData, nData); } @@ -1418,6 +1418,35 @@ static int zipfileComparePath(const char *zA, const char *zB, int nB){ return 1; } +static int zipfileBegin(sqlite3_vtab *pVtab){ + ZipfileTab *pTab = (ZipfileTab*)pVtab; + int rc = SQLITE_OK; + + assert( pTab->pWriteFd==0 ); + + /* Open a write fd on the file. Also load the entire central directory + ** structure into memory. During the transaction any new file data is + ** appended to the archive file, but the central directory is accumulated + ** in main-memory until the transaction is committed. */ + pTab->pWriteFd = fopen(pTab->zFile, "ab+"); + if( pTab->pWriteFd==0 ){ + pTab->base.zErrMsg = sqlite3_mprintf( + "zipfile: failed to open file %s for writing", pTab->zFile + ); + rc = SQLITE_ERROR; + }else{ + fseek(pTab->pWriteFd, 0, SEEK_END); + pTab->szCurrent = pTab->szOrig = (i64)ftell(pTab->pWriteFd); + rc = zipfileLoadDirectory(pTab, 0, 0); + } + + if( rc!=SQLITE_OK ){ + zipfileCleanupTransaction(pTab); + } + + return rc; +} + /* ** xUpdate method. */ @@ -1445,7 +1474,10 @@ static int zipfileUpdate( int bIsDir = 0; u32 iCrc32 = 0; - assert( (pTab->zFile==0)==(pTab->pWriteFd==0) ); + if( pTab->pWriteFd==0 ){ + rc = zipfileBegin(pVtab); + if( rc!=SQLITE_OK ) return rc; + } /* If this is a DELETE or UPDATE, find the archive entry to delete. */ if( sqlite3_value_type(apVal[0])!=SQLITE_NULL ){ @@ -1541,7 +1573,7 @@ static int zipfileUpdate( if( rc==SQLITE_OK ){ /* Create the new CDS record. */ - pNew = zipfileNewEntry(zPath, pTab->zFile ? 0 : (nData+1)); + pNew = zipfileNewEntry(zPath); if( pNew==0 ){ rc = SQLITE_NOMEM; }else{ @@ -1557,11 +1589,7 @@ static int zipfileUpdate( pNew->cds.iOffset = (u32)pTab->szCurrent; pNew->cds.nFile = nPath; pNew->mUnixTime = (u32)mTime; - if( pTab->zFile ){ - rc = zipfileAppendEntry(pTab, pNew, pData, nData); - }else{ - memcpy(pNew->aData, pData, nData); - } + rc = zipfileAppendEntry(pTab, pNew, pData, nData); zipfileAddEntry(pTab, pOld, pNew); } } @@ -1606,36 +1634,6 @@ static int zipfileAppendEOCD(ZipfileTab *pTab, ZipfileEOCD *p){ return zipfileAppendData(pTab, pTab->aBuffer, nBuf); } -static int zipfileBegin(sqlite3_vtab *pVtab){ - ZipfileTab *pTab = (ZipfileTab*)pVtab; - int rc = SQLITE_OK; - - assert( pTab->pWriteFd==0 ); - if( pTab->zFile ){ - /* Open a write fd on the file. Also load the entire central directory - ** structure into memory. During the transaction any new file data is - ** appended to the archive file, but the central directory is accumulated - ** in main-memory until the transaction is committed. */ - pTab->pWriteFd = fopen(pTab->zFile, "ab+"); - if( pTab->pWriteFd==0 ){ - pTab->base.zErrMsg = sqlite3_mprintf( - "zipfile: failed to open file %s for writing", pTab->zFile - ); - rc = SQLITE_ERROR; - }else{ - fseek(pTab->pWriteFd, 0, SEEK_END); - pTab->szCurrent = pTab->szOrig = (i64)ftell(pTab->pWriteFd); - rc = zipfileLoadDirectory(pTab, 0, 0); - } - - if( rc!=SQLITE_OK ){ - zipfileCleanupTransaction(pTab); - } - } - - return rc; -} - /* ** Serialize the CDS structure into buffer aBuf[]. Return the number ** of bytes written. @@ -1785,14 +1783,11 @@ static int zipfileFindFunction( void (**pxFunc)(sqlite3_context*,int,sqlite3_value**), /* OUT: Result */ void **ppArg /* OUT: User data for *pxFunc */ ){ - if( nArg>0 ){ - if( sqlite3_stricmp("zipfile_cds", zName)==0 ){ - *pxFunc = zipfileFunctionCds; - *ppArg = (void*)pVtab; - return 1; - } + if( sqlite3_stricmp("zipfile_cds", zName)==0 ){ + *pxFunc = zipfileFunctionCds; + *ppArg = (void*)pVtab; + return 1; } - return 0; } diff --git a/manifest b/manifest index d213885632..e4aa3f1840 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C New\sassert()\sstatements\sto\shelp\sensure\sthat\sno\sother\serrors\ssimilar\nto\s[343634942dd54ab57b7]\sever\sappear\sin\sthe\scode. -D 2018-02-01T15:57:00.046 +C Fix\sa\sproblem\striggered\swhen\sa\szipfile\svirtual\stable\sis\screated\sand\swritten\sto\nwithin\sthe\ssame\stransaction.\sAnd\sadd\sother\szipfile\stest. +D 2018-02-01T19:41:23.066 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea F Makefile.in 7a3f714b4fcf793108042b7b0a5c720b0b310ec84314d61ba7f3f49f27e550ea @@ -304,7 +304,7 @@ F ext/misc/vfsstat.c bf10ef0bc51e1ad6756629e1edb142f7a8db1178 F ext/misc/vtablog.c 31d0d8f4406795679dcd3a67917c213d3a2a5fb3ea5de35f6e773491ed7e13c9 F ext/misc/vtshim.c 1976e6dd68dd0d64508c91a6dfab8e75f8aaf6cd F ext/misc/wholenumber.c 784b12543d60702ebdd47da936e278aa03076212 -F ext/misc/zipfile.c 28f06977290c28361dcb7279016194c8e632c841e8586eb1520bbf494f487a5f +F ext/misc/zipfile.c 2856e99ce4b45669edbdc070649a7c6ea0348979d9297ab682f3c552078d47c7 F ext/rbu/rbu.c ea7d1b7eb44c123a2a619332e19fe5313500705c4a58aaa1887905c0d83ffc2e F ext/rbu/rbu1.test 43836fac8c7179a358eaf38a8a1ef3d6e6285842 F ext/rbu/rbu10.test 1846519a438697f45e9dcb246908af81b551c29e1078d0304fae83f1fed7e9ee @@ -1603,9 +1603,9 @@ F test/wordcount.c cb589cec469a1d90add05b1f8cee75c7210338d87a5afd65260ed5c0f4bbf F test/writecrash.test f1da7f7adfe8d7f09ea79b42e5ca6dcc41102f27f8e334ad71539501ddd910cc F test/zeroblob.test 3857870fe681b8185654414a9bccfde80b62a0fa F test/zerodamage.test 9c41628db7e8d9e8a0181e59ea5f189df311a9f6ce99cc376dc461f66db6f8dc -F test/zipfile.test 37cc584afebc6b64691a5df13deef0623f4bb42be21f1421930c34d9817ba1f2 -F test/zipfile2.test 5f93611307c131e83f226a471231d769b794b9e8c6a675cfa3d34b1a79df23fe -F test/zipfilefault.test e287d6783d95b7bbad3478701834a353675f48502f95d6d58e07d954b8278639 +F test/zipfile.test 1f066994bd77493c87e8ee4bd94db0651d180cff19ffbbe0b70085eb9a2cb34c +F test/zipfile2.test 67d5f08a202796d4b7a71dfa4b8dcb74aa7a9d1f42c5f17bedff9855c1ba7aa5 +F test/zipfilefault.test 73b08e3d0bbeb275e325ee7e3678ca98781de0737f9153ca23bde1f48a93d728 F tool/GetFile.cs a15e08acb5dd7539b75ba23501581d7c2b462cb5 F tool/GetTclKit.bat 8995df40c4209808b31f24de0b58f90930239a234f7591e3675d45bfbb990c5d F tool/Replace.cs 02c67258801c2fb5f63231e0ac0f220b4b36ba91 @@ -1704,7 +1704,7 @@ F vsixtest/vsixtest.tcl 6a9a6ab600c25a91a7acc6293828957a386a8a93 F vsixtest/vsixtest.vcxproj.data 2ed517e100c66dc455b492e1a33350c1b20fbcdc F vsixtest/vsixtest.vcxproj.filters 37e51ffedcdb064aad6ff33b6148725226cd608e F vsixtest/vsixtest_TemporaryKey.pfx e5b1b036facdb453873e7084e1cae9102ccc67a0 -P e6bb750697c3c7ceb5ce41d216e8ef6a1d556822a3b55e0a007b4a03e194a7d9 -R 296950b2c8b3ce7a358fc83523609b8d -U drh -Z f52624ceb0e0d6c17658803080172034 +P 5a70af1e9c567f12c997d25d0a305a8d42bf2cc92f2811e9d5fdde720665e213 +R ec6ac070bff5134b5a74e8c3afc721f5 +U dan +Z 34f839c0a19e1ff6432f6acef976c132 diff --git a/manifest.uuid b/manifest.uuid index d7e32f11f1..be5881914d 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -5a70af1e9c567f12c997d25d0a305a8d42bf2cc92f2811e9d5fdde720665e213 \ No newline at end of file +48f1c556994d7f8f359c649a1da81eec02306106b68946a9a20b276742c4610d \ No newline at end of file diff --git a/test/zipfile.test b/test/zipfile.test index bcfc4b2b1e..b31077d33c 100644 --- a/test/zipfile.test +++ b/test/zipfile.test @@ -151,10 +151,18 @@ do_catchsql_test 1.1.0.1 { INSERT INTO zz(name, mode, mtime, sz, rawdata, method) VALUES('f.txt', '-rw-r--r--', 1000000000, 5, 'abcde', 0); } {1 {constraint failed}} -do_catchsql_test 1.1.0.1 { - INSERT INTO zz(name, mtime, sz, rawdata, method) +do_catchsql_test 1.1.0.2 { + INSERT INTO zz(name, mtime, sz, data, method) VALUES('g.txt', 1000000002, 5, '12345', 0); } {1 {constraint failed}} +do_catchsql_test 1.1.0.3 { + INSERT INTO zz(name, mtime, rawdata, method) + VALUES('g.txt', 1000000002, '12345', 0); +} {1 {constraint failed}} +do_catchsql_test 1.1.0.4 { + INSERT INTO zz(name, data, method) + VALUES('g.txt', '12345', 7); +} {1 {constraint failed}} do_execsql_test 1.1.1 { INSERT INTO zz(name, mode, mtime, data, method) @@ -198,6 +206,9 @@ ifcapable json1 { h.txt 1 } } +do_catchsql_test 1.4.2 { + SELECT zipfile_cds(mode) FROM zipfile('test.zip'); +} {0 {{} {} {}}} do_execsql_test 1.5.1 { BEGIN; @@ -285,6 +296,37 @@ do_execsql_test 1.6.8 { i.txt 33188 4 zxcvb 0 } +do_execsql_test 1.6.8 { + UPDATE zz SET data = '' WHERE name='i.txt'; + SELECT name,mode,mtime,data,method from zipfile('test.zip'); +} { + blue.txt/ 16877 1000000000 {} 0 + h.txt 33189 1000000004 aaaaaaaaaabbbbbbbbbb 8 + i.txt 33188 4 {} 0 +} + +do_execsql_test 1.6.9 { + SELECT a.name, a.data + FROM zz AS a, zz AS b + WHERE a.name=+b.name AND +a.mode=b.mode +} { + blue.txt/ {} + h.txt aaaaaaaaaabbbbbbbbbb + i.txt {} +} + +do_execsql_test 1.6.10 { + SELECT name, data FROM zz WHERE name LIKE '%txt' +} { + h.txt aaaaaaaaaabbbbbbbbbb + i.txt {} +} + +do_execsql_test 1.7 { + DELETE FROM zz; + SELECT * FROM zz; +} {} + #------------------------------------------------------------------------- db close forcedelete test.zip @@ -379,6 +421,25 @@ do_catchsql_test 4.2 { CREATE VIRTUAL TABLE yyy USING zipfile('test.zip', 'test.zip'); } {1 {zipfile constructor requires one argument}} +do_catchsql_test 4.3 { + SELECT * FROM zipfile() +} {1 {zipfile() function requires an argument}} + +do_catchsql_test 4.4 { + SELECT * FROM zipfile('/path/that/does/not/exist') +} {1 {cannot open file: /path/that/does/not/exist}} + +foreach {tn mode} { + 1 abcd + 2 brwxrwxrwx + 3 lrwxrrxrwx +} { + do_catchsql_test 4.5.$tn { + WITH m(m) AS ( SELECT $mode) + SELECT zipfile('a.txt', m, 1000, 'xyz') FROM m + } [list 1 "zipfile: parse error in mode: $mode"] +} + #-------------------------------------------------------------------------- db func rt remove_timestamps @@ -448,16 +509,18 @@ ifcapable datetime { }] do_execsql_test 6.3 { - SELECT name, mtime, data FROM zipfile('test2.zip') + SELECT name, mtime, sz, rawdata, data FROM zipfile('test2.zip') } { - a.txt 946684800 abc - b.txt 1000000000 abc - c.txt 1111111000 abc + a.txt 946684800 3 abc abc + b.txt 1000000000 3 abc abc + c.txt 1111111000 3 abc abc } } } #------------------------------------------------------------------------- +# Force an IO error by truncating the zip archive to zero bytes in size +# while it is being read. forcedelete test.zip do_test 7.0 { execsql { @@ -476,6 +539,58 @@ do_test 7.0 { } msg] $msg } {1 {error in fread()}} +forcedelete test.zip +do_execsql_test 8.0.1 { + CREATE VIRTUAL TABLE zz USING zipfile('test.zip'); + BEGIN; + INSERT INTO zz(name, data) VALUES('a.txt', '1'); + INSERT INTO zz(name, data) VALUES('b.txt', '2'); + INSERT INTO zz(name, data) VALUES('c.txt', '1'); + INSERT INTO zz(name, data) VALUES('d.txt', '2'); + SELECT name, data FROM zz; +} { + a.txt 1 b.txt 2 c.txt 1 d.txt 2 +} +do_test 8.0.2 { + db eval { SELECT name, data FROM zz } { + if { $data=="2" } { db eval { DELETE FROM zz WHERE name=$name } } + } + execsql { SELECT name, data FROM zz } +} {a.txt 1 c.txt 1} +do_test 8.0.3 { + db eval { SELECT name, data FROM zz } { + db eval { DELETE FROM zz WHERE name=$name } + } + execsql { SELECT name, data FROM zz } +} {} +execsql COMMIT + +do_execsql_test 8.1.1 { + CREATE VIRTUAL TABLE nogood USING zipfile('test_unzip'); +} +do_catchsql_test 8.1.2 { + INSERT INTO nogood(name, data) VALUES('abc', 'def'); +} {1 {zipfile: failed to open file test_unzip for writing}} + +do_execsql_test 8.2.1 { + DROP TABLE nogood; + BEGIN; + CREATE VIRTUAL TABLE nogood USING zipfile('test_unzip'); +} +do_catchsql_test 8.2.2 { + INSERT INTO nogood(name, data) VALUES('abc', 'def'); +} {1 {zipfile: failed to open file test_unzip for writing}} +do_execsql_test 8.2.3 { + COMMIT; +} + +forcedelete test.zip +do_execsql_test 8.3.1 { + BEGIN; + CREATE VIRTUAL TABLE ok USING zipfile('test.zip'); + INSERT INTO ok(name, data) VALUES ('sqlite3', 'elf'); + COMMIT; +} finish_test diff --git a/test/zipfile2.test b/test/zipfile2.test index a6c3e7e3c4..f3509d0a54 100644 --- a/test/zipfile2.test +++ b/test/zipfile2.test @@ -164,6 +164,44 @@ do_catchsql_test 4.1 { SELECT name,mtime,data,method FROM zipfile($blob) } {1 {inflate() failed (0)}} +# Check the response to an unknown compression method (set data to NULL). +set blob [blob [string map {0800 0900} $archive2]] +do_execsql_test 4.2 { + SELECT name,mtime,data IS NULL,method FROM zipfile($blob) +} {a.txt 1000000 1 9} + +# Corrupt the EOCDS signature bytes in various ways. +foreach {tn sub} { + 1 {504B0500} + 2 {504B0006} + 3 {50000506} + 4 {004B0506} +} { + set blob [blob [string map [list 504B0506 $sub] $archive2]] + do_catchsql_test 4.3.$tn { + SELECT * FROM zipfile($blob) + } {1 {cannot find end of central directory record}} +} + +#------------------------------------------------------------------------- +# Test that a zero-length file with a '/' at the end is treated as +# a directory (data IS NULL). Even if the mode doesn't indicate +# that it is a directory. + +do_test 5.0 { + set blob [db one { + WITH c(n, d) AS ( + SELECT 'notadir', '' + ) + SELECT zipfile(n, d) FROM c + }] + + set hex [binary encode hex $blob] + set hex [string map {6e6f7461646972 6e6f746164692f} $hex] + set blob2 [binary decode hex $hex] + + execsql { SELECT name, data IS NULL FROM zipfile($blob2) } +} {notadi/ 1} finish_test diff --git a/test/zipfilefault.test b/test/zipfilefault.test index b2c6d7cfc4..a850b5146a 100644 --- a/test/zipfilefault.test +++ b/test/zipfilefault.test @@ -42,11 +42,19 @@ do_execsql_test 2.0 { INSERT INTO setup(name, data) VALUES('a.txt', '1234567890'); } -do_faultsim_test 2 -faults oom* -body { +do_faultsim_test 2.1 -faults oom* -body { execsql { SELECT name,data FROM zipfile('test.zip') } } -test { faultsim_test_result {0 {a.txt 1234567890}} } +do_faultsim_test 2.2 -faults oom* -body { + execsql { + SELECT json_extract( zipfile_cds(z), '$.version-made-by' ) + FROM zipfile('test.zip') + } +} -test { + faultsim_test_result {0 798} +} forcedelete test.zip reset_db @@ -75,6 +83,50 @@ do_faultsim_test 4 -faults oom* -body { faultsim_test_result {0 {1 aaaaaaaaaaabbbbbbbbbbaaaaaaaaaabbbbbbbbbb}} } +reset_db +load_static_extension db zipfile + +do_execsql_test 5.0 { + CREATE VIRTUAL TABLE setup USING zipfile('test.zip') +} + +do_faultsim_test 5.1 -faults oom* -prep { + forcedelete test.zip +} -body { + execsql { + INSERT INTO setup(name, data) + VALUES('a.txt', 'aaaaaaaaaaaaaaaaaaaaaaaaaaaa'); + } +} -test { + faultsim_test_result {0 {}} +} + +do_faultsim_test 5.2 -faults oom* -prep { + forcedelete test.zip +} -body { + execsql { + INSERT INTO setup(name, data) VALUES('dir', NULL) + } +} -test { + faultsim_test_result {0 {}} +} + +do_faultsim_test 5.3 -faults oom* -prep { + forcedelete test.zip + execsql { + DROP TABLE IF EXISTS setup; + BEGIN; + CREATE VIRTUAL TABLE setup USING zipfile('test.zip') + } +} -body { + execsql { + INSERT INTO setup(name, data) VALUES('dir', NULL) + } +} -test { + catchsql { COMMIT } + faultsim_test_result {0 {}} +} + finish_test