From 3e26b7db55c69d5eeea3f665aa0ea30f82776112 Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Thu, 12 Jul 2018 19:47:49 -0600 Subject: [PATCH] Extend SQLiteQueryBuilder for update and delete. Developers often accept selection clauses from untrusted code, and SQLiteQueryBuilder already supports a "strict" mode to help catch SQL injection attacks. This change extends the builder to support update() and delete() calls, so that we can help secure those selection clauses too. Extend it to support selection arguments being provided when appending appendWhere() clauses, meaning developers no longer need to manually track their local selection arguments along with remote arguments. Extend it to support newer ContentProvider.query() variant that accepts "Bundle queryArgs", and have all query() callers flow through that common code path. (This paves the way for a future CL that will offer to gracefully extract non-WHERE clauses that callers have tried smashing into their selections.) Updates ContentValues to internally use more efficient ArrayMap. Bug: 111268862 Test: atest frameworks/base/core/tests/utiltests/src/com/android/internal/util/ArrayUtilsTest.java Test: atest cts/tests/tests/database/src/android/database/sqlite/cts/SQLiteQueryBuilderTest.java Merged-In: I60b6f69045766bb28d2f21a32c120ec8c383b917 Change-Id: I60b6f69045766bb28d2f21a32c120ec8c383b917 --- api/current.txt | 6 + .../java/android/content/ContentResolver.java | 7 + core/java/android/content/ContentValues.java | 120 +++-- .../database/sqlite/SQLiteDatabase.java | 3 +- .../database/sqlite/SQLiteQueryBuilder.java | 483 +++++++++++++++--- .../com/android/internal/util/ArrayUtils.java | 17 + .../android/internal/util/ArrayUtilsTest.java | 64 ++- 7 files changed, 549 insertions(+), 151 deletions(-) diff --git a/api/current.txt b/api/current.txt index 8079daadc8873..70e7833f76f42 100755 --- a/api/current.txt +++ b/api/current.txt @@ -12668,22 +12668,28 @@ package android.database.sqlite { ctor public SQLiteQueryBuilder(); method public static void appendColumns(java.lang.StringBuilder, java.lang.String[]); method public void appendWhere(java.lang.CharSequence); + method public void appendWhere(java.lang.CharSequence, java.lang.String...); method public void appendWhereEscapeString(java.lang.String); + method public void appendWhereEscapeString(java.lang.String, java.lang.String...); method public java.lang.String buildQuery(java.lang.String[], java.lang.String, java.lang.String, java.lang.String, java.lang.String, java.lang.String); method public deprecated java.lang.String buildQuery(java.lang.String[], java.lang.String, java.lang.String[], java.lang.String, java.lang.String, java.lang.String, java.lang.String); method public static java.lang.String buildQueryString(boolean, java.lang.String, java.lang.String[], java.lang.String, java.lang.String, java.lang.String, java.lang.String, java.lang.String); method public java.lang.String buildUnionQuery(java.lang.String[], java.lang.String, java.lang.String); method public java.lang.String buildUnionSubQuery(java.lang.String, java.lang.String[], java.util.Set, int, java.lang.String, java.lang.String, java.lang.String, java.lang.String); method public deprecated java.lang.String buildUnionSubQuery(java.lang.String, java.lang.String[], java.util.Set, int, java.lang.String, java.lang.String, java.lang.String[], java.lang.String, java.lang.String); + method public int delete(android.database.sqlite.SQLiteDatabase, java.lang.String, java.lang.String[]); method public java.lang.String getTables(); method public android.database.Cursor query(android.database.sqlite.SQLiteDatabase, java.lang.String[], java.lang.String, java.lang.String[], java.lang.String, java.lang.String, java.lang.String); method public android.database.Cursor query(android.database.sqlite.SQLiteDatabase, java.lang.String[], java.lang.String, java.lang.String[], java.lang.String, java.lang.String, java.lang.String, java.lang.String); + method public android.database.Cursor query(android.database.sqlite.SQLiteDatabase, java.lang.String[], java.lang.String, java.lang.String[], java.lang.String, android.os.CancellationSignal); method public android.database.Cursor query(android.database.sqlite.SQLiteDatabase, java.lang.String[], java.lang.String, java.lang.String[], java.lang.String, java.lang.String, java.lang.String, java.lang.String, android.os.CancellationSignal); + method public android.database.Cursor query(android.database.sqlite.SQLiteDatabase, java.lang.String[], android.os.Bundle, android.os.CancellationSignal); method public void setCursorFactory(android.database.sqlite.SQLiteDatabase.CursorFactory); method public void setDistinct(boolean); method public void setProjectionMap(java.util.Map); method public void setStrict(boolean); method public void setTables(java.lang.String); + method public int update(android.database.sqlite.SQLiteDatabase, android.content.ContentValues, java.lang.String, java.lang.String[]); } public class SQLiteReadOnlyDatabaseException extends android.database.sqlite.SQLiteException { diff --git a/core/java/android/content/ContentResolver.java b/core/java/android/content/ContentResolver.java index b138b9d62ea2f..ac98e12bda3e6 100644 --- a/core/java/android/content/ContentResolver.java +++ b/core/java/android/content/ContentResolver.java @@ -261,6 +261,13 @@ public abstract class ContentResolver { */ public static final String QUERY_ARG_SQL_SORT_ORDER = "android:query-arg-sql-sort-order"; + /** {@hide} */ + public static final String QUERY_ARG_SQL_GROUP_BY = "android:query-arg-sql-group-by"; + /** {@hide} */ + public static final String QUERY_ARG_SQL_HAVING = "android:query-arg-sql-having"; + /** {@hide} */ + public static final String QUERY_ARG_SQL_LIMIT = "android:query-arg-sql-limit"; + /** * Specifies the list of columns against which to sort results. When first column values * are identical, records are then sorted based on second column values, and so on. diff --git a/core/java/android/content/ContentValues.java b/core/java/android/content/ContentValues.java index 54857bb55f2eb..da2049c8f6d7d 100644 --- a/core/java/android/content/ContentValues.java +++ b/core/java/android/content/ContentValues.java @@ -19,6 +19,7 @@ package android.content; import android.annotation.UnsupportedAppUsage; import android.os.Parcel; import android.os.Parcelable; +import android.util.ArrayMap; import android.util.Log; import java.util.ArrayList; @@ -33,17 +34,21 @@ import java.util.Set; public final class ContentValues implements Parcelable { public static final String TAG = "ContentValues"; - /** Holds the actual values */ + /** + * @hide + * @deprecated kept around for lame people doing reflection + */ + @Deprecated @UnsupportedAppUsage private HashMap mValues; + private final ArrayMap mMap; + /** * Creates an empty set of values using the default initial size */ public ContentValues() { - // Choosing a default size of 8 based on analysis of typical - // consumption by applications. - mValues = new HashMap(8); + mMap = new ArrayMap<>(); } /** @@ -52,7 +57,7 @@ public final class ContentValues implements Parcelable { * @param size the initial size of the set of values */ public ContentValues(int size) { - mValues = new HashMap(size, 1.0f); + mMap = new ArrayMap<>(size); } /** @@ -61,19 +66,24 @@ public final class ContentValues implements Parcelable { * @param from the values to copy */ public ContentValues(ContentValues from) { - mValues = new HashMap(from.mValues); + mMap = new ArrayMap<>(from.mMap); } /** - * Creates a set of values copied from the given HashMap. This is used - * by the Parcel unmarshalling code. - * - * @param values the values to start with - * {@hide} + * @hide + * @deprecated kept around for lame people doing reflection */ + @Deprecated @UnsupportedAppUsage - private ContentValues(HashMap values) { - mValues = values; + private ContentValues(HashMap from) { + mMap = new ArrayMap<>(); + mMap.putAll(from); + } + + /** {@hide} */ + private ContentValues(Parcel in) { + mMap = new ArrayMap<>(in.readInt()); + in.readArrayMap(mMap, null); } @Override @@ -81,12 +91,17 @@ public final class ContentValues implements Parcelable { if (!(object instanceof ContentValues)) { return false; } - return mValues.equals(((ContentValues) object).mValues); + return mMap.equals(((ContentValues) object).mMap); + } + + /** {@hide} */ + public ArrayMap getValues() { + return mMap; } @Override public int hashCode() { - return mValues.hashCode(); + return mMap.hashCode(); } /** @@ -96,7 +111,7 @@ public final class ContentValues implements Parcelable { * @param value the data for the value to put */ public void put(String key, String value) { - mValues.put(key, value); + mMap.put(key, value); } /** @@ -105,7 +120,7 @@ public final class ContentValues implements Parcelable { * @param other the ContentValues from which to copy */ public void putAll(ContentValues other) { - mValues.putAll(other.mValues); + mMap.putAll(other.mMap); } /** @@ -115,7 +130,7 @@ public final class ContentValues implements Parcelable { * @param value the data for the value to put */ public void put(String key, Byte value) { - mValues.put(key, value); + mMap.put(key, value); } /** @@ -125,7 +140,7 @@ public final class ContentValues implements Parcelable { * @param value the data for the value to put */ public void put(String key, Short value) { - mValues.put(key, value); + mMap.put(key, value); } /** @@ -135,7 +150,7 @@ public final class ContentValues implements Parcelable { * @param value the data for the value to put */ public void put(String key, Integer value) { - mValues.put(key, value); + mMap.put(key, value); } /** @@ -145,7 +160,7 @@ public final class ContentValues implements Parcelable { * @param value the data for the value to put */ public void put(String key, Long value) { - mValues.put(key, value); + mMap.put(key, value); } /** @@ -155,7 +170,7 @@ public final class ContentValues implements Parcelable { * @param value the data for the value to put */ public void put(String key, Float value) { - mValues.put(key, value); + mMap.put(key, value); } /** @@ -165,7 +180,7 @@ public final class ContentValues implements Parcelable { * @param value the data for the value to put */ public void put(String key, Double value) { - mValues.put(key, value); + mMap.put(key, value); } /** @@ -175,7 +190,7 @@ public final class ContentValues implements Parcelable { * @param value the data for the value to put */ public void put(String key, Boolean value) { - mValues.put(key, value); + mMap.put(key, value); } /** @@ -185,7 +200,7 @@ public final class ContentValues implements Parcelable { * @param value the data for the value to put */ public void put(String key, byte[] value) { - mValues.put(key, value); + mMap.put(key, value); } /** @@ -194,7 +209,7 @@ public final class ContentValues implements Parcelable { * @param key the name of the value to make null */ public void putNull(String key) { - mValues.put(key, null); + mMap.put(key, null); } /** @@ -203,7 +218,7 @@ public final class ContentValues implements Parcelable { * @return the number of values */ public int size() { - return mValues.size(); + return mMap.size(); } /** @@ -214,7 +229,7 @@ public final class ContentValues implements Parcelable { * TODO: consider exposing this new method publicly */ public boolean isEmpty() { - return mValues.isEmpty(); + return mMap.isEmpty(); } /** @@ -223,14 +238,14 @@ public final class ContentValues implements Parcelable { * @param key the name of the value to remove */ public void remove(String key) { - mValues.remove(key); + mMap.remove(key); } /** * Removes all values. */ public void clear() { - mValues.clear(); + mMap.clear(); } /** @@ -240,7 +255,7 @@ public final class ContentValues implements Parcelable { * @return {@code true} if the value is present, {@code false} otherwise */ public boolean containsKey(String key) { - return mValues.containsKey(key); + return mMap.containsKey(key); } /** @@ -252,7 +267,7 @@ public final class ContentValues implements Parcelable { * was previously added with the given {@code key} */ public Object get(String key) { - return mValues.get(key); + return mMap.get(key); } /** @@ -262,7 +277,7 @@ public final class ContentValues implements Parcelable { * @return the String for the value */ public String getAsString(String key) { - Object value = mValues.get(key); + Object value = mMap.get(key); return value != null ? value.toString() : null; } @@ -273,7 +288,7 @@ public final class ContentValues implements Parcelable { * @return the Long value, or {@code null} if the value is missing or cannot be converted */ public Long getAsLong(String key) { - Object value = mValues.get(key); + Object value = mMap.get(key); try { return value != null ? ((Number) value).longValue() : null; } catch (ClassCastException e) { @@ -298,7 +313,7 @@ public final class ContentValues implements Parcelable { * @return the Integer value, or {@code null} if the value is missing or cannot be converted */ public Integer getAsInteger(String key) { - Object value = mValues.get(key); + Object value = mMap.get(key); try { return value != null ? ((Number) value).intValue() : null; } catch (ClassCastException e) { @@ -323,7 +338,7 @@ public final class ContentValues implements Parcelable { * @return the Short value, or {@code null} if the value is missing or cannot be converted */ public Short getAsShort(String key) { - Object value = mValues.get(key); + Object value = mMap.get(key); try { return value != null ? ((Number) value).shortValue() : null; } catch (ClassCastException e) { @@ -348,7 +363,7 @@ public final class ContentValues implements Parcelable { * @return the Byte value, or {@code null} if the value is missing or cannot be converted */ public Byte getAsByte(String key) { - Object value = mValues.get(key); + Object value = mMap.get(key); try { return value != null ? ((Number) value).byteValue() : null; } catch (ClassCastException e) { @@ -373,7 +388,7 @@ public final class ContentValues implements Parcelable { * @return the Double value, or {@code null} if the value is missing or cannot be converted */ public Double getAsDouble(String key) { - Object value = mValues.get(key); + Object value = mMap.get(key); try { return value != null ? ((Number) value).doubleValue() : null; } catch (ClassCastException e) { @@ -398,7 +413,7 @@ public final class ContentValues implements Parcelable { * @return the Float value, or {@code null} if the value is missing or cannot be converted */ public Float getAsFloat(String key) { - Object value = mValues.get(key); + Object value = mMap.get(key); try { return value != null ? ((Number) value).floatValue() : null; } catch (ClassCastException e) { @@ -423,7 +438,7 @@ public final class ContentValues implements Parcelable { * @return the Boolean value, or {@code null} if the value is missing or cannot be converted */ public Boolean getAsBoolean(String key) { - Object value = mValues.get(key); + Object value = mMap.get(key); try { return (Boolean) value; } catch (ClassCastException e) { @@ -451,7 +466,7 @@ public final class ContentValues implements Parcelable { * {@code byte[]} */ public byte[] getAsByteArray(String key) { - Object value = mValues.get(key); + Object value = mMap.get(key); if (value instanceof byte[]) { return (byte[]) value; } else { @@ -465,7 +480,7 @@ public final class ContentValues implements Parcelable { * @return a set of all of the keys and values */ public Set> valueSet() { - return mValues.entrySet(); + return mMap.entrySet(); } /** @@ -474,30 +489,31 @@ public final class ContentValues implements Parcelable { * @return a set of all of the keys */ public Set keySet() { - return mValues.keySet(); + return mMap.keySet(); } public static final Parcelable.Creator CREATOR = new Parcelable.Creator() { - @SuppressWarnings({"deprecation", "unchecked"}) + @Override public ContentValues createFromParcel(Parcel in) { - // TODO - what ClassLoader should be passed to readHashMap? - HashMap values = in.readHashMap(null); - return new ContentValues(values); + return new ContentValues(in); } + @Override public ContentValues[] newArray(int size) { return new ContentValues[size]; } }; + @Override public int describeContents() { return 0; } - @SuppressWarnings("deprecation") + @Override public void writeToParcel(Parcel parcel, int flags) { - parcel.writeMap(mValues); + parcel.writeInt(mMap.size()); + parcel.writeArrayMap(mMap); } /** @@ -507,7 +523,7 @@ public final class ContentValues implements Parcelable { @Deprecated @UnsupportedAppUsage public void putStringArrayList(String key, ArrayList value) { - mValues.put(key, value); + mMap.put(key, value); } /** @@ -518,7 +534,7 @@ public final class ContentValues implements Parcelable { @Deprecated @UnsupportedAppUsage public ArrayList getStringArrayList(String key) { - return (ArrayList) mValues.get(key); + return (ArrayList) mMap.get(key); } /** @@ -528,7 +544,7 @@ public final class ContentValues implements Parcelable { @Override public String toString() { StringBuilder sb = new StringBuilder(); - for (String name : mValues.keySet()) { + for (String name : mMap.keySet()) { String value = getAsString(name); if (sb.length() > 0) sb.append(" "); sb.append(name + "=" + value); diff --git a/core/java/android/database/sqlite/SQLiteDatabase.java b/core/java/android/database/sqlite/SQLiteDatabase.java index 25d98f7edf06e..01557c59f8ac0 100644 --- a/core/java/android/database/sqlite/SQLiteDatabase.java +++ b/core/java/android/database/sqlite/SQLiteDatabase.java @@ -193,8 +193,9 @@ public final class SQLiteDatabase extends SQLiteClosable { */ public static final int CONFLICT_NONE = 0; + /** {@hide} */ @UnsupportedAppUsage - private static final String[] CONFLICT_VALUES = new String[] + public static final String[] CONFLICT_VALUES = new String[] {"", " OR ROLLBACK ", " OR ABORT ", " OR FAIL ", " OR IGNORE ", " OR REPLACE "}; /** diff --git a/core/java/android/database/sqlite/SQLiteQueryBuilder.java b/core/java/android/database/sqlite/SQLiteQueryBuilder.java index 1bd44fa5c2c62..ce610f17ae739 100644 --- a/core/java/android/database/sqlite/SQLiteQueryBuilder.java +++ b/core/java/android/database/sqlite/SQLiteQueryBuilder.java @@ -16,18 +16,40 @@ package android.database.sqlite; +import static android.content.ContentResolver.QUERY_ARG_SQL_GROUP_BY; +import static android.content.ContentResolver.QUERY_ARG_SQL_HAVING; +import static android.content.ContentResolver.QUERY_ARG_SQL_LIMIT; +import static android.content.ContentResolver.QUERY_ARG_SQL_SELECTION; +import static android.content.ContentResolver.QUERY_ARG_SQL_SELECTION_ARGS; +import static android.content.ContentResolver.QUERY_ARG_SQL_SORT_ORDER; + import android.annotation.UnsupportedAppUsage; +import android.annotation.NonNull; +import android.annotation.Nullable; +import android.content.ContentResolver; +import android.content.ContentValues; import android.database.Cursor; import android.database.DatabaseUtils; +import android.os.Build; +import android.os.Bundle; import android.os.CancellationSignal; import android.os.OperationCanceledException; import android.provider.BaseColumns; import android.text.TextUtils; +import android.util.ArrayMap; import android.util.Log; +import com.android.internal.util.ArrayUtils; + +import dalvik.system.VMRuntime; + +import libcore.util.EmptyArray; + +import java.util.Arrays; import java.util.Iterator; import java.util.Map; import java.util.Map.Entry; +import java.util.Objects; import java.util.Set; import java.util.regex.Pattern; @@ -35,8 +57,7 @@ import java.util.regex.Pattern; * This is a convenience class that helps build SQL queries to be sent to * {@link SQLiteDatabase} objects. */ -public class SQLiteQueryBuilder -{ +public class SQLiteQueryBuilder { private static final String TAG = "SQLiteQueryBuilder"; private static final Pattern sLimitPattern = Pattern.compile("\\s*\\d+\\s*(,\\s*\\d+\\s*)?"); @@ -46,6 +67,7 @@ public class SQLiteQueryBuilder private String mTables = ""; @UnsupportedAppUsage private StringBuilder mWhereClause = null; // lazily created + private String[] mWhereArgs = EmptyArray.STRING; @UnsupportedAppUsage private boolean mDistinct; private SQLiteDatabase.CursorFactory mFactory; @@ -86,43 +108,92 @@ public class SQLiteQueryBuilder mTables = inTables; } - /** - * Append a chunk to the WHERE clause of the query. All chunks appended are surrounded - * by parenthesis and ANDed with the selection passed to {@link #query}. The final - * WHERE clause looks like: - * - * WHERE (<append chunk 1><append chunk2>) AND (<query() selection parameter>) - * - * @param inWhere the chunk of text to append to the WHERE clause. - */ - public void appendWhere(CharSequence inWhere) { - if (mWhereClause == null) { - mWhereClause = new StringBuilder(inWhere.length() + 16); - } - if (mWhereClause.length() == 0) { - mWhereClause.append('('); - } - mWhereClause.append(inWhere); + /** {@hide} */ + public @Nullable String getWhere() { + return (mWhereClause != null) ? mWhereClause.toString() : null; + } + + /** {@hide} */ + public String[] getWhereArgs() { + return mWhereArgs; } /** - * Append a chunk to the WHERE clause of the query. All chunks appended are surrounded - * by parenthesis and ANDed with the selection passed to {@link #query}. The final - * WHERE clause looks like: + * Append a chunk to the {@code WHERE} clause of the query. All chunks + * appended are surrounded by parenthesis and {@code AND}ed with the + * selection passed to {@link #query}. The final {@code WHERE} clause looks + * like: * + * 
      * WHERE (<append chunk 1><append chunk2>) AND (<query() selection parameter>)
+     *
* - * @param inWhere the chunk of text to append to the WHERE clause. it will be escaped - * to avoid SQL injection attacks + * @param inWhere the chunk of text to append to the {@code WHERE} clause. */ - public void appendWhereEscapeString(String inWhere) { + public void appendWhere(@NonNull CharSequence inWhere) { + appendWhere(inWhere, EmptyArray.STRING); + } + + /** + * Append a chunk to the {@code WHERE} clause of the query. All chunks + * appended are surrounded by parenthesis and {@code AND}ed with the + * selection passed to {@link #query}. The final {@code WHERE} clause looks + * like: + * + * 
+     * WHERE (<append chunk 1><append chunk2>) AND (<query() selection parameter>)
+     *
+ * + * @param inWhere the chunk of text to append to the {@code WHERE} clause. + * @param inWhereArgs list of arguments to be bound to any '?' occurrences + * in the where clause. + */ + public void appendWhere(@NonNull CharSequence inWhere, String... inWhereArgs) { if (mWhereClause == null) { mWhereClause = new StringBuilder(inWhere.length() + 16); } - if (mWhereClause.length() == 0) { - mWhereClause.append('('); + mWhereClause.append(inWhere); + mWhereArgs = ArrayUtils.concat(String.class, mWhereArgs, inWhereArgs); + } + + /** + * Append a chunk to the {@code WHERE} clause of the query. All chunks + * appended are surrounded by parenthesis and {@code AND}ed with the + * selection passed to {@link #query}. The final {@code WHERE} clause looks + * like this: + * + * 
+     * WHERE (<append chunk 1><append chunk2>) AND (<query() selection parameter>)
+     *
+ * + * @param inWhere the chunk of text to append to the {@code WHERE} clause. + * It will be escaped to avoid SQL injection attacks. + */ + public void appendWhereEscapeString(@NonNull String inWhere) { + appendWhereEscapeString(inWhere, EmptyArray.STRING); + } + + /** + * Append a chunk to the {@code WHERE} clause of the query. All chunks + * appended are surrounded by parenthesis and {@code AND}ed with the + * selection passed to {@link #query}. The final {@code WHERE} clause looks + * like this: + * + * 
+     * WHERE (<append chunk 1><append chunk2>) AND (<query() selection parameter>)
+     *
+ * + * @param inWhere the chunk of text to append to the {@code WHERE} clause. + * It will be escaped to avoid SQL injection attacks. + * @param inWhereArgs list of arguments to be bound to any '?' occurrences + * in the where clause. + */ + public void appendWhereEscapeString(@NonNull String inWhere, String... inWhereArgs) { + if (mWhereClause == null) { + mWhereClause = new StringBuilder(inWhere.length() + 16); } DatabaseUtils.appendEscapedSQLString(mWhereClause, inWhere); + mWhereArgs = ArrayUtils.concat(String.class, mWhereArgs, inWhereArgs); } /** @@ -172,8 +243,8 @@ public class SQLiteQueryBuilder * * By default, this value is false. */ - public void setStrict(boolean flag) { - mStrict = flag; + public void setStrict(boolean strict) { + mStrict = strict; } /** @@ -267,7 +338,7 @@ public class SQLiteQueryBuilder * information passed into this method. * * @param db the database to query on - * @param projectionIn A list of which columns to return. Passing + * @param projection A list of which columns to return. Passing * null will return all columns, which is discouraged to prevent * reading data from storage that isn't going to be used. * @param selection A filter declaring which rows to return, @@ -292,10 +363,14 @@ public class SQLiteQueryBuilder * @see android.content.ContentResolver#query(android.net.Uri, String[], * String, String[], String) */ - public Cursor query(SQLiteDatabase db, String[] projectionIn, - String selection, String[] selectionArgs, String groupBy, - String having, String sortOrder) { - return query(db, projectionIn, selection, selectionArgs, groupBy, having, sortOrder, + public @Nullable Cursor query(@NonNull SQLiteDatabase db, + @Nullable String[] projection, + @Nullable String selection, + @Nullable String[] selectionArgs, + @Nullable String groupBy, + @Nullable String having, + @Nullable String sortOrder) { + return query(db, projection, selection, selectionArgs, groupBy, having, sortOrder, null /* limit */, null /* cancellationSignal */); } @@ -304,7 +379,7 @@ public class SQLiteQueryBuilder * information passed into this method. * * @param db the database to query on - * @param projectionIn A list of which columns to return. Passing + * @param projection A list of which columns to return. Passing * null will return all columns, which is discouraged to prevent * reading data from storage that isn't going to be used. * @param selection A filter declaring which rows to return, @@ -331,10 +406,15 @@ public class SQLiteQueryBuilder * @see android.content.ContentResolver#query(android.net.Uri, String[], * String, String[], String) */ - public Cursor query(SQLiteDatabase db, String[] projectionIn, - String selection, String[] selectionArgs, String groupBy, - String having, String sortOrder, String limit) { - return query(db, projectionIn, selection, selectionArgs, + public @Nullable Cursor query(@NonNull SQLiteDatabase db, + @Nullable String[] projection, + @Nullable String selection, + @Nullable String[] selectionArgs, + @Nullable String groupBy, + @Nullable String having, + @Nullable String sortOrder, + @Nullable String limit) { + return query(db, projection, selection, selectionArgs, groupBy, having, sortOrder, limit, null); } @@ -343,7 +423,42 @@ public class SQLiteQueryBuilder * information passed into this method. * * @param db the database to query on - * @param projectionIn A list of which columns to return. Passing + * @param projection A list of which columns to return. Passing + * null will return all columns, which is discouraged to prevent + * reading data from storage that isn't going to be used. + * @param selection A filter declaring which rows to return, + * formatted as an SQL WHERE clause (excluding the WHERE + * itself). Passing null will return all rows for the given URL. + * @param selectionArgs You may include ?s in selection, which + * will be replaced by the values from selectionArgs, in order + * that they appear in the selection. The values will be bound + * as Strings. + * @param sortOrder How to order the rows, formatted as an SQL + * ORDER BY clause (excluding the ORDER BY itself). Passing null + * will use the default sort order, which may be unordered. + * @param cancellationSignal A signal to cancel the operation in progress, or null if none. + * If the operation is canceled, then {@link OperationCanceledException} will be thrown + * when the query is executed. + * @return a cursor over the result set + * @see android.content.ContentResolver#query(android.net.Uri, String[], + * String, String[], String) + */ + public @Nullable Cursor query(@NonNull SQLiteDatabase db, + @Nullable String[] projection, + @Nullable String selection, + @Nullable String[] selectionArgs, + @Nullable String sortOrder, + @Nullable CancellationSignal cancellationSignal) { + return query(db, projection, selection, selectionArgs, null, null, sortOrder, null, + cancellationSignal); + } + + /** + * Perform a query by combining all current settings and the + * information passed into this method. + * + * @param db the database to query on + * @param projection A list of which columns to return. Passing * null will return all columns, which is discouraged to prevent * reading data from storage that isn't going to be used. * @param selection A filter declaring which rows to return, @@ -373,14 +488,59 @@ public class SQLiteQueryBuilder * @see android.content.ContentResolver#query(android.net.Uri, String[], * String, String[], String) */ - public Cursor query(SQLiteDatabase db, String[] projectionIn, - String selection, String[] selectionArgs, String groupBy, - String having, String sortOrder, String limit, CancellationSignal cancellationSignal) { - if (mTables == null) { + public @Nullable Cursor query(@NonNull SQLiteDatabase db, + @Nullable String[] projection, + @Nullable String selection, + @Nullable String[] selectionArgs, + @Nullable String groupBy, + @Nullable String having, + @Nullable String sortOrder, + @Nullable String limit, + @Nullable CancellationSignal cancellationSignal) { + final Bundle queryArgs = new Bundle(); + maybePutString(queryArgs, QUERY_ARG_SQL_SELECTION, selection); + maybePutStringArray(queryArgs, QUERY_ARG_SQL_SELECTION_ARGS, selectionArgs); + maybePutString(queryArgs, QUERY_ARG_SQL_GROUP_BY, groupBy); + maybePutString(queryArgs, QUERY_ARG_SQL_HAVING, having); + maybePutString(queryArgs, QUERY_ARG_SQL_SORT_ORDER, sortOrder); + maybePutString(queryArgs, QUERY_ARG_SQL_LIMIT, limit); + return query(db, projection, queryArgs, cancellationSignal); + } + + /** + * Perform a query by combining all current settings and the information + * passed into this method. + * + * @param db the database to query on + * @param projection A list of which columns to return. Passing null will + * return all columns, which is discouraged to prevent reading + * data from storage that isn't going to be used. + * @param queryArgs A collection of arguments for the query, defined using + * keys such as {@link ContentResolver#QUERY_ARG_SQL_SELECTION} + * and {@link ContentResolver#QUERY_ARG_SQL_SELECTION_ARGS}. + * @param cancellationSignal A signal to cancel the operation in progress, + * or null if none. If the operation is canceled, then + * {@link OperationCanceledException} will be thrown when the + * query is executed. + * @return a cursor over the result set + */ + public Cursor query(@NonNull SQLiteDatabase db, + @Nullable String[] projection, + @Nullable Bundle queryArgs, + @Nullable CancellationSignal cancellationSignal) { + Objects.requireNonNull(db, "No database defined"); + + if (VMRuntime.getRuntime().getTargetSdkVersion() >= Build.VERSION_CODES.Q) { + Objects.requireNonNull(mTables, "No tables defined"); + } else if (mTables == null) { return null; } - if (mStrict && selection != null && selection.length() > 0) { + if (queryArgs == null) { + queryArgs = Bundle.EMPTY; + } + + if (mStrict) { // Validate the user-supplied selection to detect syntactic anomalies // in the selection string that could indicate a SQL injection attempt. // The idea is to ensure that the selection clause is a valid SQL expression @@ -388,24 +548,128 @@ public class SQLiteQueryBuilder // originally specified. An attacker cannot create an expression that // would escape the SQL expression while maintaining balanced parentheses // in both the wrapped and original forms. - String sqlForValidation = buildQuery(projectionIn, "(" + selection + ")", groupBy, - having, sortOrder, limit); - db.validateSql(sqlForValidation, cancellationSignal); // will throw if query is invalid + + // TODO: decode SORT ORDER and LIMIT clauses, since they can contain + // "expr" inside that need to be validated + final String sql = buildQuery(projection, + wrap(queryArgs.getString(QUERY_ARG_SQL_SELECTION)), + wrap(queryArgs.getString(QUERY_ARG_SQL_GROUP_BY)), + wrap(queryArgs.getString(QUERY_ARG_SQL_HAVING)), + queryArgs.getString(QUERY_ARG_SQL_SORT_ORDER), + queryArgs.getString(QUERY_ARG_SQL_LIMIT)); + db.validateSql(sql, cancellationSignal); // will throw if query is invalid } - String sql = buildQuery( - projectionIn, selection, groupBy, having, - sortOrder, limit); + final String sql = buildQuery(projection, + queryArgs.getString(QUERY_ARG_SQL_SELECTION), + queryArgs.getString(QUERY_ARG_SQL_GROUP_BY), + queryArgs.getString(QUERY_ARG_SQL_HAVING), + queryArgs.getString(QUERY_ARG_SQL_SORT_ORDER), + queryArgs.getString(QUERY_ARG_SQL_LIMIT)); - if (Log.isLoggable(TAG, Log.DEBUG)) { - Log.d(TAG, "Performing query: " + sql); + final String[] sqlArgs = ArrayUtils.concat(String.class, + queryArgs.getStringArray(QUERY_ARG_SQL_SELECTION_ARGS), mWhereArgs); + + if (Build.IS_DEBUGGABLE && Log.isLoggable(TAG, Log.DEBUG)) { + Log.d(TAG, sql + " with args " + Arrays.toString(sqlArgs)); } + return db.rawQueryWithFactory( - mFactory, sql, selectionArgs, + mFactory, sql, sqlArgs, SQLiteDatabase.findEditTable(mTables), cancellationSignal); // will throw if query is invalid } + /** + * Perform an update by combining all current settings and the + * information passed into this method. + * + * @param db the database to update on + * @param selection A filter declaring which rows to return, + * formatted as an SQL WHERE clause (excluding the WHERE + * itself). Passing null will return all rows for the given URL. + * @param selectionArgs You may include ?s in selection, which + * will be replaced by the values from selectionArgs, in order + * that they appear in the selection. The values will be bound + * as Strings. + * @return the number of rows updated + */ + public int update(@NonNull SQLiteDatabase db, @NonNull ContentValues values, + @Nullable String selection, @Nullable String[] selectionArgs) { + Objects.requireNonNull(mTables, "No tables defined"); + Objects.requireNonNull(db, "No database defined"); + Objects.requireNonNull(values, "No values defined"); + + if (mStrict) { + // Validate the user-supplied selection to detect syntactic anomalies + // in the selection string that could indicate a SQL injection attempt. + // The idea is to ensure that the selection clause is a valid SQL expression + // by compiling it twice: once wrapped in parentheses and once as + // originally specified. An attacker cannot create an expression that + // would escape the SQL expression while maintaining balanced parentheses + // in both the wrapped and original forms. + final String sql = buildUpdate(values, wrap(selection)); + db.validateSql(sql, null); // will throw if query is invalid + } + + final ArrayMap rawValues = values.getValues(); + final String[] updateArgs = new String[rawValues.size()]; + for (int i = 0; i < updateArgs.length; i++) { + updateArgs[i] = String.valueOf(rawValues.valueAt(i)); + } + + final String sql = buildUpdate(values, selection); + final String[] sqlArgs = ArrayUtils.concat(String.class, updateArgs, + ArrayUtils.concat(String.class, selectionArgs, mWhereArgs)); + + if (Build.IS_DEBUGGABLE && Log.isLoggable(TAG, Log.DEBUG)) { + Log.d(TAG, sql + " with args " + Arrays.toString(sqlArgs)); + } + + return db.executeSql(sql, sqlArgs); + } + + /** + * Perform a delete by combining all current settings and the + * information passed into this method. + * + * @param db the database to delete on + * @param selection A filter declaring which rows to return, + * formatted as an SQL WHERE clause (excluding the WHERE + * itself). Passing null will return all rows for the given URL. + * @param selectionArgs You may include ?s in selection, which + * will be replaced by the values from selectionArgs, in order + * that they appear in the selection. The values will be bound + * as Strings. + * @return the number of rows deleted + */ + public int delete(@NonNull SQLiteDatabase db, @Nullable String selection, + @Nullable String[] selectionArgs) { + Objects.requireNonNull(mTables, "No tables defined"); + Objects.requireNonNull(db, "No database defined"); + + if (mStrict) { + // Validate the user-supplied selection to detect syntactic anomalies + // in the selection string that could indicate a SQL injection attempt. + // The idea is to ensure that the selection clause is a valid SQL expression + // by compiling it twice: once wrapped in parentheses and once as + // originally specified. An attacker cannot create an expression that + // would escape the SQL expression while maintaining balanced parentheses + // in both the wrapped and original forms. + final String sql = buildDelete(wrap(selection)); + db.validateSql(sql, null); // will throw if query is invalid + } + + final String sql = buildDelete(selection); + final String[] sqlArgs = ArrayUtils.concat(String.class, selectionArgs, mWhereArgs); + + if (Build.IS_DEBUGGABLE && Log.isLoggable(TAG, Log.DEBUG)) { + Log.d(TAG, sql + " with args " + Arrays.toString(sqlArgs)); + } + + return db.executeSql(sql, sqlArgs); + } + /** * Construct a SELECT statement suitable for use in a group of * SELECT statements that will be joined through UNION operators @@ -438,28 +702,10 @@ public class SQLiteQueryBuilder String[] projectionIn, String selection, String groupBy, String having, String sortOrder, String limit) { String[] projection = computeProjection(projectionIn); - - StringBuilder where = new StringBuilder(); - boolean hasBaseWhereClause = mWhereClause != null && mWhereClause.length() > 0; - - if (hasBaseWhereClause) { - where.append(mWhereClause.toString()); - where.append(')'); - } - - // Tack on the user's selection, if present. - if (selection != null && selection.length() > 0) { - if (hasBaseWhereClause) { - where.append(" AND "); - } - - where.append('('); - where.append(selection); - where.append(')'); - } + String where = computeWhere(selection); return buildQueryString( - mDistinct, mTables, projection, where.toString(), + mDistinct, mTables, projection, where, groupBy, having, sortOrder, limit); } @@ -476,6 +722,42 @@ public class SQLiteQueryBuilder return buildQuery(projectionIn, selection, groupBy, having, sortOrder, limit); } + /** {@hide} */ + public String buildUpdate(ContentValues values, String selection) { + if (values == null || values.isEmpty()) { + throw new IllegalArgumentException("Empty values"); + } + + StringBuilder sql = new StringBuilder(120); + sql.append("UPDATE "); + sql.append(mTables); + sql.append(" SET "); + + final ArrayMap rawValues = values.getValues(); + for (int i = 0; i < rawValues.size(); i++) { + if (i > 0) { + sql.append(','); + } + sql.append(rawValues.keyAt(i)); + sql.append("=?"); + } + + final String where = computeWhere(selection); + appendClause(sql, " WHERE ", where); + return sql.toString(); + } + + /** {@hide} */ + public String buildDelete(String selection) { + StringBuilder sql = new StringBuilder(120); + sql.append("DELETE FROM "); + sql.append(mTables); + + final String where = computeWhere(selection); + appendClause(sql, " WHERE ", where); + return sql.toString(); + } + /** * Construct a SELECT statement suitable for use in a group of * SELECT statements that will be joined through UNION operators @@ -601,7 +883,7 @@ public class SQLiteQueryBuilder } @UnsupportedAppUsage - private String[] computeProjection(String[] projectionIn) { + private @Nullable String[] computeProjection(@Nullable String[] projectionIn) { if (projectionIn != null && projectionIn.length > 0) { if (mProjectionMap != null) { String[] projection = new String[projectionIn.length]; @@ -624,7 +906,7 @@ public class SQLiteQueryBuilder } throw new IllegalArgumentException("Invalid column " - + projectionIn[i]); + + projectionIn[i] + " from tables " + mTables); } return projection; } else { @@ -650,4 +932,53 @@ public class SQLiteQueryBuilder } return null; } + + private @NonNull String computeWhere(@Nullable String selection) { + final boolean hasUser = selection != null && selection.length() > 0; + final boolean hasInternal = mWhereClause != null && mWhereClause.length() > 0; + + if (hasUser || hasInternal) { + final StringBuilder where = new StringBuilder(); + if (hasUser) { + where.append('(').append(selection).append(')'); + } + if (hasUser && hasInternal) { + where.append(" AND "); + } + if (hasInternal) { + where.append('(').append(mWhereClause.toString()).append(')'); + } + return where.toString(); + } else { + return null; + } + } + + /** + * Wrap given argument in parenthesis, unless it's {@code null} or + * {@code ()}, in which case return it verbatim. + */ + private @Nullable String wrap(@Nullable String arg) { + if (arg == null) { + return null; + } else if (arg.equals("")) { + return arg; + } else { + return "(" + arg + ")"; + } + } + + private static void maybePutString(@NonNull Bundle bundle, @NonNull String key, + @Nullable String value) { + if (value != null) { + bundle.putString(key, value); + } + } + + private static void maybePutStringArray(@NonNull Bundle bundle, @NonNull String key, + @Nullable String[] value) { + if (value != null) { + bundle.putStringArray(key, value); + } + } } diff --git a/core/java/com/android/internal/util/ArrayUtils.java b/core/java/com/android/internal/util/ArrayUtils.java index be645fe707c65..c3d33ca84ee10 100644 --- a/core/java/com/android/internal/util/ArrayUtils.java +++ b/core/java/com/android/internal/util/ArrayUtils.java @@ -308,6 +308,23 @@ public class ArrayUtils { return array; } + @SuppressWarnings("unchecked") + public static @NonNull T[] concat(Class kind, @Nullable T[] a, @Nullable T[] b) { + final int an = (a != null) ? a.length : 0; + final int bn = (b != null) ? b.length : 0; + if (an == 0 && bn == 0) { + if (kind == String.class) { + return (T[]) EmptyArray.STRING; + } else if (kind == Object.class) { + return (T[]) EmptyArray.OBJECT; + } + } + final T[] res = (T[]) Array.newInstance(kind, an + bn); + if (an > 0) System.arraycopy(a, 0, res, 0, an); + if (bn > 0) System.arraycopy(b, 0, res, an, bn); + return res; + } + /** * Adds value to given array if not already present, providing set-like * behavior. diff --git a/core/tests/utiltests/src/com/android/internal/util/ArrayUtilsTest.java b/core/tests/utiltests/src/com/android/internal/util/ArrayUtilsTest.java index 433d4d214b972..6464ad3e97096 100644 --- a/core/tests/utiltests/src/com/android/internal/util/ArrayUtilsTest.java +++ b/core/tests/utiltests/src/com/android/internal/util/ArrayUtilsTest.java @@ -16,9 +16,8 @@ package com.android.internal.util; -import android.test.MoreAsserts; +import static org.junit.Assert.assertArrayEquals; -import java.util.Arrays; import junit.framework.TestCase; /** @@ -92,29 +91,29 @@ public class ArrayUtilsTest extends TestCase { } public void testAppendInt() throws Exception { - MoreAsserts.assertEquals(new int[] { 1 }, + assertArrayEquals(new int[] { 1 }, ArrayUtils.appendInt(null, 1)); - MoreAsserts.assertEquals(new int[] { 1 }, + assertArrayEquals(new int[] { 1 }, ArrayUtils.appendInt(new int[] { }, 1)); - MoreAsserts.assertEquals(new int[] { 1, 2 }, + assertArrayEquals(new int[] { 1, 2 }, ArrayUtils.appendInt(new int[] { 1 }, 2)); - MoreAsserts.assertEquals(new int[] { 1, 2 }, + assertArrayEquals(new int[] { 1, 2 }, ArrayUtils.appendInt(new int[] { 1, 2 }, 1)); } public void testRemoveInt() throws Exception { assertNull(ArrayUtils.removeInt(null, 1)); - MoreAsserts.assertEquals(new int[] { }, + assertArrayEquals(new int[] { }, ArrayUtils.removeInt(new int[] { }, 1)); - MoreAsserts.assertEquals(new int[] { 1, 2, 3, }, + assertArrayEquals(new int[] { 1, 2, 3, }, ArrayUtils.removeInt(new int[] { 1, 2, 3}, 4)); - MoreAsserts.assertEquals(new int[] { 2, 3, }, + assertArrayEquals(new int[] { 2, 3, }, ArrayUtils.removeInt(new int[] { 1, 2, 3}, 1)); - MoreAsserts.assertEquals(new int[] { 1, 3, }, + assertArrayEquals(new int[] { 1, 3, }, ArrayUtils.removeInt(new int[] { 1, 2, 3}, 2)); - MoreAsserts.assertEquals(new int[] { 1, 2, }, + assertArrayEquals(new int[] { 1, 2, }, ArrayUtils.removeInt(new int[] { 1, 2, 3}, 3)); - MoreAsserts.assertEquals(new int[] { 2, 3, 1 }, + assertArrayEquals(new int[] { 2, 3, 1 }, ArrayUtils.removeInt(new int[] { 1, 2, 3, 1 }, 1)); } @@ -129,30 +128,51 @@ public class ArrayUtilsTest extends TestCase { } public void testAppendLong() throws Exception { - MoreAsserts.assertEquals(new long[] { 1 }, + assertArrayEquals(new long[] { 1 }, ArrayUtils.appendLong(null, 1)); - MoreAsserts.assertEquals(new long[] { 1 }, + assertArrayEquals(new long[] { 1 }, ArrayUtils.appendLong(new long[] { }, 1)); - MoreAsserts.assertEquals(new long[] { 1, 2 }, + assertArrayEquals(new long[] { 1, 2 }, ArrayUtils.appendLong(new long[] { 1 }, 2)); - MoreAsserts.assertEquals(new long[] { 1, 2 }, + assertArrayEquals(new long[] { 1, 2 }, ArrayUtils.appendLong(new long[] { 1, 2 }, 1)); } public void testRemoveLong() throws Exception { assertNull(ArrayUtils.removeLong(null, 1)); - MoreAsserts.assertEquals(new long[] { }, + assertArrayEquals(new long[] { }, ArrayUtils.removeLong(new long[] { }, 1)); - MoreAsserts.assertEquals(new long[] { 1, 2, 3, }, + assertArrayEquals(new long[] { 1, 2, 3, }, ArrayUtils.removeLong(new long[] { 1, 2, 3}, 4)); - MoreAsserts.assertEquals(new long[] { 2, 3, }, + assertArrayEquals(new long[] { 2, 3, }, ArrayUtils.removeLong(new long[] { 1, 2, 3}, 1)); - MoreAsserts.assertEquals(new long[] { 1, 3, }, + assertArrayEquals(new long[] { 1, 3, }, ArrayUtils.removeLong(new long[] { 1, 2, 3}, 2)); - MoreAsserts.assertEquals(new long[] { 1, 2, }, + assertArrayEquals(new long[] { 1, 2, }, ArrayUtils.removeLong(new long[] { 1, 2, 3}, 3)); - MoreAsserts.assertEquals(new long[] { 2, 3, 1 }, + assertArrayEquals(new long[] { 2, 3, 1 }, ArrayUtils.removeLong(new long[] { 1, 2, 3, 1 }, 1)); } + public void testConcatEmpty() throws Exception { + assertArrayEquals(new Long[] {}, + ArrayUtils.concat(Long.class, null, null)); + assertArrayEquals(new Long[] {}, + ArrayUtils.concat(Long.class, new Long[] {}, null)); + assertArrayEquals(new Long[] {}, + ArrayUtils.concat(Long.class, null, new Long[] {})); + assertArrayEquals(new Long[] {}, + ArrayUtils.concat(Long.class, new Long[] {}, new Long[] {})); + } + + public void testConcat() throws Exception { + assertArrayEquals(new Long[] { 1L }, + ArrayUtils.concat(Long.class, new Long[] { 1L }, new Long[] {})); + assertArrayEquals(new Long[] { 1L }, + ArrayUtils.concat(Long.class, new Long[] {}, new Long[] { 1L })); + assertArrayEquals(new Long[] { 1L, 2L }, + ArrayUtils.concat(Long.class, new Long[] { 1L }, new Long[] { 2L })); + assertArrayEquals(new Long[] { 1L, 2L, 3L, 4L }, + ArrayUtils.concat(Long.class, new Long[] { 1L, 2L }, new Long[] { 3L, 4L })); + } }