Remove JNI sqlite3_column_java_object(), as the protection rules of sqlite3_values makes it impossible to implement safely. Add JNI sqlite3_bind_java_object().

FossilOrigin-Name: 29bd4a23a4afd96b2cc06d2b91a4f30c0bbf2347af0b0d18f8d4cf8aafa63160
2025-07-29 08:01:23 +03:00 · 2023-08-27 13:43:45 +00:00
parent 88bd53dfd0
commit 5575d6421b
6 changed files with 135 additions and 94 deletions
--- a/ext/jni/src/c/sqlite3-jni.c
+++ b/ext/jni/src/c/sqlite3-jni.c
@ -1260,14 +1260,16 @@ static S3JniNphClass * S3JniGlobal_nph(JNIEnv * const env, S3NphRef const* pRef)
 ** Returns the ID of the "nativePointer" field from the given
 ** NativePointerHolder<T> class.
 */
-static jfieldID NativePointerHolder_field(JNIEnv * const env, S3NphRef const* pRef){
+static jfieldID NativePointerHolder_field(JNIEnv * const env,
+                                          S3NphRef const* pRef){
  S3JniNphClass * const pNC = S3JniGlobal_nph(env, pRef);
  if( !pNC->fidValue ){
    S3JniMutex_Nph_enter;
    if( !pNC->fidValue ){
      pNC->fidValue = (*env)->GetFieldID(env, pNC->klazz,
                                         pRef->zMember, pRef->zTypeSig);
-      S3JniExceptionIsFatal("Code maintenance required: missing nativePointer field.");
+      S3JniExceptionIsFatal("Code maintenance required: missing "
+                            "nativePointer field.");
    }
    S3JniMutex_Nph_leave;
  }
@ -1291,7 +1293,8 @@ static void NativePointerHolder_set(JNIEnv * env, S3NphRef const* pRef,
 ** zClassName must be a static string so we can use its address as a
 ** cache key. This is a no-op if pObj is NULL.
 */
-static void * NativePointerHolder_get(JNIEnv * env, jobject pObj, S3NphRef const* pRef){
+static void * NativePointerHolder_get(JNIEnv * env, jobject pObj,
+                                      S3NphRef const* pRef){
  if( pObj ){
    void * const rv = (void*)(*env)->GetLongField(
      env, pObj, NativePointerHolder_field(env, pRef)
@ -1344,29 +1347,17 @@ static S3JniDb * S3JniDb_alloc(JNIEnv * const env, jobject jDb){
 }

 /*
-** Returns the S3JniDb object for the given db. At most, one of jDb or
-** pDb may be non-NULL.
+** Returns the S3JniDb object for the given org.sqlite.jni.sqlite3
+** object, or NULL if jDb is NULL, no pointer can be extracted
+** from it, or no matching entry can be found.
 **
-** The 3rd argument should normally only be non-0 for routines which
-** are called from the C library and pass a native db handle instead of
-** 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.
+** Requires locking the S3JniDb mutex.
 */
-static S3JniDb * S3JniDb_get(JNIEnv * const env, jobject jDb, sqlite3 *pDb){
+static S3JniDb * S3JniDb__from_java(JNIEnv * const env, jobject jDb){
  S3JniDb * s = 0;
-
-  if( 0==jDb && 0==pDb ) return 0;
-  assert( jDb ? !pDb : !!pDb );
+  sqlite3 * pDb = 0;
  S3JniMutex_S3JniDb_enter;
  if( jDb ){
-    assert(!pDb);
    pDb = PtrGet_sqlite3(jDb);
  }
  s = SJG.perDb.aHead;
@ -1379,8 +1370,28 @@ static S3JniDb * S3JniDb_get(JNIEnv * const env, jobject jDb, sqlite3 *pDb){
  return s;
 }

-#define S3JniDb_from_java(jObject) S3JniDb_get(env,(jObject),0)
-#define S3JniDb_from_c(sqlite3Ptr) S3JniDb_get(env,0,(sqlite3Ptr))
+/*
+** Returns the S3JniDb object for the sqlite3 object, or NULL if pDb
+** is NULL, or no matching entry
+** can be found.
+**
+** Requires locking the S3JniDb mutex.
+*/
+static S3JniDb * S3JniDb__from_c(JNIEnv * const env, sqlite3 *pDb){
+  S3JniDb * s = 0;
+  S3JniMutex_S3JniDb_enter;
+  s = SJG.perDb.aHead;
+  for( ; pDb && s; s = s->pNext){
+    if( s->pDb == pDb ){
+      break;
+    }
+  }
+  S3JniMutex_S3JniDb_leave;
+  return s;
+}
+
+#define S3JniDb_from_java(jObject) S3JniDb__from_java(env,(jObject))
+#define S3JniDb_from_c(sqlite3Ptr) S3JniDb__from_c(env,(sqlite3Ptr))

 /*
 ** Unref any Java-side state in (S3JniAutoExtension*) AX and zero out
@ -2181,6 +2192,29 @@ S3JniApi(sqlite3_bind_int64(),jint,1bind_1int64)(
  return (jint)sqlite3_bind_int64(PtrGet_sqlite3_stmt(jpStmt), (int)ndx, (sqlite3_int64)val);
 }

+/*
+** Bind a new global ref to Object `val` using sqlite3_bind_pointer().
+*/
+S3JniApi(sqlite3_bind_java_object(),jint,1bind_1java_1object)(
+  JniArgsEnvClass, jobject jpStmt, jint ndx, jobject val
+){
+  sqlite3_stmt * const pStmt = PtrGet_sqlite3_stmt(jpStmt);
+  int rc = 0;
+
+  if(pStmt){
+    jobject const rv = val ? S3JniRefGlobal(val) : 0;
+    if( rv ){
+      rc = sqlite3_bind_pointer(pStmt, ndx, rv, ResultJavaValuePtrStr,
+                                ResultJavaValue_finalizer);
+    }else if(val){
+      rc = SQLITE_NOMEM;
+    }
+  }else{
+    rc = SQLITE_MISUSE;
+  }
+  return rc;
+}
+
 S3JniApi(sqlite3_bind_null(),jint,1bind_1null)(
  JniArgsEnvClass, jobject jpStmt, jint ndx
 ){
@ -2337,7 +2371,9 @@ static jint s3jni_close_db(JNIEnv * const env, jobject jDb, int version){
  S3JniDb * const ps = S3JniDb_from_java(jDb);
  assert(version == 1 || version == 2);
  if( ps ){
-    rc = 1==version ? (jint)sqlite3_close(ps->pDb) : (jint)sqlite3_close_v2(ps->pDb);
+    rc = 1==version
+      ? (jint)sqlite3_close(ps->pDb)
+      : (jint)sqlite3_close_v2(ps->pDb);
    if( 0==rc ){
      S3JniDb_set_aside(env, ps)
        /* MUST come after close() because of ps->trace. */;
--- a/ext/jni/src/c/sqlite3-jni.h
+++ b/ext/jni/src/c/sqlite3-jni.h
@ -819,6 +819,14 @@ JNIEXPORT jint JNICALL Java_org_sqlite_jni_SQLite3Jni_sqlite3_1bind_1int
 JNIEXPORT jint JNICALL Java_org_sqlite_jni_SQLite3Jni_sqlite3_1bind_1int64
  (JNIEnv *, jclass, jobject, jint, jlong);

+/*
+ * Class:     org_sqlite_jni_SQLite3Jni
+ * Method:    sqlite3_bind_java_object
+ * Signature: (Lorg/sqlite/jni/sqlite3_stmt;ILjava/lang/Object;)I
+ */
+JNIEXPORT jint JNICALL Java_org_sqlite_jni_SQLite3Jni_sqlite3_1bind_1java_1object
+  (JNIEnv *, jclass, jobject, jint, jobject);
+
 /*
 * Class:     org_sqlite_jni_SQLite3Jni
 * Method:    sqlite3_bind_null
--- a/ext/jni/src/org/sqlite/jni/SQLite3Jni.java
+++ b/ext/jni/src/org/sqlite/jni/SQLite3Jni.java
@ -184,6 +184,15 @@ public final class SQLite3Jni {
    @NotNull sqlite3_stmt stmt, int ndx, long v
  );

+  /**
+     Binds the given object at the given index.
+
+     @see #sqlite3_result_java_object
+  */
+  public static native int sqlite3_bind_java_object(
+    @NotNull sqlite3_stmt cx, int ndx, @Nullable Object o
+  );
+
  public static native int sqlite3_bind_null(
    @NotNull sqlite3_stmt stmt, int ndx
  );
@ -210,7 +219,7 @@ public final class SQLite3Jni {
     Works like the C-level sqlite3_bind_text() but assumes
     SQLITE_TRANSIENT for the final C API parameter.

-     Results are undefined if data is not null and
+     <p>Results are undefined if data is not null and
     maxBytes>=data.length. If maxBytes is negative then results are
     undefined if data is not null and does not contain a NUL byte.
  */
@ -358,35 +367,6 @@ public final class SQLite3Jni {
    @NotNull sqlite3_stmt stmt, int ndx
  );

-  /**
-     Column counterpart of sqlite3_value_java_object().
-  */
-  public static Object sqlite3_column_java_object(
-    @NotNull sqlite3_stmt stmt, int ndx
-  ){
-    Object rv = null;
-    sqlite3_value v = sqlite3_column_value(stmt, ndx);
-    if(null!=v){
-      v = sqlite3_value_dup(v) /* we need a "protected" value */;
-      if(null!=v){
-        rv = sqlite3_value_java_object(v);
-        sqlite3_value_free(v);
-      }
-    }
-    return rv;
-  }
-
-  /**
-     Column counterpart of sqlite3_value_java_casted().
-  */
-  @SuppressWarnings("unchecked")
-  public static <T> T sqlite3_column_java_casted(
-    @NotNull sqlite3_stmt stmt, int ndx, @NotNull Class<T> type
-  ){
-    final Object o = sqlite3_column_java_object(stmt, ndx);
-    return type.isInstance(o) ? (T)o : null;
-  }
-
  public static native String sqlite3_column_origin_name(
    @NotNull sqlite3_stmt stmt, int ndx
  );
@ -614,7 +594,7 @@ public final class SQLite3Jni {
     heed. Passing the object to sqlite3_close() or sqlite3_close_v2()
     will clear that pointer mapping.

-     Recall that even if opening fails, the output pointer might be
+     <p>Recall that even if opening fails, the output pointer might be
     non-null. Any error message about the failure will be in that
     object and it is up to the caller to sqlite3_close() that
     db handle.
@ -628,7 +608,7 @@ public final class SQLite3Jni {
     object might not have been successfully opened: use sqlite3_errcode() to
     check whether it is in an error state.

-     Ownership of the returned value is passed to the caller, who must eventually
+     <p>Ownership of the returned value is passed to the caller, who must eventually
     pass it to sqlite3_close() or sqlite3_close_v2().
  */
  public static sqlite3 sqlite3_open(@Nullable String filename){
@ -659,10 +639,10 @@ public final class SQLite3Jni {
     retain functionally equivalent semantics and (B) overloading
     allows us to install several convenience forms.

-     All of them which take their SQL in the form of a byte[] require
+     <p>All of them which take their SQL in the form of a byte[] require
     that it be in UTF-8 encoding unless explicitly noted otherwise.

-     The forms which take a "tail" output pointer return (via that
+     <p>The forms which take a "tail" output pointer return (via that
     output object) the index into their SQL byte array at which the
     end of the first SQL statement processed by the call was
     found. That's fundamentally how the C APIs work but making use of
@ -959,19 +939,22 @@ public final class SQLite3Jni {
  /**
     Binds the SQL result to the given object, or
     {@link #sqlite3_result_null} if {@code o} is null. Use
-     {@link #sqlite3_value_java_object(sqlite3_value) sqlite3_value_java_object()} or
-     {@link #sqlite3_column_java_object(sqlite3_stmt,int) sqlite3_column_java_object()} to
+     {@link #sqlite3_value_java_object(sqlite3_value) sqlite3_value_java_object()} to
     fetch it.

-     This is implemented in terms of sqlite3_result_pointer(), but
-     that function is not exposed to JNI because its 3rd argument must
-     be a constant string (the library does not copy it), which we
-     cannot implement cross-language here unless, in the JNI layer, we
-     allocate such strings and store them somewhere for long-term use
-     (leaking them more likely than not). Even then, passing around a
-     pointer via Java like that has little practical use.
+     <p>This is implemented in terms of C's sqlite3_result_pointer(),
+     but that function is not exposed to JNI because its 3rd argument
+     must be a constant string (the library does not copy it), and
+     those semantics are cumbersome to bridge cross-language. Java
+     doesn't need that argument for type safety, in any case: the
+     object can, after extraction on the other end of the API, be
+     inspected with {@code instanceof}.

-     Note that there is no sqlite3_bind_java_object() counterpart.
+     <p>Note that there is no sqlite3_column_java_object(), as the
+     C-level API has no sqlite3_column_pointer() to proxy.
+
+     @see #sqlite3_value_java_object
+     @see #sqlite3_bind_java_object
  */
  public static native void sqlite3_result_java_object(
    @NotNull sqlite3_context cx, @NotNull Object o
@ -1056,12 +1039,17 @@ public final class SQLite3Jni {
  /**
     Binds the given text using C's sqlite3_result_blob64() unless:

-     - @param blob is null ==> sqlite3_result_null()
+     <ul>

-     - @param blob is too large ==> sqlite3_result_error_toobig()
+     <li>@param blob is null: translates to sqlite3_result_null()</li>

-     If @param maxLen is larger than blob.length, it is truncated to that
-     value. If it is negative, results are undefined.
+     <li>@param blob is too large: translates to
+     sqlite3_result_error_toobig()</li>
+
+     </ul>
+
+     If @param maxLen is larger than blob.length, it is truncated to
+     that value. If it is negative, results are undefined.
  */
  private static native void sqlite3_result_blob64(
    @NotNull sqlite3_context cx, @Nullable byte[] blob, long maxLen
@ -1096,17 +1084,21 @@ public final class SQLite3Jni {
  /**
     Binds the given text using C's sqlite3_result_text64() unless:

-     - text is null: translates to a call to sqlite3_result_null()
+     <ul>

-     - text is too large: translates to a call to
-       sqlite3_result_error_toobig()
+     <li>text is null: translates to a call to sqlite3_result_null()</li>

-       - The @param encoding argument has an invalid value: translates to
-       sqlite3_result_error_code() with code SQLITE_FORMAT.
+     <li>text is too large: translates to a call to
+     {@link #sqlite3_result_error_toobig}</li>
+
+     <li>The @param encoding argument has an invalid value: translates to
+     {@link sqlite3_result_error_code} with code SQLITE_FORMAT.</li>
+
+     </ul>

     If maxLength (in bytes, not characters) is larger than
     text.length, it is silently truncated to text.length. If it is
-     negative, results are undefined. If text is null, the following
+     negative, results are undefined. If text is null, the subsequent
     arguments are ignored.
  */
  private static native void sqlite3_result_text64(
@ -1224,7 +1216,7 @@ public final class SQLite3Jni {
     function is elided here because the roles of that functions' 3rd and 4th
     arguments are encapsulated in the final argument to this function.

-     Unlike the C API, which is documented as always returning 0, this
+     <p>Unlike the C API, which is documented as always returning 0, this
     implementation returns non-0 if initialization of the tracer
     mapping state fails.
  */
@ -1257,10 +1249,11 @@ public final class SQLite3Jni {
  public static native long sqlite3_value_int64(@NotNull sqlite3_value v);

  /**
-     If the given value was set using sqlite3_result_java_value() then
-     this function returns that object, else it returns null.
+     If the given value was set using {@link
+     #sqlite3_result_java_object} then this function returns that
+     object, else it returns null.

-     It is up to the caller to inspect the object to determine its
+     <p>It is up to the caller to inspect the object to determine its
     type, and cast it if necessary.
  */
  public static native Object sqlite3_value_java_object(
--- a/ext/jni/src/org/sqlite/jni/Tester1.java
+++ b/ext/jni/src/org/sqlite/jni/Tester1.java
@ -695,16 +695,20 @@ public class Tester1 implements Runnable {
  private void testUdfJavaObject(){
    final sqlite3 db = createNewDb();
    final ValueHolder<sqlite3> testResult = new ValueHolder<>(db);
+    final ValueHolder<Integer> boundObj = new ValueHolder<>(42);
    final SQLFunction func = new ScalarFunction(){
        public void xFunc(sqlite3_context cx, sqlite3_value args[]){
          sqlite3_result_java_object(cx, testResult.value);
+          affirm( sqlite3_value_java_object(args[0]) == boundObj );
        }
      };
    int rc = sqlite3_create_function(db, "myfunc", -1, SQLITE_UTF8, func);
    affirm(0 == rc);
-    final sqlite3_stmt stmt = prepare(db, "select myfunc()");
+    sqlite3_stmt stmt = prepare(db, "select myfunc(?)");
    affirm( 0 != stmt.getNativePointer() );
    affirm( testResult.value == db );
+    rc = sqlite3_bind_java_object(stmt, 1, boundObj);
+    affirm( 0==rc );
    int n = 0;
    if( SQLITE_ROW == sqlite3_step(stmt) ){
      final sqlite3_value v = sqlite3_column_value(stmt, 0);
@ -1346,7 +1350,7 @@ public class Tester1 implements Runnable {
    affirm( 8 == val.value );
  }

-  @ManualTest /* because we only want to run this test manually */
+  @ManualTest /* we really only want to run this test manually. */
  private void testSleep(){
    out("Sleeping briefly... ");
    sqlite3_sleep(600);