From: stephan Date: Sat, 26 Aug 2023 19:34:49 +0000 (+0000) Subject: Correct a string length misuse in JNI sqlite3_result_error() in an OOM case. Unrelate... X-Git-Tag: version-3.44.0~274 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=6428cd18d61279701eac1d39dbf0b4e8fccf90eb;p=thirdparty%2Fsqlite.git Correct a string length misuse in JNI sqlite3_result_error() in an OOM case. Unrelated minor JNI cleanups. FossilOrigin-Name: 4252f56f3d8574b7b43306440726daf3b5f5500d5d9105784b2f82753e7c71dd --- diff --git a/ext/jni/src/c/sqlite3-jni.c b/ext/jni/src/c/sqlite3-jni.c index 93cb975af7..bc776ae166 100644 --- a/ext/jni/src/c/sqlite3-jni.c +++ b/ext/jni/src/c/sqlite3-jni.c @@ -669,14 +669,16 @@ static void s3jni_incr( volatile unsigned int * const p ){ SJG.envCache.locker = 0; \ sqlite3_mutex_leave( SJG.envCache.mutex ) +#define S3JniMutex_S3JniDb_assertLocker \ + assert( (env) == SJG.perDb.locker && "Misuse of S3JniGlobal.perDb.mutex" ) #define S3JniMutex_S3JniDb_enter \ sqlite3_mutex_enter( SJG.perDb.mutex ); \ - assert( 0==SJG.perDb.locker ); \ + assert( 0==SJG.perDb.locker && "Misuse of S3JniGlobal.perDb.mutex" ); \ s3jni_incr( &SJG.metrics.nMutexPerDb ); \ SJG.perDb.locker = env; #define S3JniMutex_S3JniDb_leave \ - assert( env == SJG.perDb.locker ); \ - SJG.perDb.locker = 0; \ + assert( env == SJG.perDb.locker && "Misuse of S3JniGlobal.perDb.mutex" ); \ + SJG.perDb.locker = 0; \ sqlite3_mutex_leave( SJG.perDb.mutex ) #else /* SQLITE_THREADSAFE==0 */ @@ -1016,19 +1018,18 @@ static void S3JniHook_unref(JNIEnv * const env, S3JniHook * const s, /* ** Internal helper for many hook callback impls. Locks the S3JniDb -** mutex, makes a copy of src into dest, with one change if src->jObj +** mutex, makes a copy of src into dest, with one change: if src->jObj ** is not NULL then dest->jObj will be a new LOCAL ref to src->jObj -** instead of a copy of the prior GLOBAL ref. Then unlocks the +** instead of a copy of the prior GLOBAL ref. Then it unlocks the ** mutex. If dest->jObj is not NULL when this returns then the caller ** is obligated to eventually free the new ref by passing dest->jObj ** to S3JniUnrefLocal(). The dest pointer must NOT be passed to ** S3JniHook_unref(), as that one assumes that dest->jObj is a GLOBAL -** ref. +** ref (it's illegal to try to unref the wrong ref type).. ** ** Background: when running a hook we need a call-local copy lest ** another thread modify the hook while we're running it. That copy -** has to haves its own Java reference, but it need only be -** call-local. +** has to have its own Java reference, but it need only be call-local. */ static void S3JniHook_localdup( JNIEnv * const env, S3JniHook const * const src, S3JniHook * const dest ){ @@ -1039,13 +1040,12 @@ static void S3JniHook_localdup( JNIEnv * const env, S3JniHook const * const src, } /* -** Clears s's state and moves it to the free-list. +** Clears s's state and moves it to the free-list. Requires that that the +** caller has locked S3JniGlobal.perDb.mutex. */ -static void S3JniDb_set_aside_unlocked(JNIEnv * env, S3JniDb * const s){ +static void S3JniDb_set_aside(JNIEnv * env, S3JniDb * const s){ if( s ){ -#if SQLITE_THREADSAFE - assert( S3JniGlobal.perDb.locker == env ); -#endif + S3JniMutex_S3JniDb_enter; assert(s->pPrev != s); assert(s->pNext != s); assert(s->pPrev ? (s->pPrev!=s->pNext) : 1); @@ -1075,12 +1075,6 @@ static void S3JniDb_set_aside_unlocked(JNIEnv * env, S3JniDb * const s){ s->pNext = SJG.perDb.aFree; if(s->pNext) s->pNext->pPrev = s; SJG.perDb.aFree = s; - } -} -static void S3JniDb_set_aside(JNIEnv * env, S3JniDb * const s){ - if( s ){ - S3JniMutex_S3JniDb_enter; - S3JniDb_set_aside_unlocked(env, s); S3JniMutex_S3JniDb_leave; } } @@ -1168,7 +1162,8 @@ static jfieldID NativePointerHolder_field(JNIEnv * const env, S3NphRef const* pR if( !pNC->fidValue ){ S3JniMutex_Nph_enter; if( !pNC->fidValue ){ - pNC->fidValue = (*env)->GetFieldID(env, pNC->klazz, "nativePointer", "J"); + pNC->fidValue = (*env)->GetFieldID(env, pNC->klazz, + pRef->zMember, pRef->zTypeSig); S3JniExceptionIsFatal("Code maintenance required: missing nativePointer field."); } S3JniMutex_Nph_leave; @@ -1254,6 +1249,10 @@ static S3JniDb * S3JniDb_alloc(JNIEnv * const env, jobject jDb){ ** a Java handle. In normal usage, the 2nd argument is a Java-side sqlite3 ** object, from which the db is fished out. ** +** If the lockMutex argument is true then the S3JniDb mutex is locked +** before starting work, else the caller is required to have locked +** it. +** ** Returns NULL if jDb and pDb are both NULL or if there is no ** matching S3JniDb entry for pDb or the pointer fished out of jDb. */ @@ -2226,9 +2225,8 @@ S3JniApi(sqlite3_cancel_auto_extension(),jboolean,1cancel_1auto_1extension)( /* Wrapper for sqlite3_close(_v2)(). */ static jint s3jni_close_db(JNIEnv * const env, jobject jDb, int version){ int rc = 0; - S3JniDb * ps = 0; + S3JniDb * const ps = S3JniDb_from_java(jDb); assert(version == 1 || version == 2); - ps = S3JniDb_from_java(jDb); if( ps ){ rc = 1==version ? (jint)sqlite3_close(ps->pDb) : (jint)sqlite3_close_v2(ps->pDb); if( 0==rc ){ @@ -2282,6 +2280,7 @@ static void s3jni_collation_needed_impl16(void *pState, sqlite3 *pDb, ps->hooks.collationNeeded.midCallback, ps->jDb, (jint)eTextRep, jName); S3JniIfThrew{ + S3JniExceptionWarnCallbackThrew("sqlite3_collation_needed() callback"); s3jni_db_exception(env, ps, 0, "sqlite3_collation_needed() callback threw"); } S3JniUnrefLocal(jName); @@ -2394,9 +2393,9 @@ static int s3jni_commit_rollback_hook_impl(int isCommit, int rc = 0; S3JniHook hook; - S3JniHook_localdup( env, - isCommit ? &ps->hooks.commit : &ps->hooks.rollback, - &hook); + S3JniHook_localdup( env, isCommit + ? &ps->hooks.commit : &ps->hooks.rollback, + &hook); if( hook.jObj ){ rc = isCommit ? (int)(*env)->CallIntMethod(env, hook.jObj, hook.midCallback) @@ -3495,8 +3494,7 @@ S3JniApi(sqlite3_result_double(),void,1result_1double)( } S3JniApi(sqlite3_result_error(),void,1result_1error)( - JniArgsEnvClass, jobject jpCx, jbyteArray baMsg, - int eTextRep + JniArgsEnvClass, jobject jpCx, jbyteArray baMsg, int eTextRep ){ const char * zUnspecified = "Unspecified error."; jsize const baLen = (*env)->GetArrayLength(env, baMsg); @@ -3504,12 +3502,12 @@ S3JniApi(sqlite3_result_error(),void,1result_1error)( switch( pjBuf ? eTextRep : SQLITE_UTF8 ){ case SQLITE_UTF8: { const char *zMsg = pjBuf ? (const char *)pjBuf : zUnspecified; - sqlite3_result_error(PtrGet_sqlite3_context(jpCx), zMsg, (int)baLen); + int const n = pjBuf ? (int)baLen : (int)sqlite3Strlen30(zMsg); + sqlite3_result_error(PtrGet_sqlite3_context(jpCx), zMsg, n); break; } case SQLITE_UTF16: { - const void *zMsg = pjBuf - ? (const void *)pjBuf : (const void *)zUnspecified; + const void *zMsg = pjBuf; sqlite3_result_error16(PtrGet_sqlite3_context(jpCx), zMsg, (int)baLen); break; } @@ -3678,13 +3676,12 @@ S3JniApi(sqlite3_set_authorizer(),jint,1set_1authorizer)( ")I"); S3JniUnrefLocal(klazz); S3JniIfThrew { - S3JniHook_unref(env, pHook, 0); rc = s3jni_db_error(ps->pDb, SQLITE_ERROR, "Error setting up Java parts of authorizer hook."); }else{ rc = sqlite3_set_authorizer(ps->pDb, s3jni_xAuth, ps); - if( rc ) S3JniHook_unref(env, pHook, 0); } + if( rc ) S3JniHook_unref(env, pHook, 0); } S3JniMutex_S3JniDb_leave; return rc; diff --git a/ext/jni/src/org/sqlite/jni/SQLite3Jni.java b/ext/jni/src/org/sqlite/jni/SQLite3Jni.java index 4c13286fe0..598fa2a016 100644 --- a/ext/jni/src/org/sqlite/jni/SQLite3Jni.java +++ b/ext/jni/src/org/sqlite/jni/SQLite3Jni.java @@ -587,7 +587,8 @@ public final class SQLite3Jni { public static native String sqlite3_errstr(int resultCode); /** - Note that the offset values assume UTF-8-encoded SQL. + Note that the returned byte offset values assume UTF-8-encoded + inputs, so won't always match character offsets in Java Strings. */ public static native int sqlite3_error_offset(@NotNull sqlite3 db); @@ -595,23 +596,8 @@ public final class SQLite3Jni { public static native int sqlite3_initialize(); - /** - Design note/FIXME: we have a problem vis-a-vis 'synchronized' - here: we specifically want other threads to be able to cancel a - long-running thread, but this routine requires access to C-side - global state which does not have a mutex. Making this function - synchronized would make it impossible for a long-running job to - be cancelled from another thread. - - The mutexing problem here is not within the core lib or Java, but - within the cached data held by the JNI binding. The cache holds - per-thread state, used by all but a tiny fraction of the JNI - binding layer, and access to that state needs to be - mutex-protected. - */ public static native void sqlite3_interrupt(@NotNull sqlite3 db); - //! See sqlite3_interrupt() for threading concerns. public static native boolean sqlite3_is_interrupted(@NotNull sqlite3 db); public static native long sqlite3_last_insert_rowid(@NotNull sqlite3 db); @@ -907,7 +893,7 @@ public final class SQLite3Jni { a complaint about the invalid argument. */ private static native void sqlite3_result_error( - @NotNull sqlite3_context cx, @Nullable byte[] msg, + @NotNull sqlite3_context cx, @NotNull byte[] msg, int eTextRep ); @@ -920,12 +906,12 @@ public final class SQLite3Jni { public static void sqlite3_result_error( @NotNull sqlite3_context cx, @NotNull String msg ){ - final byte[] utf8 = (msg+"\0").getBytes(StandardCharsets.UTF_8); + final byte[] utf8 = msg.getBytes(StandardCharsets.UTF_8); sqlite3_result_error(cx, utf8, SQLITE_UTF8); } public static void sqlite3_result_error16( - @NotNull sqlite3_context cx, @Nullable byte[] utf16 + @NotNull sqlite3_context cx, @NotNull byte[] utf16 ){ sqlite3_result_error(cx, utf16, SQLITE_UTF16); } @@ -933,7 +919,7 @@ public final class SQLite3Jni { public static void sqlite3_result_error16( @NotNull sqlite3_context cx, @NotNull String msg ){ - final byte[] utf8 = (msg+"\0").getBytes(StandardCharsets.UTF_16); + final byte[] utf8 = msg.getBytes(StandardCharsets.UTF_16); sqlite3_result_error(cx, utf8, SQLITE_UTF16); } diff --git a/manifest b/manifest index 69941ee6c3..e504f7eaa7 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Remove\sa\sbunch\sof\scommented-out\sdebug\soutput. -D 2023-08-26T18:15:33.351 +C Correct\sa\sstring\slength\smisuse\sin\sJNI\ssqlite3_result_error()\sin\san\sOOM\scase.\sUnrelated\sminor\sJNI\scleanups. +D 2023-08-26T19:34:49.574 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724 @@ -236,7 +236,7 @@ F ext/icu/sqliteicu.h fa373836ed5a1ee7478bdf8a1650689294e41d0c89c1daab26e9ae78a3 F ext/jni/GNUmakefile d9244b5addf58868343a74a94faa71f829e7f40c163486d053f4b4bbea173703 F ext/jni/README.md 1332b1fa27918bd5d9ca2d0d4f3ac3a6ab86b9e3699dc5bfe32904a027f3d2a9 F ext/jni/jar-dist.make 030aaa4ae71dd86e4ec5e7c1e6cd86f9dfa47c4592c070d2e35157e42498e1fa -F ext/jni/src/c/sqlite3-jni.c 5ccdea9487e66f0850aaa817ffe8523a84c4376235ae967c3700513665c0883a +F ext/jni/src/c/sqlite3-jni.c 14ac371890c91b15eb0f996fe5bcc9f30dde996983845a5e32a147826b81f72d F ext/jni/src/c/sqlite3-jni.h 22c6c760a31ebfc3fe13d45d2a3a4dd7c8f9c6207aeba3fdc38137452cbf3a04 F ext/jni/src/org/sqlite/jni/AggregateFunction.java e0aac6ccae05702f8ee779820570866a2760aaa57a73135c57c8d3580bef52d5 F ext/jni/src/org/sqlite/jni/AuthorizerCallback.java c374bb76409cce7a0bdba94877706b59ac6127fa5d9e6af3e8058c99ce99c030 @@ -262,7 +262,7 @@ F ext/jni/src/org/sqlite/jni/ResultCode.java ba701f20213a5f259e94cfbfdd36eb7ac7c F ext/jni/src/org/sqlite/jni/RollbackHookCallback.java be7f7a26d1102fb514d835e11198d51302af8053d97188bfb2e34c2133208568 F ext/jni/src/org/sqlite/jni/SQLFunction.java d060f302b2cc4cf7a4f5a6b2d36458a2e6fc9648374b5d09c36a43665af41207 F ext/jni/src/org/sqlite/jni/SQLite3CallbackProxy.java 13c4ea6f35871261eba63fa4117715515e0beecbdebfb879ec5b1f340ed36904 -F ext/jni/src/org/sqlite/jni/SQLite3Jni.java 663600b216653850bfe4e2c00cfc8da12db94b3f36c4b79f6a7d1bde6ad0b484 +F ext/jni/src/org/sqlite/jni/SQLite3Jni.java 77a8f965c87af3913839ee12666f6feed6c97fd981c55dadcad8f069aad89c00 F ext/jni/src/org/sqlite/jni/ScalarFunction.java 21301a947e49f0dd9c682dfe2cc8a6518226c837253dd791cd512f847eeca52c F ext/jni/src/org/sqlite/jni/Tester1.java 37b46dc15ac8fbeb916dcf1f7771023d2be025d05422d725d5891935eda506ac F ext/jni/src/org/sqlite/jni/TesterFts5.java 6f135c60e24c89e8eecb9fe61dde0f3bb2906de668ca6c9186bcf34bdaf94629 @@ -2103,8 +2103,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 49d3be002ce5e594027f47a3ba448f0c21ec68b416b8df997497753f53e3ca52 -R a3f75122b9068beff86867201ce6d194 +P b49488481e2952294960bb0ee971f6eca126c19d68ef92152894aa28393e6865 +R 9cb44f7d9077da136013e6794a7e1baf U stephan -Z 6e844ab70f4837c2de7f262b42dd7747 +Z b123384a6d9af143d63c67db0e5f0b49 # Remove this line to create a well-formed Fossil manifest. diff --git a/manifest.uuid b/manifest.uuid index ea4bb054c3..90c4134155 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -b49488481e2952294960bb0ee971f6eca126c19d68ef92152894aa28393e6865 \ No newline at end of file +4252f56f3d8574b7b43306440726daf3b5f5500d5d9105784b2f82753e7c71dd \ No newline at end of file