From: stephan Date: Wed, 15 Nov 2023 08:29:42 +0000 (+0000) Subject: JNI: clear out the sqlite3_context native pointer after calling UDF callbacks which... X-Git-Tag: version-3.45.0~153 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=bd06d8672dc2afb5549c18cf62c5e6a0fde65eef;p=thirdparty%2Fsqlite.git JNI: clear out the sqlite3_context native pointer after calling UDF callbacks which do not have an argv (as was already done for those which have an argv). Add related tests and code commentary. FossilOrigin-Name: 138f40543b26b2e02e27d830d92e30b12cfef5a8dc3f0b58b39c68e1b3c91cc6 --- diff --git a/ext/jni/README.md b/ext/jni/README.md index e3ed0435a4..fc7b5f7611 100644 --- a/ext/jni/README.md +++ b/ext/jni/README.md @@ -16,9 +16,8 @@ Technical support is available in the forum: > **FOREWARNING:** this subproject is very much in development and subject to any number of changes. Please do not rely on any information about its API until this disclaimer is removed. The JNI - bindings released with version 3.43 are a "tech preview" and 3.44 - will be "final," at which point strong backward compatibility - guarantees will apply. + bindings released with version 3.43 are a "tech preview." Once + finalized, strong backward compatibility guarantees will apply. Project goals/requirements: @@ -43,11 +42,13 @@ Non-goals: - Creation of high-level OO wrapper APIs. Clients are free to create them off of the C-style API. +- Virtual tables are unlikely to be supported due to the amount of + glue code needed to fit them into Java. + - Support for mixed-mode operation, where client code accesses SQLite both via the Java-side API and the C API via their own native - code. In such cases, proxy functionalities (primarily callback - handler wrappers of all sorts) may fail because the C-side use of - the SQLite APIs will bypass those proxies. + code. Such cases would be a minefield of potential mis-interactions + between this project's JNI bindings and mixed-mode client code. Hello World diff --git a/ext/jni/src/c/sqlite3-jni.c b/ext/jni/src/c/sqlite3-jni.c index aa0c15c12d..14c447acd5 100644 --- a/ext/jni/src/c/sqlite3-jni.c +++ b/ext/jni/src/c/sqlite3-jni.c @@ -1995,13 +1995,16 @@ error_oom: /* ** Requires that jCx and jArgv are sqlite3_context -** resp. array-of-sqlite3_value values initialized by udf_args(). This +** resp. array-of-sqlite3_value values initialized by udf_args(). The +** latter will be 0-and-NULL for UDF types with no arguments. This ** function zeroes out the nativePointer member of jCx and each entry ** in jArgv. This is a safety-net precaution to avoid undefined -** behavior if a Java-side UDF holds a reference to one of its -** arguments. This MUST be called from any function which successfully -** calls udf_args(), after calling the corresponding UDF and checking -** its exception status. It MUST NOT be called in any other case. +** behavior if a Java-side UDF holds a reference to its context or one +** of its arguments. This MUST be called from any function which +** successfully calls udf_args(), after calling the corresponding UDF +** and checking its exception status, or which Java-wraps a +** sqlite3_context for use with a UDF(ish) call. It MUST NOT be called +** in any other case. */ static void udf_unargs(JNIEnv *env, jobject jCx, int argc, jobjectArray jArgv){ int i = 0; @@ -2009,8 +2012,29 @@ static void udf_unargs(JNIEnv *env, jobject jCx, int argc, jobjectArray jArgv){ NativePointerHolder_set(S3JniNph(sqlite3_context), jCx, 0); for( ; i < argc; ++i ){ jobject jsv = (*env)->GetObjectArrayElement(env, jArgv, i); + /* + ** There is a potential Java-triggerable case of Undefined + ** Behavior here, but it would require intentional misuse of the + ** API: + ** + ** If a Java UDF grabs an sqlite3_value from its argv and then + ** assigns that element to null, it becomes unreachable to us so + ** we cannot clear out its pointer. That Java-side object's + ** getNativePointer() will then refer to a stale value, so passing + ** it into (e.g.) sqlite3_value_SOMETHING() would invoke UB. + ** + ** High-level wrappers can avoid that possibility if they do not + ** expose sqlite3_value directly to clients (as is the case in + ** org.sqlite.jni.wrapper1.SqlFunction). + ** + ** One potential (but expensive) workaround for this would be to + ** privately store a duplicate argv array in each sqlite3_context + ** wrapper object, and clear the native pointers from that copy. + */ assert(jsv && "Someone illegally modified a UDF argument array."); - NativePointerHolder_set(S3JniNph(sqlite3_value), jsv, 0); + if( jsv ){ + NativePointerHolder_set(S3JniNph(sqlite3_value), jsv, 0); + } } } @@ -2099,6 +2123,7 @@ static int udf_xFV(sqlite3_context* cx, S3JniUdf * s, rc = udf_report_exception(env, isFinal, cx, s->zFuncName, zFuncType); } + udf_unargs(env, jcx, 0, 0); S3JniUnrefLocal(jcx); }else{ if( isFinal ) sqlite3_result_error_nomem(cx); diff --git a/ext/jni/src/org/sqlite/jni/capi/Tester1.java b/ext/jni/src/org/sqlite/jni/capi/Tester1.java index d21d75e3be..05b1cfeaed 100644 --- a/ext/jni/src/org/sqlite/jni/capi/Tester1.java +++ b/ext/jni/src/org/sqlite/jni/capi/Tester1.java @@ -928,15 +928,28 @@ public class Tester1 implements Runnable { // To confirm that xFinal() is called with no aggregate state // when the corresponding result set is empty. new ValueHolder<>(false); + final ValueHolder neverEverDoThisInClientCode = new ValueHolder<>(null); + final ValueHolder neverEverDoThisInClientCode2 = new ValueHolder<>(null); SQLFunction func = new AggregateFunction(){ @Override public void xStep(sqlite3_context cx, sqlite3_value[] args){ + if( null==neverEverDoThisInClientCode.value ){ + /* !!!NEVER!!! hold a reference to an sqlite3_value or + sqlite3_context object like this in client code! They + are ONLY legal for the duration of their single + call. We do it here ONLY to test that the defenses + against clients doing this are working. */ + neverEverDoThisInClientCode.value = args; + } final ValueHolder agg = this.getAggregateState(cx, 0); agg.value += sqlite3_value_int(args[0]); affirm( agg == this.getAggregateState(cx, 0) ); } @Override public void xFinal(sqlite3_context cx){ + if( null==neverEverDoThisInClientCode2.value ){ + neverEverDoThisInClientCode2.value = cx; + } final Integer v = this.takeAggregateState(cx); if(null == v){ xFinalNull.value = true; @@ -961,6 +974,10 @@ public class Tester1 implements Runnable { } affirm( 1==n ); affirm(!xFinalNull.value); + affirm( null!=neverEverDoThisInClientCode.value ); + affirm( null!=neverEverDoThisInClientCode2.value ); + affirm( 0