Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 03aae8eb authored by Treehugger Robot's avatar Treehugger Robot Committed by Android (Google) Code Review
Browse files

Merge changes from topic "295555884" into udc-qpr-dev

* changes:
  [SettingsProvider] clean up if a setting failed to be serialized
  [SettingsProvider] limit individual setting length
parents 592c04ab a71de26c
Loading
Loading
Loading
Loading
+50 −15
Original line number Original line Diff line number Diff line
@@ -94,6 +94,7 @@ final class SettingsState {


    static final int SETTINGS_VERSION_NEW_ENCODING = 121;
    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 WRITE_SETTINGS_DELAY_MILLIS = 200;
    private static final long MAX_WRITE_SETTINGS_DELAY_MILLIS = 2000;
    private static final long MAX_WRITE_SETTINGS_DELAY_MILLIS = 2000;


@@ -430,6 +431,19 @@ final class SettingsState {
            return false;
            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);
        Setting oldState = mSettings.get(name);
        String oldValue = (oldState != null) ? oldState.value : null;
        String oldValue = (oldState != null) ? oldState.value : null;
        String oldDefaultValue = (oldState != null) ? oldState.defaultValue : null;
        String oldDefaultValue = (oldState != null) ? oldState.defaultValue : null;
@@ -831,6 +845,7 @@ final class SettingsState {


    private void doWriteState() {
    private void doWriteState() {
        boolean wroteState = false;
        boolean wroteState = false;
        String settingFailedToBePersisted = null;
        final int version;
        final int version;
        final ArrayMap<String, Setting> settings;
        final ArrayMap<String, Setting> settings;
        final ArrayMap<String, String> namespaceBannedHashes;
        final ArrayMap<String, String> namespaceBannedHashes;
@@ -860,7 +875,6 @@ final class SettingsState {


                final int settingCount = settings.size();
                final int settingCount = settings.size();
                for (int i = 0; i < settingCount; i++) {
                for (int i = 0; i < settingCount; i++) {

                    Setting setting = settings.valueAt(i);
                    Setting setting = settings.valueAt(i);
                    if (setting.isTransient()) {
                    if (setting.isTransient()) {
                        if (DEBUG_PERSISTENCE) {
                        if (DEBUG_PERSISTENCE) {
@@ -869,8 +883,11 @@ final class SettingsState {
                        continue;
                        continue;
                    }
                    }


                    if (writeSingleSetting(mVersion, serializer, setting.getId(), setting.getName(),
                    try {
                            setting.getValue(), setting.getDefaultValue(), setting.getPackageName(),
                        if (writeSingleSetting(mVersion, serializer, setting.getId(),
                                setting.getName(),
                                setting.getValue(), setting.getDefaultValue(),
                                setting.getPackageName(),
                                setting.getTag(), setting.isDefaultFromSystem(),
                                setting.getTag(), setting.isDefaultFromSystem(),
                                setting.isValuePreservedInRestore())) {
                                setting.isValuePreservedInRestore())) {
                            if (DEBUG_PERSISTENCE) {
                            if (DEBUG_PERSISTENCE) {
@@ -878,6 +895,16 @@ final class SettingsState {
                                        + setting.getValue());
                                        + 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);
                serializer.endTag(null, TAG_SETTINGS);


@@ -902,14 +929,14 @@ final class SettingsState {
                    Slog.i(LOG_TAG, "[PERSIST END]");
                    Slog.i(LOG_TAG, "[PERSIST END]");
                }
                }
            } catch (Throwable t) {
            } 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 (t instanceof IOException) {
                    if (t.getMessage().contains("Couldn't create directory")) {
                        if (DEBUG) {
                        if (DEBUG) {
                            // we failed to create a directory, so log the permissions and existence
                            // we failed to create a directory, so log the permissions and existence
                            // state for the settings file and directory
                            // state for the settings file and directory
                            logSettingsDirectoryInformation(destination.getBaseFile());
                            logSettingsDirectoryInformation(destination.getBaseFile());
                        }
                        }
                    if (t.getMessage().contains("Couldn't create directory")) {
                        // attempt to create the directory with Files.createDirectories, which
                        // attempt to create the directory with Files.createDirectories, which
                        // throws more informative errors than File.mkdirs.
                        // throws more informative errors than File.mkdirs.
                        Path parentPath = destination.getBaseFile().getParentFile().toPath();
                        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) {
            synchronized (mLock) {
                addHistoricalOperationLocked(HISTORICAL_OPERATION_PERSIST, null);
                addHistoricalOperationLocked(HISTORICAL_OPERATION_PERSIST, null);
            }
            }
+66 −20
Original line number Original line Diff line number Diff line
@@ -408,4 +408,50 @@ public class SettingsStateTest extends AndroidTestCase {
        }
        }
        assertEquals(expectedMemUsage, settingsState.getMemoryUsage(TEST_PACKAGE));
        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 "));
            }
        }
    }
}
}