Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

am 0dbc4108: Merge "Fix Settings writes to a different user" into jb-mr1-dev

Browse Source 
* commit '0dbc4108005445d241c5f6990000d25a09a8e00e':
  Fix Settings writes to a different user
This commit is contained in:
Christopher Tate
2012-09-14 11:37:45 -07:00
committed by Android Git Automerger
parent 28c709d35e 0dbc410800
commit 45b2dce982
4 changed files with 152 additions and 87 deletions
Download Patch File Download Diff File Expand all files Collapse all files

42
core/java/android/provider/Settings.java
View File

@@ -754,22 +754,29 @@ public final class Settings {
} }
public String getStringForUser(ContentResolver cr, String name, final int userHandle) { public String getStringForUser(ContentResolver cr, String name, final int userHandle) {
long newValuesVersion = SystemProperties.getLong(mVersionSystemProperty, 0); final boolean isSelf = (userHandle == UserHandle.myUserId());
if (isSelf) {
long newValuesVersion = SystemProperties.getLong(mVersionSystemProperty, 0);
synchronized (this) { // Our own user's settings data uses a client-side cache
if (mValuesVersion != newValuesVersion) { synchronized (this) {
if (LOCAL_LOGV || false) { if (mValuesVersion != newValuesVersion) {
Log.v(TAG, "invalidate [" + mUri.getLastPathSegment() + "]: current " + if (LOCAL_LOGV || false) {
newValuesVersion + " != cached " + mValuesVersion); Log.v(TAG, "invalidate [" + mUri.getLastPathSegment() + "]: current "
+ newValuesVersion + " != cached " + mValuesVersion);
}
mValues.clear();
mValuesVersion = newValuesVersion;
} }
mValues.clear(); if (mValues.containsKey(name)) {
mValuesVersion = newValuesVersion; return mValues.get(name); // Could be null, that's OK -- negative caching
} }
if (mValues.containsKey(name)) {
return mValues.get(name); // Could be null, that's OK -- negative caching
} }
} else {
if (LOCAL_LOGV) Log.v(TAG, "get setting for user " + userHandle
+ " by user " + UserHandle.myUserId() + " so skipping cache");
} }
IContentProvider cp = lazyGetProvider(cr); IContentProvider cp = lazyGetProvider(cr);
@@ -788,8 +795,15 @@ public final class Settings {
Bundle b = cp.call(mCallGetCommand, name, args); Bundle b = cp.call(mCallGetCommand, name, args);
if (b != null) { if (b != null) {
String value = b.getPairValue(); String value = b.getPairValue();
synchronized (this) { // Don't update our cache for reads of other users' data
mValues.put(name, value); if (isSelf) {
synchronized (this) {
mValues.put(name, value);
}
} else {
if (LOCAL_LOGV) Log.i(TAG, "call-query of user " + userHandle
+ " by " + UserHandle.myUserId()
+ " so not updating cache");
} }
return value; return value;
} }

4
core/tests/coretests/AndroidManifest.xml
View File

@@ -69,6 +69,10 @@
<uses-permission android:name="com.google.android.googleapps.permission.GOOGLE_AUTH" /> <uses-permission android:name="com.google.android.googleapps.permission.GOOGLE_AUTH" />
<uses-permission android:name="com.google.android.googleapps.permission.GOOGLE_AUTH.ALL_SERVICES" /> <uses-permission android:name="com.google.android.googleapps.permission.GOOGLE_AUTH.ALL_SERVICES" />
<uses-permission android:name="android.permission.MANAGE_USERS" />
<uses-permission android:name="android.permission.INTERACT_ACROSS_USERS" />
<uses-permission android:name="android.permission.INTERACT_ACROSS_USERS_FULL" />
<uses-permission android:name="android.permission.RECEIVE_SMS"/> <uses-permission android:name="android.permission.RECEIVE_SMS"/>
<uses-permission android:name="android.permission.INTERNET" /> <uses-permission android:name="android.permission.INTERNET" />
<uses-permission android:name="android.permission.READ_CONTACTS" /> <uses-permission android:name="android.permission.READ_CONTACTS" />

51
core/tests/coretests/src/android/provider/SettingsProviderTest.java
View File

@@ -19,11 +19,15 @@ package android.provider;
import android.content.ContentResolver; import android.content.ContentResolver;
import android.content.ContentUris; import android.content.ContentUris;
import android.content.ContentValues; import android.content.ContentValues;
import android.content.Context;
import android.content.Intent; import android.content.Intent;
import android.content.pm.PackageManager; import android.content.pm.PackageManager;
import android.content.pm.ResolveInfo; import android.content.pm.ResolveInfo;
import android.content.pm.UserInfo;
import android.database.Cursor; import android.database.Cursor;
import android.net.Uri; import android.net.Uri;
import android.os.UserHandle;
import android.os.UserManager;
import android.provider.Settings; import android.provider.Settings;
import android.test.AndroidTestCase; import android.test.AndroidTestCase;
import android.test.suitebuilder.annotation.MediumTest; import android.test.suitebuilder.annotation.MediumTest;
@@ -197,6 +201,53 @@ public class SettingsProviderTest extends AndroidTestCase {
Settings.Secure.getString(r, Settings.Secure.LOCATION_PROVIDERS_ALLOWED)); Settings.Secure.getString(r, Settings.Secure.LOCATION_PROVIDERS_ALLOWED));
} }
private boolean findUser(UserManager um, int userHandle) {
for (UserInfo user : um.getUsers()) {
if (user.id == userHandle) {
return true;
}
}
return false;
}
@MediumTest
public void testPerUserSettings() {
UserManager um = (UserManager) getContext().getSystemService(Context.USER_SERVICE);
ContentResolver r = getContext().getContentResolver();
// Make sure there's an owner
assertTrue(findUser(um, UserHandle.USER_OWNER));
// create a new user to use for testing
UserInfo user = um.createUser("TestUser1", UserInfo.FLAG_GUEST);
assertTrue(user != null);
try {
// Write some settings for that user as well as the current user
final String TEST_KEY = "test_setting";
final int SELF_VALUE = 40;
final int OTHER_VALUE = 27;
Settings.System.putInt(r, TEST_KEY, SELF_VALUE);
Settings.System.putIntForUser(r, TEST_KEY, OTHER_VALUE, user.id);
// Verify that they read back as intended
int myValue = Settings.System.getInt(r, TEST_KEY, 0);
int otherValue = Settings.System.getIntForUser(r, TEST_KEY, 0, user.id);
assertTrue("Running as user " + UserHandle.myUserId()
+ " and reading/writing as user " + user.id
+ ", expected to read " + SELF_VALUE + " but got " + myValue,
myValue == SELF_VALUE);
assertTrue("Running as user " + UserHandle.myUserId()
+ " and reading/writing as user " + user.id
+ ", expected to read " + OTHER_VALUE + " but got " + otherValue,
otherValue == OTHER_VALUE);
} finally {
// Tidy up
um.removeUser(user.id);
}
}
@SmallTest @SmallTest
public void testSettings() { public void testSettings() {
assertCanBeHandled(new Intent(Settings.ACTION_ACCESSIBILITY_SETTINGS)); assertCanBeHandled(new Intent(Settings.ACTION_ACCESSIBILITY_SETTINGS));

142
packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java
View File

@@ -87,6 +87,9 @@ public class SettingsProvider extends ContentProvider {
private static final SparseArray<AtomicInteger> sKnownMutationsInFlight private static final SparseArray<AtomicInteger> sKnownMutationsInFlight
= new SparseArray<AtomicInteger>(); = new SparseArray<AtomicInteger>();
// Each defined user has their own settings
protected final SparseArray<DatabaseHelper> mOpenHelpers = new SparseArray<DatabaseHelper>();
// Over this size we don't reject loading or saving settings but // Over this size we don't reject loading or saving settings but
// we do consider them broken/malicious and don't keep them in // we do consider them broken/malicious and don't keep them in
// memory at least: // memory at least:
@@ -98,9 +101,6 @@ public class SettingsProvider extends ContentProvider {
// want to cache the existence of a key, but not store its value. // want to cache the existence of a key, but not store its value.
private static final Bundle TOO_LARGE_TO_CACHE_MARKER = Bundle.forPair("_dummy", null); private static final Bundle TOO_LARGE_TO_CACHE_MARKER = Bundle.forPair("_dummy", null);
// Each defined user has their own settings
protected final SparseArray<DatabaseHelper> mOpenHelpers = new SparseArray<DatabaseHelper>();
//protected DatabaseHelper mOpenHelper;
private UserManager mUserManager; private UserManager mUserManager;
private BackupManager mBackupManager; private BackupManager mBackupManager;
@@ -411,32 +411,29 @@ public class SettingsProvider extends ContentProvider {
mBackupManager = new BackupManager(getContext()); mBackupManager = new BackupManager(getContext());
mUserManager = (UserManager) getContext().getSystemService(Context.USER_SERVICE); mUserManager = (UserManager) getContext().getSystemService(Context.USER_SERVICE);
synchronized (this) { establishDbTracking(UserHandle.USER_OWNER);
establishDbTrackingLocked(UserHandle.USER_OWNER);
IntentFilter userFilter = new IntentFilter(); IntentFilter userFilter = new IntentFilter();
userFilter.addAction(Intent.ACTION_USER_REMOVED); userFilter.addAction(Intent.ACTION_USER_REMOVED);
getContext().registerReceiver(new BroadcastReceiver() { getContext().registerReceiver(new BroadcastReceiver() {
@Override @Override
public void onReceive(Context context, Intent intent) { public void onReceive(Context context, Intent intent) {
if (intent.getAction().equals(Intent.ACTION_USER_REMOVED)) { if (intent.getAction().equals(Intent.ACTION_USER_REMOVED)) {
final int userHandle = intent.getIntExtra(Intent.EXTRA_USER_HANDLE, final int userHandle = intent.getIntExtra(Intent.EXTRA_USER_HANDLE,
UserHandle.USER_OWNER); UserHandle.USER_OWNER);
if (userHandle != UserHandle.USER_OWNER) { if (userHandle != UserHandle.USER_OWNER) {
onUserRemoved(userHandle); onUserRemoved(userHandle);
}
} }
} }
}, userFilter); }
} }, userFilter);
return true; return true;
} }
void onUserRemoved(int userHandle) { void onUserRemoved(int userHandle) {
// the db file itself will be deleted automatically, but we need to tear down
// our caches and other internal bookkeeping. Creation/deletion of a user's
// settings db infrastructure is synchronized on 'this'
synchronized (this) { synchronized (this) {
// the db file itself will be deleted automatically, but we need to tear down
// our caches and other internal bookkeeping.
FileObserver observer = sObserverInstances.get(userHandle); FileObserver observer = sObserverInstances.get(userHandle);
if (observer != null) { if (observer != null) {
observer.stopWatching(); observer.stopWatching();
@@ -455,25 +452,43 @@ public class SettingsProvider extends ContentProvider {
} }
} }
private void establishDbTrackingLocked(int userHandle) { private void establishDbTracking(int userHandle) {
if (LOCAL_LOGV) { if (LOCAL_LOGV) {
Slog.i(TAG, "Installing settings db helper and caches for user " + userHandle); Slog.i(TAG, "Installing settings db helper and caches for user " + userHandle);
} }
DatabaseHelper dbhelper = new DatabaseHelper(getContext(), userHandle); DatabaseHelper dbhelper;
mOpenHelpers.append(userHandle, dbhelper);
// Watch for external modifications to the database files, synchronized (this) {
// keeping our caches in sync. dbhelper = mOpenHelpers.get(userHandle);
sSystemCaches.append(userHandle, new SettingsCache(TABLE_SYSTEM)); if (dbhelper == null) {
sSecureCaches.append(userHandle, new SettingsCache(TABLE_SECURE)); dbhelper = new DatabaseHelper(getContext(), userHandle);
sKnownMutationsInFlight.append(userHandle, new AtomicInteger(0)); mOpenHelpers.append(userHandle, dbhelper);
sSystemCaches.append(userHandle, new SettingsCache(TABLE_SYSTEM));
sSecureCaches.append(userHandle, new SettingsCache(TABLE_SECURE));
sKnownMutationsInFlight.append(userHandle, new AtomicInteger(0));
}
}
// Initialization of the db *outside* the locks. It's possible that racing
// threads might wind up here, the second having read the cache entries
// written by the first, but that's benign: the SQLite helper implementation
// manages concurrency itself, and it's important that we not run the db
// initialization with any of our own locks held, so we're fine.
SQLiteDatabase db = dbhelper.getWritableDatabase(); SQLiteDatabase db = dbhelper.getWritableDatabase();
// Now we can start observing it for changes // Watch for external modifications to the database files,
SettingsFileObserver observer = new SettingsFileObserver(userHandle, db.getPath()); // keeping our caches in sync. We synchronize the observer set
sObserverInstances.append(userHandle, observer); // separately, and of course it has to run after the db file
observer.startWatching(); // itself was set up by the DatabaseHelper.
synchronized (sObserverInstances) {
if (sObserverInstances.get(userHandle) == null) {
SettingsFileObserver observer = new SettingsFileObserver(userHandle, db.getPath());
sObserverInstances.append(userHandle, observer);
observer.startWatching();
}
}
ensureAndroidIdIsSet(userHandle); ensureAndroidIdIsSet(userHandle);
@@ -571,19 +586,17 @@ public class SettingsProvider extends ContentProvider {
// Lazy-initialize the settings caches for non-primary users // Lazy-initialize the settings caches for non-primary users
private SettingsCache getOrConstructCache(int callingUser, SparseArray<SettingsCache> which) { private SettingsCache getOrConstructCache(int callingUser, SparseArray<SettingsCache> which) {
synchronized (this) { getOrEstablishDatabase(callingUser); // ignore return value; we don't need it
getOrEstablishDatabaseLocked(callingUser); // ignore return value; we don't need it return which.get(callingUser);
return which.get(callingUser);
}
} }
// Lazy initialize the database helper and caches for this user, if necessary // Lazy initialize the database helper and caches for this user, if necessary
private DatabaseHelper getOrEstablishDatabaseLocked(int callingUser) { private DatabaseHelper getOrEstablishDatabase(int callingUser) {
long oldId = Binder.clearCallingIdentity(); long oldId = Binder.clearCallingIdentity();
try { try {
DatabaseHelper dbHelper = mOpenHelpers.get(callingUser); DatabaseHelper dbHelper = mOpenHelpers.get(callingUser);
if (null == dbHelper) { if (null == dbHelper) {
establishDbTrackingLocked(callingUser); establishDbTracking(callingUser);
dbHelper = mOpenHelpers.get(callingUser); dbHelper = mOpenHelpers.get(callingUser);
} }
return dbHelper; return dbHelper;
@@ -646,25 +659,21 @@ public class SettingsProvider extends ContentProvider {
// Get methods // Get methods
if (Settings.CALL_METHOD_GET_SYSTEM.equals(method)) { if (Settings.CALL_METHOD_GET_SYSTEM.equals(method)) {
if (LOCAL_LOGV) Slog.v(TAG, "call(system:" + request + ") for " + callingUser); if (LOCAL_LOGV) Slog.v(TAG, "call(system:" + request + ") for " + callingUser);
synchronized (this) { dbHelper = getOrEstablishDatabase(callingUser);
dbHelper = getOrEstablishDatabaseLocked(callingUser); cache = sSystemCaches.get(callingUser);
cache = sSystemCaches.get(callingUser);
}
return lookupValue(dbHelper, TABLE_SYSTEM, cache, request); return lookupValue(dbHelper, TABLE_SYSTEM, cache, request);
} }
if (Settings.CALL_METHOD_GET_SECURE.equals(method)) { if (Settings.CALL_METHOD_GET_SECURE.equals(method)) {
if (LOCAL_LOGV) Slog.v(TAG, "call(secure:" + request + ") for " + callingUser); if (LOCAL_LOGV) Slog.v(TAG, "call(secure:" + request + ") for " + callingUser);
synchronized (this) { dbHelper = getOrEstablishDatabase(callingUser);
dbHelper = getOrEstablishDatabaseLocked(callingUser); cache = sSecureCaches.get(callingUser);
cache = sSecureCaches.get(callingUser);
}
return lookupValue(dbHelper, TABLE_SECURE, cache, request); return lookupValue(dbHelper, TABLE_SECURE, cache, request);
} }
if (Settings.CALL_METHOD_GET_GLOBAL.equals(method)) { if (Settings.CALL_METHOD_GET_GLOBAL.equals(method)) {
if (LOCAL_LOGV) Slog.v(TAG, "call(global:" + request + ") for " + callingUser); if (LOCAL_LOGV) Slog.v(TAG, "call(global:" + request + ") for " + callingUser);
// fast path: owner db & cache are immutable after onCreate() so we need not // fast path: owner db & cache are immutable after onCreate() so we need not
// guard on the attempt to look them up // guard on the attempt to look them up
return lookupValue(getOrEstablishDatabaseLocked(UserHandle.USER_OWNER), TABLE_GLOBAL, return lookupValue(getOrEstablishDatabase(UserHandle.USER_OWNER), TABLE_GLOBAL,
sGlobalCache, request); sGlobalCache, request);
} }
@@ -678,13 +687,13 @@ public class SettingsProvider extends ContentProvider {
values.put(Settings.NameValueTable.VALUE, newValue); values.put(Settings.NameValueTable.VALUE, newValue);
if (Settings.CALL_METHOD_PUT_SYSTEM.equals(method)) { if (Settings.CALL_METHOD_PUT_SYSTEM.equals(method)) {
if (LOCAL_LOGV) Slog.v(TAG, "call_put(system:" + request + "=" + newValue + ") for " + callingUser); if (LOCAL_LOGV) Slog.v(TAG, "call_put(system:" + request + "=" + newValue + ") for " + callingUser);
insert(Settings.System.CONTENT_URI, values); insertForUser(Settings.System.CONTENT_URI, values, callingUser);
} else if (Settings.CALL_METHOD_PUT_SECURE.equals(method)) { } else if (Settings.CALL_METHOD_PUT_SECURE.equals(method)) {
if (LOCAL_LOGV) Slog.v(TAG, "call_put(secure:" + request + "=" + newValue + ") for " + callingUser); if (LOCAL_LOGV) Slog.v(TAG, "call_put(secure:" + request + "=" + newValue + ") for " + callingUser);
insert(Settings.Secure.CONTENT_URI, values); insertForUser(Settings.Secure.CONTENT_URI, values, callingUser);
} else if (Settings.CALL_METHOD_PUT_GLOBAL.equals(method)) { } else if (Settings.CALL_METHOD_PUT_GLOBAL.equals(method)) {
if (LOCAL_LOGV) Slog.v(TAG, "call_put(global:" + request + "=" + newValue + ") for " + callingUser); if (LOCAL_LOGV) Slog.v(TAG, "call_put(global:" + request + "=" + newValue + ") for " + callingUser);
insert(Settings.Global.CONTENT_URI, values); insertForUser(Settings.Global.CONTENT_URI, values, callingUser);
} else { } else {
Slog.w(TAG, "call() with invalid method: " + method); Slog.w(TAG, "call() with invalid method: " + method);
} }
@@ -747,10 +756,8 @@ public class SettingsProvider extends ContentProvider {
if (LOCAL_LOGV) Slog.v(TAG, "query(" + url + ") for user " + forUser); if (LOCAL_LOGV) Slog.v(TAG, "query(" + url + ") for user " + forUser);
SqlArguments args = new SqlArguments(url, where, whereArgs); SqlArguments args = new SqlArguments(url, where, whereArgs);
DatabaseHelper dbH; DatabaseHelper dbH;
synchronized (this) { dbH = getOrEstablishDatabase(
dbH = getOrEstablishDatabaseLocked( TABLE_GLOBAL.equals(args.table) ? UserHandle.USER_OWNER : forUser);
TABLE_GLOBAL.equals(args.table) ? UserHandle.USER_OWNER : forUser);
}
SQLiteDatabase db = dbH.getReadableDatabase(); SQLiteDatabase db = dbH.getReadableDatabase();
// The favorites table was moved from this provider to a provider inside Home // The favorites table was moved from this provider to a provider inside Home
@@ -805,11 +812,8 @@ public class SettingsProvider extends ContentProvider {
final AtomicInteger mutationCount = sKnownMutationsInFlight.get(callingUser); final AtomicInteger mutationCount = sKnownMutationsInFlight.get(callingUser);
mutationCount.incrementAndGet(); mutationCount.incrementAndGet();
DatabaseHelper dbH; DatabaseHelper dbH = getOrEstablishDatabase(
synchronized (this) { TABLE_GLOBAL.equals(args.table) ? UserHandle.USER_OWNER : callingUser);
dbH = getOrEstablishDatabaseLocked(
TABLE_GLOBAL.equals(args.table) ? UserHandle.USER_OWNER : callingUser);
}
SQLiteDatabase db = dbH.getWritableDatabase(); SQLiteDatabase db = dbH.getWritableDatabase();
db.beginTransaction(); db.beginTransaction();
try { try {
@@ -945,10 +949,7 @@ public class SettingsProvider extends ContentProvider {
final AtomicInteger mutationCount = sKnownMutationsInFlight.get(desiredUserHandle); final AtomicInteger mutationCount = sKnownMutationsInFlight.get(desiredUserHandle);
mutationCount.incrementAndGet(); mutationCount.incrementAndGet();
DatabaseHelper dbH; DatabaseHelper dbH = getOrEstablishDatabase(desiredUserHandle);
synchronized (this) {
dbH = getOrEstablishDatabaseLocked(desiredUserHandle);
}
SQLiteDatabase db = dbH.getWritableDatabase(); SQLiteDatabase db = dbH.getWritableDatabase();
final long rowId = db.insert(args.table, null, initialValues); final long rowId = db.insert(args.table, null, initialValues);
mutationCount.decrementAndGet(); mutationCount.decrementAndGet();
@@ -956,7 +957,8 @@ public class SettingsProvider extends ContentProvider {
SettingsCache.populate(cache, initialValues); // before we notify SettingsCache.populate(cache, initialValues); // before we notify
if (LOCAL_LOGV) Log.v(TAG, args.table + " <- " + initialValues); if (LOCAL_LOGV) Log.v(TAG, args.table + " <- " + initialValues
+ " for user " + desiredUserHandle);
// Note that we use the original url here, not the potentially-rewritten table name // Note that we use the original url here, not the potentially-rewritten table name
url = getUriFor(url, initialValues, rowId); url = getUriFor(url, initialValues, rowId);
sendNotify(url, desiredUserHandle); sendNotify(url, desiredUserHandle);
@@ -979,10 +981,7 @@ public class SettingsProvider extends ContentProvider {
final AtomicInteger mutationCount = sKnownMutationsInFlight.get(callingUser); final AtomicInteger mutationCount = sKnownMutationsInFlight.get(callingUser);
mutationCount.incrementAndGet(); mutationCount.incrementAndGet();
DatabaseHelper dbH; DatabaseHelper dbH = getOrEstablishDatabase(callingUser);
synchronized (this) {
dbH = getOrEstablishDatabaseLocked(callingUser);
}
SQLiteDatabase db = dbH.getWritableDatabase(); SQLiteDatabase db = dbH.getWritableDatabase();
int count = db.delete(args.table, args.where, args.args); int count = db.delete(args.table, args.where, args.args);
mutationCount.decrementAndGet(); mutationCount.decrementAndGet();
@@ -1014,10 +1013,7 @@ public class SettingsProvider extends ContentProvider {
final AtomicInteger mutationCount = sKnownMutationsInFlight.get(callingUser); final AtomicInteger mutationCount = sKnownMutationsInFlight.get(callingUser);
mutationCount.incrementAndGet(); mutationCount.incrementAndGet();
DatabaseHelper dbH; DatabaseHelper dbH = getOrEstablishDatabase(callingUser);
synchronized (this) {
dbH = getOrEstablishDatabaseLocked(callingUser);
}
SQLiteDatabase db = dbH.getWritableDatabase(); SQLiteDatabase db = dbH.getWritableDatabase();
int count = db.update(args.table, initialValues, args.where, args.args); int count = db.update(args.table, initialValues, args.where, args.args);
mutationCount.decrementAndGet(); mutationCount.decrementAndGet();