Cache nextPageTokens that a package can use.
A user-package can only manipulate their own nextPageTokens (i.e.
advance or invalidate it) otherwise some other package could affect the
search results of a package. Because we check at the user-package level,
this still allows a package to manipulate nextPageTokens that were
returned for different databases. This isn't recommended, but not a
security risk since it's the package's own data.
Note that manipulating nextPageTokens isn't available through
AppSearch's public API. This would be if someone were circumventing the
normal AppSearchSession.
Bug: 187972715
Test: AppSearchImplTest
Change-Id: I67a22f3ae171ea2886eb89dcf493286a8421408d
diff --git a/service/java/com/android/server/appsearch/AppSearchManagerService.java b/service/java/com/android/server/appsearch/AppSearchManagerService.java
index ec37c3f..85a9cce 100644
--- a/service/java/com/android/server/appsearch/AppSearchManagerService.java
+++ b/service/java/com/android/server/appsearch/AppSearchManagerService.java
@@ -777,7 +777,7 @@
AppSearchUserInstance instance =
mAppSearchUserInstanceManager.getUserInstance(callingUser);
SearchResultPage searchResultPage =
- instance.getAppSearchImpl().getNextPage(nextPageToken);
+ instance.getAppSearchImpl().getNextPage(packageName, nextPageToken);
invokeCallbackOnResult(
callback,
AppSearchResult.newSuccessfulResult(searchResultPage.getBundle()));
@@ -803,7 +803,7 @@
verifyNotInstantApp(userContext, packageName);
AppSearchUserInstance instance =
mAppSearchUserInstanceManager.getUserInstance(callingUser);
- instance.getAppSearchImpl().invalidateNextPageToken(nextPageToken);
+ instance.getAppSearchImpl().invalidateNextPageToken(packageName, nextPageToken);
} catch (Throwable t) {
Log.e(TAG, "Unable to invalidate the query page token", t);
}
@@ -853,7 +853,7 @@
.getGenericDocument().getBundle());
}
searchResultPage = instance.getAppSearchImpl().getNextPage(
- searchResultPage.getNextPageToken());
+ packageName, searchResultPage.getNextPageToken());
}
}
invokeCallbackOnResult(callback, AppSearchResult.newSuccessfulResult(null));
diff --git a/service/java/com/android/server/appsearch/external/localstorage/AppSearchImpl.java b/service/java/com/android/server/appsearch/external/localstorage/AppSearchImpl.java
index a1b93ce..830e76c 100644
--- a/service/java/com/android/server/appsearch/external/localstorage/AppSearchImpl.java
+++ b/service/java/com/android/server/appsearch/external/localstorage/AppSearchImpl.java
@@ -173,6 +173,21 @@
@GuardedBy("mReadWriteLock")
private final Map<String, Integer> mDocumentCountMapLocked = new ArrayMap<>();
+ // Maps packages to the set of valid nextPageTokens that the package can manipulate. A token
+ // is unique and constant per query (i.e. the same token '123' is used to iterate through
+ // pages of search results). The tokens themselves are generated and tracked by
+ // IcingSearchEngine. IcingSearchEngine considers a token valid and won't be reused
+ // until we call invalidateNextPageToken on the token.
+ //
+ // Note that we synchronize on itself because the nextPageToken cache is checked at
+ // query-time, and queries are done in parallel with a read lock. Ideally, this would be
+ // guarded by the normal mReadWriteLock.writeLock, but ReentrantReadWriteLocks can't upgrade
+ // read to write locks. This lock should be acquired at the smallest scope possible.
+ // mReadWriteLock is a higher-level lock, so calls shouldn't be made out
+ // to any functions that grab the lock.
+ @GuardedBy("mNextPageTokensLocked")
+ private final Map<String, Set<Long>> mNextPageTokensLocked = new ArrayMap<>();
+
/**
* The counter to check when to call {@link #checkForOptimize}. The interval is {@link
* #CHECK_OPTIMIZE_INTERVAL}.
@@ -837,12 +852,15 @@
String prefix = createPrefix(packageName, databaseName);
Set<String> allowedPrefixedSchemas = getAllowedPrefixSchemasLocked(prefix, searchSpec);
- return doQueryLocked(
- Collections.singleton(createPrefix(packageName, databaseName)),
- allowedPrefixedSchemas,
- queryExpression,
- searchSpec,
- sStatsBuilder);
+ SearchResultPage searchResultPage =
+ doQueryLocked(
+ Collections.singleton(createPrefix(packageName, databaseName)),
+ allowedPrefixedSchemas,
+ queryExpression,
+ searchSpec,
+ sStatsBuilder);
+ addNextPageToken(packageName, searchResultPage.getNextPageToken());
+ return searchResultPage;
} finally {
mReadWriteLock.readLock().unlock();
if (logger != null) {
@@ -956,12 +974,15 @@
}
}
- return doQueryLocked(
- prefixFilters,
- prefixedSchemaFilters,
- queryExpression,
- searchSpec,
- sStatsBuilder);
+ SearchResultPage searchResultPage =
+ doQueryLocked(
+ prefixFilters,
+ prefixedSchemaFilters,
+ queryExpression,
+ searchSpec,
+ sStatsBuilder);
+ addNextPageToken(callerPackageName, searchResultPage.getNextPageToken());
+ return searchResultPage;
} finally {
mReadWriteLock.readLock().unlock();
@@ -1093,17 +1114,20 @@
*
* <p>This method belongs to query group.
*
+ * @param packageName Package name of the caller.
* @param nextPageToken The token of pre-loaded results of previously executed query.
* @return The next page of results of previously executed query.
- * @throws AppSearchException on IcingSearchEngine error.
+ * @throws AppSearchException on IcingSearchEngine error or if can't advance on nextPageToken.
*/
@NonNull
- public SearchResultPage getNextPage(long nextPageToken) throws AppSearchException {
+ public SearchResultPage getNextPage(@NonNull String packageName, long nextPageToken)
+ throws AppSearchException {
mReadWriteLock.readLock().lock();
try {
throwIfClosedLocked();
mLogUtil.piiTrace("getNextPage, request", nextPageToken);
+ checkNextPageToken(packageName, nextPageToken);
SearchResultProto searchResultProto =
mIcingSearchEngineLocked.getNextPage(nextPageToken);
mLogUtil.piiTrace(
@@ -1122,16 +1146,26 @@
*
* <p>This method belongs to query group.
*
+ * @param packageName Package name of the caller.
* @param nextPageToken The token of pre-loaded results of previously executed query to be
* Invalidated.
+ * @throws AppSearchException if nextPageToken is unusable.
*/
- public void invalidateNextPageToken(long nextPageToken) {
+ public void invalidateNextPageToken(@NonNull String packageName, long nextPageToken)
+ throws AppSearchException {
mReadWriteLock.readLock().lock();
try {
throwIfClosedLocked();
mLogUtil.piiTrace("invalidateNextPageToken, request", nextPageToken);
+ checkNextPageToken(packageName, nextPageToken);
mIcingSearchEngineLocked.invalidateNextPageToken(nextPageToken);
+
+ synchronized (mNextPageTokensLocked) {
+ // At this point, we're guaranteed that this nextPageToken exists for this package,
+ // otherwise checkNextPageToken would've thrown an exception.
+ mNextPageTokensLocked.get(packageName).remove(nextPageToken);
+ }
} finally {
mReadWriteLock.readLock().unlock();
}
@@ -1568,6 +1602,9 @@
Set<String> databaseNames = entry.getValue();
if (!installedPackages.contains(packageName) && databaseNames != null) {
mDocumentCountMapLocked.remove(packageName);
+ synchronized (mNextPageTokensLocked) {
+ mNextPageTokensLocked.remove(packageName);
+ }
for (String databaseName : databaseNames) {
String removedPrefix = createPrefix(packageName, databaseName);
mSchemaMapLocked.remove(removedPrefix);
@@ -1601,6 +1638,9 @@
mSchemaMapLocked.clear();
mNamespaceMapLocked.clear();
mDocumentCountMapLocked.clear();
+ synchronized (mNextPageTokensLocked) {
+ mNextPageTokensLocked.clear();
+ }
if (initStatsBuilder != null) {
initStatsBuilder
.setHasReset(true)
@@ -2015,6 +2055,32 @@
return schemaProto.getSchema();
}
+ private void addNextPageToken(String packageName, long nextPageToken) {
+ synchronized (mNextPageTokensLocked) {
+ Set<Long> tokens = mNextPageTokensLocked.get(packageName);
+ if (tokens == null) {
+ tokens = new ArraySet<>();
+ mNextPageTokensLocked.put(packageName, tokens);
+ }
+ tokens.add(nextPageToken);
+ }
+ }
+
+ private void checkNextPageToken(String packageName, long nextPageToken)
+ throws AppSearchException {
+ synchronized (mNextPageTokensLocked) {
+ Set<Long> nextPageTokens = mNextPageTokensLocked.get(packageName);
+ if (nextPageTokens == null || !nextPageTokens.contains(nextPageToken)) {
+ throw new AppSearchException(
+ AppSearchResult.RESULT_SECURITY_ERROR,
+ "Package \""
+ + packageName
+ + "\" cannot use nextPageToken: "
+ + nextPageToken);
+ }
+ }
+ }
+
private static void addToMap(
Map<String, Set<String>> map, String prefix, String prefixedValue) {
Set<String> values = map.get(prefix);