From: stephan <stephan@noemail.net>
Date: Thu, 10 Aug 2023 12:36:40 +0000 (+0000)
Subject: Resolve two assertions in the auto-extension JNI which were triggered via new SQLTest... 
X-Git-Tag: version-3.43.0~47^2~18
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=152801470332bccbde78d0f086e761287716e9d1;p=thirdparty%2Fsqlite.git

Resolve two assertions in the auto-extension JNI which were triggered via new SQLTester infrastructure. Move SQLTester's db-init SQL injection into an auto-extension.

FossilOrigin-Name: 2952906c30bc2b7987f2c39837d56bd121f5817dc094e6ccdb6d4eea5e9b8d17
---

diff --git a/ext/jni/src/c/sqlite3-jni.c b/ext/jni/src/c/sqlite3-jni.c
index 51576771de..0987d5f846 100644
--- a/ext/jni/src/c/sqlite3-jni.c
+++ b/ext/jni/src/c/sqlite3-jni.c
@@ -1877,13 +1877,14 @@ WRAP_INT_SVALUE(1value_1type,          sqlite3_value_type)
 
 #if S3JNI_ENABLE_AUTOEXT
 /* Central auto-extension handler. */
+FIXME_THREADING
 static int s3jni_auto_extension(sqlite3 *pDb, const char **pzErr,
                                 const struct sqlite3_api_routines *ignored){
   S3JniAutoExtension const * pAX = S3JniGlobal.autoExt.pHead;
   int rc;
   JNIEnv * env = 0;
   S3JniDb * const ps = S3JniGlobal.autoExt.psOpening;
-
+  //MARKER(("auto-extension on open()ing ps@%p db@%p\n", ps, pDb));
   S3JniGlobal.autoExt.psOpening = 0;
   if( !pAX ){
     assert( 0==S3JniGlobal.autoExt.isRunning );
@@ -2089,6 +2090,7 @@ static jint s3jni_close_db(JNIEnv * const env, jobject jDb, int version){
   assert(version == 1 || version == 2);
   ps = S3JniDb_for_db(env, jDb, 0, 0);
   if(ps){
+    //MARKER(("close()ing db@%p\n", ps->pDb));
     rc = 1==version ? (jint)sqlite3_close(ps->pDb) : (jint)sqlite3_close_v2(ps->pDb);
     S3JniDb_set_aside(ps)
       /* MUST come after close() because of ps->trace. */;
@@ -2566,6 +2568,7 @@ static int s3jni_open_pre(JNIEnv * const env, S3JniEnvCache **jc,
     S3JniGlobal.autoExt.psOpening = *ps;
   }
 #endif
+  //MARKER(("pre-open ps@%p\n", *ps));
   return *ps ? 0 : SQLITE_NOMEM;
 }
 
@@ -2582,14 +2585,17 @@ static int s3jni_open_pre(JNIEnv * const env, S3JniEnvCache **jc,
 */
 static int s3jni_open_post(JNIEnv * const env, S3JniDb * ps,
                            sqlite3 **ppDb, jobject jOut, int theRc){
+  //MARKER(("post-open() ps@%p db@%p\n", ps, *ppDb));
 #if S3JNI_ENABLE_AUTOEXT
-  assert( S3JniGlobal.autoExt.pHead ? 0==S3JniGlobal.autoExt.psOpening : 1 );
+  assert( S3JniGlobal.autoExt.pHead ? ps!=S3JniGlobal.autoExt.psOpening : 1 );
   S3JniGlobal.autoExt.psOpening = 0;
 #endif
   if(*ppDb){
     assert(ps->jDb);
 #if S3JNI_ENABLE_AUTOEXT
-    assert( S3JniGlobal.autoExt.pHead ? *ppDb==ps->pDb : 0==ps->pDb );
+    //MARKER(("*autoExt.pHead=%p, ppDb=%p, ps->pDb=%p\n", S3JniGlobal.autoExt.pHead, *ppDb, ps->pDb));
+    // invalid when an autoext triggers another open():
+    // assert( S3JniGlobal.autoExt.pHead ? *ppDb==ps->pDb : 0==ps->pDb );
 #endif
     ps->pDb = *ppDb;
     NativePointerHolder_set(env, ps->jDb, *ppDb, S3JniClassNames.sqlite3);
@@ -2607,13 +2613,17 @@ JDECL(jint,1open)(JENV_CSELF, jstring strName, jobject jOut){
   jobject jDb = 0;
   S3JniDb * ps = 0;
   S3JniEnvCache * jc = 0;
+  S3JniDb * const prevOpening = S3JniGlobal.autoExt.psOpening;
   int rc = s3jni_open_pre(env, &jc, strName, &zName, &ps, &jDb);
-  if( rc ) return rc;
-  rc = sqlite3_open(zName, &pOut);
-  //MARKER(("env=%p, *env=%p\n", env, *env));
-  rc = s3jni_open_post(env, ps, &pOut, jOut, rc);
-  assert(rc==0 ? pOut!=0 : 1);
-  sqlite3_free(zName);
+  if( 0==rc ){
+    rc = sqlite3_open(zName, &pOut);
+    //MARKER(("env=%p, *env=%p\n", env, *env));
+    //MARKER(("open() ps@%p db@%p\n", ps, pOut));
+    rc = s3jni_open_post(env, ps, &pOut, jOut, rc);
+    assert(rc==0 ? pOut!=0 : 1);
+    sqlite3_free(zName);
+  }
+  S3JniGlobal.autoExt.psOpening = prevOpening;
   return (jint)rc;
 }
 
@@ -2625,6 +2635,7 @@ JDECL(jint,1open_1v2)(JENV_CSELF, jstring strName,
   S3JniDb * ps = 0;
   S3JniEnvCache * jc = 0;
   char *zVfs = 0;
+  S3JniDb * const prevOpening = S3JniGlobal.autoExt.psOpening;
   int rc = s3jni_open_pre(env, &jc, strName, &zName, &ps, &jDb);
   if( 0==rc && strVfs ){
     zVfs = s3jni_jstring_to_utf8(jc, strVfs, 0);
@@ -2635,12 +2646,14 @@ JDECL(jint,1open_1v2)(JENV_CSELF, jstring strName,
   if( 0==rc ){
     rc = sqlite3_open_v2(zName, &pOut, (int)flags, zVfs);
   }
+  //MARKER(("open_v2() ps@%p db@%p\n", ps, pOut));
   /*MARKER(("zName=%s, zVfs=%s, pOut=%p, flags=%d, nrc=%d\n",
     zName, zVfs, pOut, (int)flags, nrc));*/
   rc = s3jni_open_post(env, ps, &pOut, jOut, rc);
   assert(rc==0 ? pOut!=0 : 1);
   sqlite3_free(zName);
   sqlite3_free(zVfs);
+  S3JniGlobal.autoExt.psOpening = prevOpening;
   return (jint)rc;
 }
 
diff --git a/ext/jni/src/org/sqlite/jni/AutoExtension.java b/ext/jni/src/org/sqlite/jni/AutoExtension.java
index 3a58b6589f..a2ab6a0f75 100644
--- a/ext/jni/src/org/sqlite/jni/AutoExtension.java
+++ b/ext/jni/src/org/sqlite/jni/AutoExtension.java
@@ -24,6 +24,8 @@ public interface AutoExtension {
      As an exception (as it were) to the callbacks-must-not-throw
      rule, AutoExtensions may do so and the exception's error message
      will be set as the db's error string.
+
+     Results are undefined if db is closed by an auto-extension.
   */
   int xEntryPoint(sqlite3 db);
 }
diff --git a/ext/jni/src/org/sqlite/jni/SQLite3Jni.java b/ext/jni/src/org/sqlite/jni/SQLite3Jni.java
index 182515e72f..3ffb771ec7 100644
--- a/ext/jni/src/org/sqlite/jni/SQLite3Jni.java
+++ b/ext/jni/src/org/sqlite/jni/SQLite3Jni.java
@@ -188,22 +188,25 @@ public final class SQLite3Jni {
   */
   public static synchronized native int sqlite3_auto_extension(@NotNull AutoExtension callback);
 
-  public static int sqlite3_bind_blob(@NotNull sqlite3_stmt stmt, int ndx,
-                                      @Nullable byte[] data){
+  public static int sqlite3_bind_blob(
+    @NotNull sqlite3_stmt stmt, int ndx, @Nullable byte[] data
+  ){
     return (null == data)
       ? sqlite3_bind_null(stmt, ndx)
       : sqlite3_bind_blob(stmt, ndx, data, data.length);
   }
 
-  private static native int sqlite3_bind_blob(@NotNull sqlite3_stmt stmt,
-                                              int ndx, @Nullable byte[] data,
-                                              int n);
+  private static native int sqlite3_bind_blob(
+    @NotNull sqlite3_stmt stmt, int ndx, @Nullable byte[] data, int n
+  );
 
-  public static native int sqlite3_bind_double(@NotNull sqlite3_stmt stmt,
-                                               int ndx, double v);
+  public static native int sqlite3_bind_double(
+    @NotNull sqlite3_stmt stmt, int ndx, double v
+  );
 
-  public static native int sqlite3_bind_int(@NotNull sqlite3_stmt stmt,
-                                            int ndx, int v);
+  public static native int sqlite3_bind_int(
+    @NotNull sqlite3_stmt stmt, int ndx, int v
+  );
 
   public static native int sqlite3_bind_int64(@NotNull sqlite3_stmt stmt,
                                               int ndx, long v);
@@ -257,10 +260,10 @@ public final class SQLite3Jni {
      to clear the busy handler. Calling this multiple times with the
      same object is a no-op on the second and subsequent calls.
   */
-  public static native int sqlite3_busy_handler(@NotNull sqlite3 db,
+  public static native synchronized int sqlite3_busy_handler(@NotNull sqlite3 db,
                                                 @Nullable BusyHandler handler);
 
-  public static native int sqlite3_busy_timeout(@NotNull sqlite3 db, int ms);
+  public static native synchronized int sqlite3_busy_timeout(@NotNull sqlite3 db, int ms);
 
   /**
      Works like the C API except that it returns false, without side
@@ -489,12 +492,12 @@ public final class SQLite3Jni {
      or sqlite3_open_v2() so that they have a predictible object to
      pass to, e.g., the sqlite3_collation_needed() callback.
   */
-  public static native int sqlite3_open(@Nullable String filename,
-                                        @NotNull OutputPointer.sqlite3 ppDb);
+  public static native synchronized int sqlite3_open(@Nullable String filename,
+                                                     @NotNull OutputPointer.sqlite3 ppDb);
 
-  public static native int sqlite3_open_v2(@Nullable String filename,
-                                           @NotNull OutputPointer.sqlite3 ppDb,
-                                           int flags, @Nullable String zVfs);
+  public static native synchronized int sqlite3_open_v2(@Nullable String filename,
+                                                        @NotNull OutputPointer.sqlite3 ppDb,
+                                                        int flags, @Nullable String zVfs);
 
   /**
      The sqlite3_prepare() family of functions require slightly
diff --git a/ext/jni/src/org/sqlite/jni/tester/SQLTester.java b/ext/jni/src/org/sqlite/jni/tester/SQLTester.java
index 6930b32a10..eabbb4580b 100644
--- a/ext/jni/src/org/sqlite/jni/tester/SQLTester.java
+++ b/ext/jni/src/org/sqlite/jni/tester/SQLTester.java
@@ -227,7 +227,9 @@ public class SQLTester {
     //verbose("Added file ",filename);
   }
 
-  private void setupInitialDb() throws Exception {
+  private void setupInitialDb() throws DbException {
+    outln("setupInitialDb()");
+    closeDb(0);
     Util.unlink(initialDbName);
     openDb(0, initialDbName, true);
   }
@@ -245,7 +247,6 @@ public class SQLTester {
   public void runTests() throws Exception {
     for(String f : listInFiles){
       reset();
-      setupInitialDb();
       ++nTestFile;
       final TestScript ts = new TestScript(f);
       outln(nextStartEmoji(), " starting [",f,"]");
@@ -289,13 +290,14 @@ public class SQLTester {
     if(addNL) resultBuffer.append('\n');
   }
 
-  void appendDbInitSql(String n) throws SQLTesterException {
+  void appendDbInitSql(String n) throws DbException {
     dbInitSql.append(n).append('\n');
     if( null!=getCurrentDb() ){
       //outln("RUNNING DB INIT CODE: ",n);
       execSql(null, true, ResultBufferMode.NONE, null, n);
     }
   }
+  String getDbInitSql(){ return dbInitSql.toString(); }
 
   String getInputText(){ return inputBuffer.toString(); }
 
@@ -330,7 +332,7 @@ public class SQLTester {
     return affirmDbId(id).aDb[id];
   }
 
-  void closeDb(int id) throws Exception{
+  void closeDb(int id) {
     final sqlite3 db = affirmDbId(id).aDb[id];
     if( null != db ){
       sqlite3_close_v2(db);
@@ -338,7 +340,7 @@ public class SQLTester {
     }
   }
 
-  void closeDb() throws Exception { closeDb(iCurrentDb); }
+  void closeDb() { closeDb(iCurrentDb); }
 
   void closeAllDbs(){
     for(int i = 0; i<aDb.length; ++i){
@@ -347,7 +349,7 @@ public class SQLTester {
     }
   }
 
-  sqlite3 openDb(String name, boolean createIfNeeded) throws Exception {
+  sqlite3 openDb(String name, boolean createIfNeeded) throws DbException {
     closeDb();
     int flags = SQLITE_OPEN_READWRITE;
     if( createIfNeeded ) flags |= SQLITE_OPEN_CREATE;
@@ -365,7 +367,7 @@ public class SQLTester {
     return aDb[iCurrentDb] = db;
   }
 
-  sqlite3 openDb(int slot, String name, boolean createIfNeeded) throws Exception {
+  sqlite3 openDb(int slot, String name, boolean createIfNeeded) throws DbException {
     affirmDbId(slot);
     iCurrentDb = slot;
     return openDb(name, createIfNeeded);
@@ -384,6 +386,7 @@ public class SQLTester {
     nullView = "nil";
     emitColNames = false;
     iCurrentDb = 0;
+    dbInitSql.append("SELECT 1;");
   }
 
   void setNullValue(String v){nullView = v;}
@@ -459,12 +462,19 @@ public class SQLTester {
 
      appendMode specifies how/whether to append results to the result
      buffer. lineMode specifies whether to output all results in a
-     single line or one line per row.
+     single line or one line per row. If appendMode is
+     ResultBufferMode.NONE then lineMode is ignored and may be null.
   */
   public int execSql(sqlite3 db, boolean throwOnError,
-                     ResultBufferMode appendMode,
-                     ResultRowMode lineMode,
+                     ResultBufferMode appendMode, ResultRowMode lineMode,
                      String sql) throws SQLTesterException {
+    if( null==db && null==aDb[0] ){
+      // Delay opening of the initial db to enable tests to change its
+      // name and inject on-connect code via, e.g., the MEMDB
+      // directive.  this setup as the potential to misinteract with
+      // auto-extension timing and must be done carefully.
+      setupInitialDb();
+    }
     final OutputPointer.Int32 oTail = new OutputPointer.Int32();
     final OutputPointer.sqlite3_stmt outStmt = new OutputPointer.sqlite3_stmt();
     final byte[] sqlUtf8 = sql.getBytes(StandardCharsets.UTF_8);
@@ -568,7 +578,6 @@ public class SQLTester {
   public static void main(String[] argv) throws Exception{
     installCustomExtensions();
     final SQLTester t = new SQLTester();
-    boolean v2 = false;
     for(String a : argv){
       if(a.startsWith("-")){
         final String flag = a.replaceFirst("-+","");
@@ -582,9 +591,22 @@ public class SQLTester {
       }
       t.addTestScript(a);
     }
+    final AutoExtension ax = new AutoExtension() {
+        private final SQLTester tester = t;
+        public int xEntryPoint(sqlite3 db){
+          tester.outln("AutoExtension running db init code on ",db);
+          final String init = tester.getDbInitSql();
+          if( !init.isEmpty() ){
+            tester.execSql(db, true, ResultBufferMode.NONE, null, init);
+          }
+          return 0;
+        }
+      };
+    sqlite3_auto_extension(ax);
     try {
       t.runTests();
     }finally{
+      sqlite3_cancel_auto_extension(ax);
       t.outln("Processed ",t.nTotalTest," test(s) in ",t.nTestFile," file(s).");
       if( t.nAbortedScript > 0 ){
         t.outln("Aborted ",t.nAbortedScript," script(s).");
diff --git a/manifest b/manifest
index 31949ac767..8ea514fc24 100644
--- a/manifest
+++ b/manifest
@@ -1,5 +1,5 @@
-C Make\stest\scompletion\sstatus\smore\svisible\sat\sa\sglance\son\smodern\sterminals.
-D 2023-08-10T11:15:20.741
+C Resolve\stwo\sassertions\sin\sthe\sauto-extension\sJNI\swhich\swere\striggered\svia\snew\sSQLTester\sinfrastructure.\sMove\sSQLTester's\sdb-init\sSQL\sinjection\sinto\san\sauto-extension.
+D 2023-08-10T12:36:40.522
 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1
 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea
 F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724
@@ -233,10 +233,10 @@ F ext/icu/icu.c c074519b46baa484bb5396c7e01e051034da8884bad1a1cb7f09bbe6be3f0282
 F ext/icu/sqliteicu.h fa373836ed5a1ee7478bdf8a1650689294e41d0c89c1daab26e9ae78a32075a8
 F ext/jni/GNUmakefile d69b26fb294b7a86a2f838012f4161311c06d607680b86ecdb1334f6f78c165c
 F ext/jni/README.md e965674505e105626127ad45e628e4d19fcd379cdafc4d23c814c1ac2c55681d
-F ext/jni/src/c/sqlite3-jni.c 3fda1e271054835adec606b4dbaaa6db5ae120b59a72308e4b906739c0a27a87
+F ext/jni/src/c/sqlite3-jni.c 12ac735b074d681694ecdbb27d99c66273ea21b8f1047b659b34581bd129a118
 F ext/jni/src/c/sqlite3-jni.h b19a104e0566440af566366cea72188bd994a96ba85c3f196acaa6f4a4609a55
 F ext/jni/src/org/sqlite/jni/Authorizer.java 1308988f7f40579ea0e4deeaec3c6be971630566bd021c31367fe3f5140db892
-F ext/jni/src/org/sqlite/jni/AutoExtension.java 3409ad8954d6466bf772e6be9379e0e337312b446b668287062845755a16844d
+F ext/jni/src/org/sqlite/jni/AutoExtension.java 18e83f6f463e306df60b2dceb65247d32af1f78af4bbbae9155411a8c6cdb093
 F ext/jni/src/org/sqlite/jni/BusyHandler.java 1b1d3e5c86cd796a0580c81b6af6550ad943baa25e47ada0dcca3aff3ebe978c
 F ext/jni/src/org/sqlite/jni/Collation.java 8dffbb00938007ad0967b2ab424d3c908413af1bbd3d212b9c9899910f1218d1
 F ext/jni/src/org/sqlite/jni/CollationNeeded.java ad67843b6dd1c06b6b0a1dc72887b7c48e2a98042fcf6cacf14d42444037eab8
@@ -253,7 +253,7 @@ F ext/jni/src/org/sqlite/jni/ProgressHandler.java 6f62053a828a572de809828b1ee495
 F ext/jni/src/org/sqlite/jni/ResultCode.java 7cdf993f2037ab7bd244c9a34dbaef2ace3beb5da5d7e7fda5c6f67634ceb647
 F ext/jni/src/org/sqlite/jni/RollbackHook.java b04c8abcc6ade44a8a57129e33765793f69df0ba909e49ba18d73f4268d92564
 F ext/jni/src/org/sqlite/jni/SQLFunction.java 09ce81c1c637e31c3a830d4c859cce95d65f5e02ff45f8bd1985b3479381bc46
-F ext/jni/src/org/sqlite/jni/SQLite3Jni.java 578fb62013bbffefc0c37afcbf07dddd290b32147fe6f0d995bc55d044c714ce
+F ext/jni/src/org/sqlite/jni/SQLite3Jni.java 1291facf4e681a740c5cc8086ffa8a47d2b73f1faf660dab29c3527d6fba70f1
 F ext/jni/src/org/sqlite/jni/Tester1.java 22dca3ab0d93951382230f71e3cfb65898b80f12704a018c8ab9062df609b4fe
 F ext/jni/src/org/sqlite/jni/TesterFts5.java cf2d687baafffdeba219b77cf611fd47a0556248820ea794ae3e8259bfbdc5ee
 F ext/jni/src/org/sqlite/jni/Tracer.java a5cece9f947b0af27669b8baec300b6dd7ff859c3e6a6e4a1bd8b50f9714775d
@@ -266,7 +266,7 @@ F ext/jni/src/org/sqlite/jni/sqlite3.java 62b1b81935ccf3393472d17cb883dc5ff39c38
 F ext/jni/src/org/sqlite/jni/sqlite3_context.java d26573fc7b309228cb49786e9078597d96232257defa955a3425d10897bca810
 F ext/jni/src/org/sqlite/jni/sqlite3_stmt.java 78e6d1b95ac600a9475e9db4623f69449322b0c93d1bd4e1616e76ed547ed9fc
 F ext/jni/src/org/sqlite/jni/sqlite3_value.java 3d1d4903e267bc0bc81d57d21f5e85978eff389a1a6ed46726dbe75f85e6914a
-F ext/jni/src/org/sqlite/jni/tester/SQLTester.java 24a1099dc1dde1508de44172b2438ba8f585bbc591fe2389171ce8647663a2b2
+F ext/jni/src/org/sqlite/jni/tester/SQLTester.java e0b6c9c24176f6541baded998f7dda4328937d18c894f6d0bff36dec0c279ff2
 F ext/jni/src/org/sqlite/jni/tester/test-script-interpreter.md f9f25126127045d051e918fe59004a1485311c50a13edbf18c79a6ff9160030e
 F ext/jni/src/tests/000-000-sanity.test cfe6dc1b950751d6096e3f5695becaadcdaa048bfe9567209d6eb676e693366d
 F ext/jni/src/tests/000-001-ignored.test e17e874c6ab3c437f1293d88093cf06286083b65bf162317f91bbfd92f961b70
@@ -2089,8 +2089,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 ddc534cb25b59faf18a860a51f2dd41a1a73963aeb541b9553301fe784608393
-R 86c85827eee124ab2195331beb10cb02
+P a4e96c306c4c270f417243e7923d7e6c4f860528dd67990dfd8d9768a6c4873f
+R e78d561aec5cdd8a539cc739fbc7cee6
 U stephan
-Z a074cfea0973f3e7d868cadec52b5a31
+Z e3784a9c8c2c97335fd71d5cd2d6fc99
 # Remove this line to create a well-formed Fossil manifest.
diff --git a/manifest.uuid b/manifest.uuid
index 15a49cb7ed..8136499772 100644
--- a/manifest.uuid
+++ b/manifest.uuid
@@ -1 +1 @@
-a4e96c306c4c270f417243e7923d7e6c4f860528dd67990dfd8d9768a6c4873f
\ No newline at end of file
+2952906c30bc2b7987f2c39837d56bd121f5817dc094e6ccdb6d4eea5e9b8d17
\ No newline at end of file