Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit c69fa839 authored by Lee Shombert's avatar Lee Shombert Committed by Android (Google) Code Review
Browse files

Merge "Read-ony transactions may modify TEMP tables" into main

parents 1e203823 fef32c39
Loading
Loading
Loading
Loading
+14 −2
Original line number Diff line number Diff line
@@ -121,8 +121,12 @@ public final class SQLiteConnection implements CancellationSignal.OnCancelListen
    // The native SQLiteConnection pointer.  (FOR INTERNAL USE ONLY)
    private long mConnectionPtr;

    // Restrict this connection to read-only operations.
    private boolean mOnlyAllowReadOnlyOperations;

    // Allow this connection to treat updates to temporary tables as read-only operations.
    private boolean mAllowTempTableRetry = Flags.sqliteAllowTempTables();

    // The number of times attachCancellationSignal has been called.
    // Because SQLite statement execution can be reentrant, we keep track of how many
    // times we have attempted to attach a cancellation signal to the connection so that
@@ -142,6 +146,7 @@ public final class SQLiteConnection implements CancellationSignal.OnCancelListen
    private static native void nativeFinalizeStatement(long connectionPtr, long statementPtr);
    private static native int nativeGetParameterCount(long connectionPtr, long statementPtr);
    private static native boolean nativeIsReadOnly(long connectionPtr, long statementPtr);
    private static native boolean nativeUpdatesTempOnly(long connectionPtr, long statementPtr);
    private static native int nativeGetColumnCount(long connectionPtr, long statementPtr);
    private static native String nativeGetColumnName(long connectionPtr, long statementPtr,
            int index);
@@ -1097,7 +1102,7 @@ public final class SQLiteConnection implements CancellationSignal.OnCancelListen
        try {
            final int numParameters = nativeGetParameterCount(mConnectionPtr, statementPtr);
            final int type = DatabaseUtils.getSqlStatementTypeExtended(sql);
            final boolean readOnly = nativeIsReadOnly(mConnectionPtr, statementPtr);
            boolean readOnly = nativeIsReadOnly(mConnectionPtr, statementPtr);
            statement = obtainPreparedStatement(sql, statementPtr, numParameters, type, readOnly,
                    seqNum);
            if (!skipCache && isCacheable(type)) {
@@ -1265,13 +1270,20 @@ public final class SQLiteConnection implements CancellationSignal.OnCancelListen

    /**
     * Verify that the statement is read-only, if the connection only allows read-only
     * operations.
     * operations.  If the connection allows updates to temporary tables, then the statement is
     * read-only if the only updates are to temporary tables.
     * @param statement The statement to check.
     * @throws SQLiteException if the statement could update the database inside a read-only
     * transaction.
     */
    void throwIfStatementForbidden(PreparedStatement statement) {
        if (mOnlyAllowReadOnlyOperations && !statement.mReadOnly) {
            if (mAllowTempTableRetry) {
                statement.mReadOnly =
                        nativeUpdatesTempOnly(mConnectionPtr, statement.mStatementPtr);
                if (statement.mReadOnly) return;
            }

            throw new SQLiteException("Cannot execute this statement because it "
                    + "might modify the database but the connection is read-only.");
        }
+8 −0
Original line number Diff line number Diff line
@@ -7,3 +7,11 @@ flag {
     description: "SQLite APIs held back for Android 15"
     bug: "279043253"
}

flag {
     name: "sqlite_allow_temp_tables"
     namespace: "system_performance"
     is_fixed_read_only: true
     description: "Permit updates to TEMP tables in read-only transactions"
     bug: "317993835"
}
+52 −1
Original line number Diff line number Diff line
@@ -82,10 +82,16 @@ struct SQLiteConnection {
    const String8 path;
    const String8 label;

    // The prepared statement used to determine which tables are updated by a statement.  This
    // is is initially null.  It is set non-null on first use.
    sqlite3_stmt* tableQuery;

    volatile bool canceled;

    SQLiteConnection(sqlite3* db, int openFlags, const String8& path, const String8& label) :
        db(db), openFlags(openFlags), path(path), label(label), canceled(false) { }
            db(db), openFlags(openFlags), path(path), label(label), tableQuery(nullptr),
            canceled(false) { }

};

// Called each time a statement begins execution, when tracing is enabled.
@@ -188,6 +194,9 @@ static void nativeClose(JNIEnv* env, jclass clazz, jlong connectionPtr) {

    if (connection) {
        ALOGV("Closing connection %p", connection->db);
        if (connection->tableQuery != nullptr) {
            sqlite3_finalize(connection->tableQuery);
        }
        int err = sqlite3_close(connection->db);
        if (err != SQLITE_OK) {
            // This can happen if sub-objects aren't closed first.  Make sure the caller knows.
@@ -419,6 +428,46 @@ static jboolean nativeIsReadOnly(JNIEnv* env, jclass clazz, jlong connectionPtr,
    return sqlite3_stmt_readonly(statement) != 0;
}

static jboolean nativeUpdatesTempOnly(JNIEnv* env, jclass,
        jlong connectionPtr, jlong statementPtr) {
    sqlite3_stmt* statement = reinterpret_cast<sqlite3_stmt*>(statementPtr);
    SQLiteConnection* connection = reinterpret_cast<SQLiteConnection*>(connectionPtr);

    int result = SQLITE_OK;
    if (connection->tableQuery == nullptr) {
        static char const* sql =
                "SELECT COUNT(*) FROM tables_used(?) WHERE schema != 'temp' AND wr != 0";
        result = sqlite3_prepare_v2(connection->db, sql, -1, &connection->tableQuery, nullptr);
        if (result != SQLITE_OK) {
            ALOGE("failed to compile query table: %s",
                  sqlite3_errstr(sqlite3_extended_errcode(connection->db)));
            return false;
        }
    }

    // A temporary, to simplify the code.
    sqlite3_stmt* query = connection->tableQuery;
    sqlite3_reset(query);
    sqlite3_clear_bindings(query);
    result = sqlite3_bind_text(query, 1, sqlite3_sql(statement), -1, SQLITE_STATIC);
    if (result != SQLITE_OK) {
        ALOGE("tables bind pointer returns %s", sqlite3_errstr(result));
        return false;
    }
    result = sqlite3_step(query);
    if (result != SQLITE_ROW) {
        ALOGE("tables query error: %d/%s", result, sqlite3_errstr(result));
        // Make sure the query is no longer bound to the statement.
        sqlite3_clear_bindings(query);
        return false;
    }

    int count = sqlite3_column_int(query, 0);
    // Make sure the query is no longer bound to the statement SQL string.
    sqlite3_clear_bindings(query);
    return count == 0;
}

static jint nativeGetColumnCount(JNIEnv* env, jclass clazz, jlong connectionPtr,
        jlong statementPtr) {
    sqlite3_stmt* statement = reinterpret_cast<sqlite3_stmt*>(statementPtr);
@@ -915,6 +964,8 @@ static const JNINativeMethod sMethods[] =
            (void*)nativeGetParameterCount },
    { "nativeIsReadOnly", "(JJ)Z",
            (void*)nativeIsReadOnly },
    { "nativeUpdatesTempOnly", "(JJ)Z",
            (void*)nativeUpdatesTempOnly },
    { "nativeGetColumnCount", "(JJ)I",
            (void*)nativeGetColumnCount },
    { "nativeGetColumnName", "(JJI)Ljava/lang/String;",
+54 −0
Original line number Diff line number Diff line
@@ -26,6 +26,9 @@ import android.content.Context;
import android.database.Cursor;
import android.database.DatabaseUtils;
import android.os.SystemClock;
import android.platform.test.annotations.RequiresFlagsEnabled;
import android.platform.test.flag.junit.CheckFlagsRule;
import android.platform.test.flag.junit.DeviceFlagsValueProvider;
import android.test.AndroidTestCase;
import android.util.Log;

@@ -35,6 +38,7 @@ import androidx.test.runner.AndroidJUnit4;

import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;

@@ -53,6 +57,10 @@ import java.util.concurrent.TimeUnit;
@SmallTest
public class SQLiteDatabaseTest {

    @Rule
    public final CheckFlagsRule mCheckFlagsRule =
            DeviceFlagsValueProvider.createCheckFlagsRule();

    private static final String TAG = "SQLiteDatabaseTest";

    private final Context mContext = InstrumentationRegistry.getInstrumentation().getContext();
@@ -347,4 +355,50 @@ public class SQLiteDatabaseTest {

        assertTrue("ReadThread failed with errors: " + errors, errors.isEmpty());
    }

    @RequiresFlagsEnabled(Flags.FLAG_SQLITE_ALLOW_TEMP_TABLES)
    @Test
    public void testTempTable() {
        boolean allowed;
        allowed = true;
        mDatabase.beginTransactionReadOnly();
        try {
            mDatabase.execSQL("CREATE TEMP TABLE t1 (i int, j int);");
            mDatabase.execSQL("INSERT INTO t1 (i, j) VALUES (2, 20)");
            mDatabase.execSQL("INSERT INTO t1 (i, j) VALUES (3, 30)");

            final String sql = "SELECT i FROM t1 WHERE j = 30";
            try (SQLiteRawStatement s = mDatabase.createRawStatement(sql)) {
                assertTrue(s.step());
                assertEquals(3, s.getColumnInt(0));
            }

        } catch (SQLiteException e) {
            allowed = false;
        } finally {
            mDatabase.endTransaction();
        }
        assertTrue(allowed);

        // Repeat the test on the main schema.
        allowed = true;
        mDatabase.beginTransactionReadOnly();
        try {
            mDatabase.execSQL("CREATE TABLE t2 (i int, j int);");
            mDatabase.execSQL("INSERT INTO t2 (i, j) VALUES (2, 20)");
            mDatabase.execSQL("INSERT INTO t2 (i, j) VALUES (3, 30)");

            final String sql = "SELECT i FROM t2 WHERE j = 30";
            try (SQLiteRawStatement s = mDatabase.createRawStatement(sql)) {
                assertTrue(s.step());
                assertEquals(3, s.getColumnInt(0));
            }

        } catch (SQLiteException e) {
            allowed = false;
        } finally {
            mDatabase.endTransaction();
        }
        assertFalse(allowed);
    }
}