diff options
| -rw-r--r-- | packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java | 65 | ||||
| -rw-r--r-- | packages/SettingsProvider/test/src/com/android/providers/settings/SettingsStateTest.java | 86 |
2 files changed, 116 insertions, 35 deletions
diff --git a/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java b/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java index e3153e046a96..ac65b20aefc0 100644 --- a/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java +++ b/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java @@ -94,6 +94,7 @@ final class SettingsState { static final int SETTINGS_VERSION_NEW_ENCODING = 121; + public static final int MAX_LENGTH_PER_STRING = 32768; private static final long WRITE_SETTINGS_DELAY_MILLIS = 200; private static final long MAX_WRITE_SETTINGS_DELAY_MILLIS = 2000; @@ -430,6 +431,19 @@ final class SettingsState { return false; } + final boolean isNameTooLong = name.length() > SettingsState.MAX_LENGTH_PER_STRING; + final boolean isValueTooLong = + value != null && value.length() > SettingsState.MAX_LENGTH_PER_STRING; + if (isNameTooLong || isValueTooLong) { + // only print the first few bytes of the name in case it is long + final String errorMessage = "The " + (isNameTooLong ? "name" : "value") + + " of your setting [" + + (name.length() > 20 ? (name.substring(0, 20) + "...") : name) + + "] is too long. The max length allowed for the string is " + + MAX_LENGTH_PER_STRING + "."; + throw new IllegalArgumentException(errorMessage); + } + Setting oldState = mSettings.get(name); String oldValue = (oldState != null) ? oldState.value : null; String oldDefaultValue = (oldState != null) ? oldState.defaultValue : null; @@ -831,6 +845,7 @@ final class SettingsState { private void doWriteState() { boolean wroteState = false; + String settingFailedToBePersisted = null; final int version; final ArrayMap<String, Setting> settings; final ArrayMap<String, String> namespaceBannedHashes; @@ -860,7 +875,6 @@ final class SettingsState { final int settingCount = settings.size(); for (int i = 0; i < settingCount; i++) { - Setting setting = settings.valueAt(i); if (setting.isTransient()) { if (DEBUG_PERSISTENCE) { @@ -869,14 +883,27 @@ final class SettingsState { continue; } - if (writeSingleSetting(mVersion, serializer, setting.getId(), setting.getName(), - setting.getValue(), setting.getDefaultValue(), setting.getPackageName(), - setting.getTag(), setting.isDefaultFromSystem(), - setting.isValuePreservedInRestore())) { - if (DEBUG_PERSISTENCE) { - Slog.i(LOG_TAG, "[PERSISTED]" + setting.getName() + "=" - + setting.getValue()); + try { + if (writeSingleSetting(mVersion, serializer, setting.getId(), + setting.getName(), + setting.getValue(), setting.getDefaultValue(), + setting.getPackageName(), + setting.getTag(), setting.isDefaultFromSystem(), + setting.isValuePreservedInRestore())) { + if (DEBUG_PERSISTENCE) { + Slog.i(LOG_TAG, "[PERSISTED]" + setting.getName() + "=" + + setting.getValue()); + } } + } catch (IOException ex) { + Slog.e(LOG_TAG, "[ABORT PERSISTING]" + setting.getName() + + " due to error writing to disk", ex); + // A setting failed to be written. Abort the serialization to avoid leaving + // a partially serialized setting on disk, which can cause parsing errors. + // Note down the problematic setting, so that we can delete it before trying + // again to persist the rest of the settings. + settingFailedToBePersisted = setting.getName(); + throw ex; } } serializer.endTag(null, TAG_SETTINGS); @@ -902,14 +929,14 @@ final class SettingsState { Slog.i(LOG_TAG, "[PERSIST END]"); } } catch (Throwable t) { - Slog.wtf(LOG_TAG, "Failed to write settings, restoring backup", t); + Slog.wtf(LOG_TAG, "Failed to write settings, restoring old file", t); if (t instanceof IOException) { - if (DEBUG) { - // we failed to create a directory, so log the permissions and existence - // state for the settings file and directory - logSettingsDirectoryInformation(destination.getBaseFile()); - } if (t.getMessage().contains("Couldn't create directory")) { + if (DEBUG) { + // we failed to create a directory, so log the permissions and existence + // state for the settings file and directory + logSettingsDirectoryInformation(destination.getBaseFile()); + } // attempt to create the directory with Files.createDirectories, which // throws more informative errors than File.mkdirs. Path parentPath = destination.getBaseFile().getParentFile().toPath(); @@ -930,7 +957,15 @@ final class SettingsState { } } - if (wroteState) { + if (!wroteState) { + if (settingFailedToBePersisted != null) { + synchronized (mLock) { + // Delete the problematic setting. This will schedule a write as well. + deleteSettingLocked(settingFailedToBePersisted); + } + } + } else { + // success synchronized (mLock) { addHistoricalOperationLocked(HISTORICAL_OPERATION_PERSIST, null); } diff --git a/packages/SettingsProvider/test/src/com/android/providers/settings/SettingsStateTest.java b/packages/SettingsProvider/test/src/com/android/providers/settings/SettingsStateTest.java index 55160fbfd92e..26cac37f8b35 100644 --- a/packages/SettingsProvider/test/src/com/android/providers/settings/SettingsStateTest.java +++ b/packages/SettingsProvider/test/src/com/android/providers/settings/SettingsStateTest.java @@ -31,21 +31,21 @@ import java.io.PrintStream; public class SettingsStateTest extends AndroidTestCase { public static final String CRAZY_STRING = "\u0000\u0001\u0002\u0003\u0004\u0005\u0006\u0007\u0008\u0009\n\u000b\u000c\r" + - "\u000e\u000f\u0010\u0011\u0012\u0013\u0014\u0015\u0016\u0017\u0018\u0019\u001a" + - "\u001b\u001c\u001d\u001e\u001f\u0020" + - "fake_setting_value_1" + - "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + - "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + - "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + - "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + - "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + - "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + - "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + - "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + - "\u1000 \u2000 \u5000 \u8000 \uc000 \ue000" + - "\ud800\udc00\udbff\udfff" + // surrogate pairs - "\uD800ab\uDC00 " + // broken surrogate pairs - "日本語"; + "\u000e\u000f\u0010\u0011\u0012\u0013\u0014\u0015\u0016\u0017\u0018\u0019\u001a" + + "\u001b\u001c\u001d\u001e\u001f\u0020" + + "fake_setting_value_1" + + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + + "\u1000 \u2000 \u5000 \u8000 \uc000 \ue000" + + "\ud800\udc00\udbff\udfff" + // surrogate pairs + "\uD800ab\uDC00 " + // broken surrogate pairs + "日本語"; private static final String TEST_PACKAGE = "package"; private static final String SYSTEM_PACKAGE = "android"; @@ -170,11 +170,11 @@ public class SettingsStateTest extends AndroidTestCase { final PrintStream os = new PrintStream(new FileOutputStream(file)); os.print( "<?xml version='1.0' encoding='UTF-8' standalone='yes' ?>" + - "<settings version=\"120\">" + - " <setting id=\"0\" name=\"k0\" value=\"null\" package=\"null\" />" + - " <setting id=\"1\" name=\"k1\" value=\"\" package=\"\" />" + - " <setting id=\"2\" name=\"k2\" value=\"v2\" package=\"p2\" />" + - "</settings>"); + "<settings version=\"120\">" + + " <setting id=\"0\" name=\"k0\" value=\"null\" package=\"null\" />" + + " <setting id=\"1\" name=\"k1\" value=\"\" package=\"\" />" + + " <setting id=\"2\" name=\"k2\" value=\"v2\" package=\"p2\" />" + + "</settings>"); os.close(); final SettingsState ss = new SettingsState(getContext(), lock, file, 1, @@ -408,4 +408,50 @@ public class SettingsStateTest extends AndroidTestCase { } assertEquals(expectedMemUsage, settingsState.getMemoryUsage(TEST_PACKAGE)); } + + public void testLargeSettingKey() { + SettingsState settingsState = new SettingsState(getContext(), mLock, mSettingsFile, 1, + SettingsState.MAX_BYTES_PER_APP_PACKAGE_LIMITED, Looper.getMainLooper()); + final String largeKey = Strings.repeat("A", SettingsState.MAX_LENGTH_PER_STRING + 1); + final String testValue = "testValue"; + synchronized (mLock) { + // Test system package + try { + settingsState.insertSettingLocked(largeKey, testValue, null, true, SYSTEM_PACKAGE); + fail("Should throw because it exceeded max string length"); + } catch (IllegalArgumentException ex) { + assertTrue(ex.getMessage().contains("The max length allowed for the string is ")); + } + // Test non system package + try { + settingsState.insertSettingLocked(largeKey, testValue, null, true, TEST_PACKAGE); + fail("Should throw because it exceeded max string length"); + } catch (IllegalArgumentException ex) { + assertTrue(ex.getMessage().contains("The max length allowed for the string is ")); + } + } + } + + public void testLargeSettingValue() { + SettingsState settingsState = new SettingsState(getContext(), mLock, mSettingsFile, 1, + SettingsState.MAX_BYTES_PER_APP_PACKAGE_UNLIMITED, Looper.getMainLooper()); + final String testKey = "testKey"; + final String largeValue = Strings.repeat("A", SettingsState.MAX_LENGTH_PER_STRING + 1); + synchronized (mLock) { + // Test system package + try { + settingsState.insertSettingLocked(testKey, largeValue, null, true, SYSTEM_PACKAGE); + fail("Should throw because it exceeded max string length"); + } catch (IllegalArgumentException ex) { + assertTrue(ex.getMessage().contains("The max length allowed for the string is ")); + } + // Test non system package + try { + settingsState.insertSettingLocked(testKey, largeValue, null, true, TEST_PACKAGE); + fail("Should throw because it exceeded max string length"); + } catch (IllegalArgumentException ex) { + assertTrue(ex.getMessage().contains("The max length allowed for the string is ")); + } + } + } } |