[SettingsProvider] mem limit should be checked before settings are updated am: d85a428210

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/20211078

Change-Id: If42885e3a174636bc373194f47495d12a9f0bce2
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

packages/SettingsProvider/Android.bp

@@ -24,7 +24,10 @@ android_test {
         "src/com/android/providers/settings/SettingsState.java",
         "src/com/android/providers/settings/SettingsHelper.java",
     ],
-    static_libs: ["androidx.test.rules"],
+    static_libs: [
+        "androidx.test.rules",
+        "truth-prebuilt",
+    ],
     libs: ["android.test.base"],
     resource_dirs: ["res"],
     aaptflags: [

packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java

@@ -369,9 +369,11 @@ final class SettingsState {
             Setting newSetting = new Setting(name, oldSetting.getValue(), null,
                     oldSetting.getPackageName(), oldSetting.getTag(), false,
                     oldSetting.getId());
-            mSettings.put(name, newSetting);
+            int newSize = getNewMemoryUsagePerPackageLocked(newSetting.getPackageName(), oldValue,
-            updateMemoryUsagePerPackageLocked(newSetting.getPackageName(), oldValue,
                     newSetting.getValue(), oldDefaultValue, newSetting.getDefaultValue());
+            checkNewMemoryUsagePerPackageLocked(newSetting.getPackageName(), newSize);
+            mSettings.put(name, newSetting);
+            updateMemoryUsagePerPackageLocked(newSetting.getPackageName(), newSize);
             scheduleWriteIfNeededLocked();
         }
     }
@@ -394,6 +396,12 @@ final class SettingsState {
         Setting oldState = mSettings.get(name);
         String oldValue = (oldState != null) ? oldState.value : null;
         String oldDefaultValue = (oldState != null) ? oldState.defaultValue : null;
+        String newDefaultValue = makeDefault ? value : oldDefaultValue;
+
+        int newSize = getNewMemoryUsagePerPackageLocked(packageName, oldValue, value,
+                oldDefaultValue, newDefaultValue);
+        checkNewMemoryUsagePerPackageLocked(packageName, newSize);
+
         Setting newState;
 
         if (oldState != null) {
@@ -411,8 +419,7 @@ final class SettingsState {
 
         addHistoricalOperationLocked(HISTORICAL_OPERATION_UPDATE, newState);
 
-        updateMemoryUsagePerPackageLocked(packageName, oldValue, value,
+        updateMemoryUsagePerPackageLocked(packageName, newSize);
-                oldDefaultValue, newState.getDefaultValue());
 
         scheduleWriteIfNeededLocked();
 
@@ -433,13 +440,14 @@ final class SettingsState {
         }
 
         Setting oldState = mSettings.remove(name);
+        int newSize = getNewMemoryUsagePerPackageLocked(oldState.packageName, oldState.value,
+                null, oldState.defaultValue, null);
 
         StatsLog.write(StatsLog.SETTING_CHANGED, name, /* value= */ "", /* newValue= */ "",
             oldState.value, /* tag */ "", false, getUserIdFromKey(mKey),
             StatsLog.SETTING_CHANGED__REASON__DELETED);
 
-        updateMemoryUsagePerPackageLocked(oldState.packageName, oldState.value,
+        updateMemoryUsagePerPackageLocked(oldState.packageName, newSize);
-                null, oldState.defaultValue, null);
 
         addHistoricalOperationLocked(HISTORICAL_OPERATION_DELETE, oldState);
 
@@ -460,16 +468,18 @@ final class SettingsState {
         Setting oldSetting = new Setting(setting);
         String oldValue = setting.getValue();
         String oldDefaultValue = setting.getDefaultValue();
+        String newValue = oldDefaultValue;
+        String newDefaultValue = oldDefaultValue;
+
+        int newSize = getNewMemoryUsagePerPackageLocked(setting.packageName, oldValue,
+                newValue, oldDefaultValue, newDefaultValue);
+        checkNewMemoryUsagePerPackageLocked(setting.packageName, newSize);
 
         if (!setting.reset()) {
             return false;
         }
 
-        String newValue = setting.getValue();
+        updateMemoryUsagePerPackageLocked(setting.packageName, newSize);
-        String newDefaultValue = setting.getDefaultValue();
-
-        updateMemoryUsagePerPackageLocked(setting.packageName, oldValue,
-                newValue, oldDefaultValue, newDefaultValue);
 
         addHistoricalOperationLocked(HISTORICAL_OPERATION_RESET, oldSetting);
 
@@ -577,38 +587,49 @@ final class SettingsState {
     }
 
     @GuardedBy("mLock")
-    private void updateMemoryUsagePerPackageLocked(String packageName, String oldValue,
+    private boolean isExemptFromMemoryUsageCap(String packageName) {
+        return mMaxBytesPerAppPackage == MAX_BYTES_PER_APP_PACKAGE_UNLIMITED
+                || SYSTEM_PACKAGE_NAME.equals(packageName);
+    }
+
+    @GuardedBy("mLock")
+    private void checkNewMemoryUsagePerPackageLocked(String packageName, int newSize)
+            throws IllegalStateException {
+        if (isExemptFromMemoryUsageCap(packageName)) {
+            return;
+        }
+        if (newSize > mMaxBytesPerAppPackage) {
+            throw new IllegalStateException("You are adding too many system settings. "
+                    + "You should stop using system settings for app specific data"
+                    + " package: " + packageName);
+        }
+    }
+
+    @GuardedBy("mLock")
+    private int getNewMemoryUsagePerPackageLocked(String packageName, String oldValue,
             String newValue, String oldDefaultValue, String newDefaultValue) {
-        if (mMaxBytesPerAppPackage == MAX_BYTES_PER_APP_PACKAGE_UNLIMITED) {
+        if (isExemptFromMemoryUsageCap(packageName)) {
-            return;
+            return 0;
         }
-
+        final Integer currentSize = mPackageToMemoryUsage.get(packageName);
-        if (SYSTEM_PACKAGE_NAME.equals(packageName)) {
-            return;
-        }
-
         final int oldValueSize = (oldValue != null) ? oldValue.length() : 0;
         final int newValueSize = (newValue != null) ? newValue.length() : 0;
         final int oldDefaultValueSize = (oldDefaultValue != null) ? oldDefaultValue.length() : 0;
         final int newDefaultValueSize = (newDefaultValue != null) ? newDefaultValue.length() : 0;
         final int deltaSize = newValueSize + newDefaultValueSize
                 - oldValueSize - oldDefaultValueSize;
+        return Math.max((currentSize != null) ? currentSize + deltaSize : deltaSize, 0);
+    }
 
-        Integer currentSize = mPackageToMemoryUsage.get(packageName);
+    @GuardedBy("mLock")
-        final int newSize = Math.max((currentSize != null)
+    private void updateMemoryUsagePerPackageLocked(String packageName, int newSize) {
-                ? currentSize + deltaSize : deltaSize, 0);
+        if (isExemptFromMemoryUsageCap(packageName)) {
-
+            return;
-        if (newSize > mMaxBytesPerAppPackage) {
-            throw new IllegalStateException("You are adding too many system settings. "
-                    + "You should stop using system settings for app specific data"
-                    + " package: " + packageName);
         }
-
         if (DEBUG) {
             Slog.i(LOG_TAG, "Settings for package: " + packageName
                     + " size: " + newSize + " bytes.");
         }
-
         mPackageToMemoryUsage.put(packageName, newSize);
     }
 

packages/SettingsProvider/test/src/com/android/providers/settings/SettingsStateTest.java

@@ -21,6 +21,8 @@ import android.util.Xml;
 
 import org.xmlpull.v1.XmlSerializer;
 
+import com.google.common.base.Strings;
+
 import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.FileOutputStream;
@@ -46,7 +48,6 @@ public class SettingsStateTest extends AndroidTestCase {
             "\uD800ab\uDC00 " + // broken surrogate pairs
             "日本語";
 
-
     public void testIsBinary() {
         assertFalse(SettingsState.isBinary(" abc 日本語"));
 
@@ -182,4 +183,44 @@ public class SettingsStateTest extends AndroidTestCase {
             assertEquals("p2", s.getPackageName());
         }
     }
+
+    public void testInsertSetting_memoryUsage() {
+        final Object lock = new Object();
+	final File file = new File(getContext().getCacheDir(), "setting.xml");
+	final String settingName = "test_setting";
+
+        SettingsState settingsState = new SettingsState(getContext(), lock, file, 1,
+                SettingsState.MAX_BYTES_PER_APP_PACKAGE_UNLIMITED, Looper.getMainLooper());
+        // No exception should be thrown when there is no cap
+        settingsState.insertSettingLocked(settingName, Strings.repeat("A", 20001),
+                null, false, "p1");
+        settingsState.deleteSettingLocked(settingName);
+
+        settingsState = new SettingsState(getContext(), lock, file, 1,
+                SettingsState.MAX_BYTES_PER_APP_PACKAGE_LIMITED, Looper.getMainLooper());
+        // System package doesn't have memory usage limit
+        settingsState.insertSettingLocked(settingName, Strings.repeat("A", 20001),
+                null, false, "android");
+        settingsState.deleteSettingLocked(settingName);
+
+        // Should not throw if usage is under the cap
+        settingsState.insertSettingLocked(settingName, Strings.repeat("A", 19999),
+                null, false, "p1");
+        settingsState.deleteSettingLocked(settingName);
+        try {
+            settingsState.insertSettingLocked(settingName, Strings.repeat("A", 20001),
+                    null, false, "p1");
+            fail("Should throw because it exceeded per package memory usage");
+        } catch (IllegalStateException ex) {
+            assertTrue(ex.getMessage().contains("p1"));
+        }
+        try {
+            settingsState.insertSettingLocked(settingName, Strings.repeat("A", 20001),
+                    null, false, "p1");
+            fail("Should throw because it exceeded per package memory usage");
+        } catch (IllegalStateException ex) {
+            assertTrue(ex.getMessage().contains("p1"));
+        }
+        assertTrue(settingsState.getSettingLocked(settingName).isNull());
+    }
 }