From f21e14afc7778eb7e88708b310de657e55d5f7a9 Mon Sep 17 00:00:00 2001 From: cketti Date: Sat, 8 Oct 2011 17:58:57 +0200 Subject: [PATCH] Added input validation for identity settings --- src/com/fsck/k9/Account.java | 11 --- .../fsck/k9/preferences/IdentitySettings.java | 99 +++++++++++++++++++ src/com/fsck/k9/preferences/Settings.java | 3 +- .../fsck/k9/preferences/StorageExporter.java | 56 +++++++---- .../fsck/k9/preferences/StorageImporter.java | 65 +++++++++--- 5 files changed, 185 insertions(+), 49 deletions(-) create mode 100644 src/com/fsck/k9/preferences/IdentitySettings.java diff --git a/src/com/fsck/k9/Account.java b/src/com/fsck/k9/Account.java index cccabd157..94de6abb2 100644 --- a/src/com/fsck/k9/Account.java +++ b/src/com/fsck/k9/Account.java @@ -22,12 +22,10 @@ import java.util.ArrayList; import java.util.Calendar; import java.util.Collections; import java.util.Date; -import java.util.HashSet; import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Random; -import java.util.Set; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; @@ -63,15 +61,6 @@ public class Account implements BaseAccount { public static final String IDENTITY_EMAIL_KEY = "email"; public static final String IDENTITY_DESCRIPTION_KEY = "description"; - public static final Set IDENTITY_KEYS = new HashSet(); - static { - IDENTITY_KEYS.add(IDENTITY_NAME_KEY); - IDENTITY_KEYS.add(IDENTITY_EMAIL_KEY); - IDENTITY_KEYS.add(IDENTITY_DESCRIPTION_KEY); - IDENTITY_KEYS.add("signatureUse"); - IDENTITY_KEYS.add("signature"); - IDENTITY_KEYS.add("replyTo"); - } /** *
diff --git a/src/com/fsck/k9/preferences/IdentitySettings.java b/src/com/fsck/k9/preferences/IdentitySettings.java
new file mode 100644
index 000000000..f796446e1
--- /dev/null
+++ b/src/com/fsck/k9/preferences/IdentitySettings.java
@@ -0,0 +1,99 @@
+package com.fsck.k9.preferences;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import android.content.SharedPreferences;
+
+import com.fsck.k9.EmailAddressValidator;
+import com.fsck.k9.K9;
+import com.fsck.k9.R;
+import com.fsck.k9.preferences.Settings.*;
+
+public class IdentitySettings {
+    public static final Map SETTINGS;
+
+    static {
+        Map s = new LinkedHashMap();
+
+        s.put("signature", new SignatureSetting());
+        s.put("signatureUse", new BooleanSetting(true));
+        s.put("replyTo", new OptionalEmailAddressSetting());
+
+        SETTINGS = Collections.unmodifiableMap(s);
+    }
+
+    public static Map validate(Map importedSettings,
+            boolean useDefaultValues) {
+        return Settings.validate(SETTINGS, importedSettings, useDefaultValues);
+    }
+
+    public static Map getIdentitySettings(SharedPreferences storage, String uuid,
+            int identityIndex) {
+        Map result = new HashMap();
+        String prefix = uuid + ".";
+        String suffix = "." + Integer.toString(identityIndex);
+        for (String key : SETTINGS.keySet()) {
+            String value = storage.getString(prefix + key + suffix, null);
+            if (value != null) {
+                result.put(key, value);
+            }
+        }
+        return result;
+    }
+
+
+    public static boolean isEmailAddressValid(String email) {
+        return new EmailAddressValidator().isValidAddressOnly(email);
+    }
+
+    /**
+     * The message signature setting.
+     */
+    public static class SignatureSetting extends SettingsDescription {
+        public SignatureSetting() {
+            super(null);
+        }
+
+        @Override
+        public Object getDefaultValue() {
+            return K9.app.getResources().getString(R.string.default_signature);
+        }
+
+        @Override
+        public Object fromString(String value) throws InvalidSettingValueException {
+            return value;
+        }
+    }
+
+    /**
+     * An optional email address setting.
+     */
+    public static class OptionalEmailAddressSetting extends SettingsDescription {
+        private EmailAddressValidator mValidator;
+
+        public OptionalEmailAddressSetting() {
+            super(null);
+            mValidator = new EmailAddressValidator();
+        }
+
+        @Override
+        public Object fromString(String value) throws InvalidSettingValueException {
+            if (value != null && !mValidator.isValidAddressOnly(value)) {
+                throw new InvalidSettingValueException();
+            }
+            return value;
+        }
+
+        @Override
+        public String toPrettyString(Object value) {
+            return (value == null) ? "" : value.toString();
+        }
+
+        @Override
+        public Object fromPrettyString(String value) throws InvalidSettingValueException {
+            return ("".equals(value)) ? null : fromString(value);
+        }
+    }
+}
diff --git a/src/com/fsck/k9/preferences/Settings.java b/src/com/fsck/k9/preferences/Settings.java
index fca782242..29f02636c 100644
--- a/src/com/fsck/k9/preferences/Settings.java
+++ b/src/com/fsck/k9/preferences/Settings.java
@@ -64,7 +64,8 @@ public class Settings {
 
             if (useDefaultValue) {
                 Object defaultValue = desc.getDefaultValue();
-                validatedSettings.put(key, desc.toString(defaultValue));
+                String value = (defaultValue != null) ? desc.toString(defaultValue) : null;
+                validatedSettings.put(key, value);
             }
         }
 
diff --git a/src/com/fsck/k9/preferences/StorageExporter.java b/src/com/fsck/k9/preferences/StorageExporter.java
index 4bccb544e..0134e947c 100644
--- a/src/com/fsck/k9/preferences/StorageExporter.java
+++ b/src/com/fsck/k9/preferences/StorageExporter.java
@@ -295,7 +295,7 @@ public class StorageExporter {
             if (comps.length >= 3) {
                 String thirdPart = comps[2];
 
-                if (Account.IDENTITY_KEYS.contains(secondPart)) {
+                if (Account.IDENTITY_DESCRIPTION_KEY.equals(secondPart)) {
                     // This is an identity key. Save identity index for later...
                     try {
                         identities.add(Integer.parseInt(thirdPart));
@@ -362,48 +362,62 @@ public class StorageExporter {
 
         serializer.startTag(null, IDENTITY_ELEMENT);
 
-        String name = (String) prefs.get(accountUuid + "." + Account.IDENTITY_NAME_KEY +
-                "." + identity);
+        String prefix = accountUuid + ".";
+        String suffix = "." + identity;
+
+        // Write name belonging to the identity
+        String name = (String) prefs.get(prefix + Account.IDENTITY_NAME_KEY + suffix);
         serializer.startTag(null, NAME_ELEMENT);
         serializer.text(name);
         serializer.endTag(null, NAME_ELEMENT);
 
-        String email = (String) prefs.get(accountUuid + "." + Account.IDENTITY_EMAIL_KEY +
-                "." + identity);
+        // Write email address belonging to the identity
+        String email = (String) prefs.get(prefix + Account.IDENTITY_EMAIL_KEY + suffix);
         serializer.startTag(null, EMAIL_ELEMENT);
         serializer.text(email);
         serializer.endTag(null, EMAIL_ELEMENT);
 
-        String description = (String) prefs.get(accountUuid + "." +
-                Account.IDENTITY_DESCRIPTION_KEY + "." + identity);
+        // Write identity description
+        String description = (String) prefs.get(prefix + Account.IDENTITY_DESCRIPTION_KEY + suffix);
         if (description != null) {
             serializer.startTag(null, DESCRIPTION_ELEMENT);
             serializer.text(description);
             serializer.endTag(null, DESCRIPTION_ELEMENT);
         }
 
+        // Write identity settings
         serializer.startTag(null, SETTINGS_ELEMENT);
         for (Map.Entry entry : prefs.entrySet()) {
             String key = entry.getKey();
-            String value = entry.getValue().toString();
+            String valueString = entry.getValue().toString();
             String[] comps = key.split("\\.");
-            if (comps.length >= 3) {
-                String keyUuid = comps[0];
-                String identityKey = comps[1];
-                String identityIndex = comps[2];
-                if (!keyUuid.equals(accountUuid) || !identityIndex.equals(identity)
-                        || !Account.IDENTITY_KEYS.contains(identityKey)
-                        || Account.IDENTITY_NAME_KEY.equals(identityKey)
-                        || Account.IDENTITY_EMAIL_KEY.equals(identityKey)
-                        || Account.IDENTITY_DESCRIPTION_KEY.equals(identityKey)) {
-                    continue;
-                }
-            } else {
+
+            if (comps.length < 3) {
                 // Skip non-identity config entries
                 continue;
             }
 
-            writeKeyValue(serializer, comps[1], value);
+            String keyUuid = comps[0];
+            String identityKey = comps[1];
+            String identityIndex = comps[2];
+            if (!keyUuid.equals(accountUuid) || !identityIndex.equals(identity)) {
+                // Skip entries that belong to another identity
+                continue;
+            }
+
+            SettingsDescription setting = IdentitySettings.SETTINGS.get(identityKey);
+            if (setting != null) {
+                // Only write settings that have an entry in IdentitySettings.SETTINGS
+                try {
+                    Object value = setting.fromString(valueString);
+                    String outputValue = setting.toPrettyString(value);
+                    writeKeyValue(serializer, identityKey, outputValue);
+                } catch (InvalidSettingValueException e) {
+                    Log.w(K9.LOG_TAG, "Identity setting \"" + identityKey +
+                            "\" has invalid value \"" + valueString +
+                            "\" in preference storage. This shouldn't happen!");
+                }
+            }
         }
         serializer.endTag(null, SETTINGS_ELEMENT);
 
diff --git a/src/com/fsck/k9/preferences/StorageImporter.java b/src/com/fsck/k9/preferences/StorageImporter.java
index 76ac954d2..3fff2fea1 100644
--- a/src/com/fsck/k9/preferences/StorageImporter.java
+++ b/src/com/fsck/k9/preferences/StorageImporter.java
@@ -28,6 +28,7 @@ import com.fsck.k9.mail.ConnectionSecurity;
 import com.fsck.k9.mail.ServerSettings;
 import com.fsck.k9.mail.Store;
 import com.fsck.k9.mail.Transport;
+import com.fsck.k9.preferences.Settings.InvalidSettingValueException;
 
 public class StorageImporter {
 
@@ -277,7 +278,8 @@ public class StorageImporter {
     }
 
     private static AccountDescriptionPair importAccount(Context context,
-            SharedPreferences.Editor editor, ImportedAccount account, boolean overwrite) {
+            SharedPreferences.Editor editor, ImportedAccount account, boolean overwrite)
+            throws InvalidSettingValueException {
 
         AccountDescription original = new AccountDescription(account.name, account.uuid);
 
@@ -352,7 +354,7 @@ public class StorageImporter {
         }
 
         if (account.identities != null) {
-            importIdentities(editor, uuid, account, overwrite, existingAccount);
+            importIdentities(editor, uuid, account, overwrite, existingAccount, prefs);
         }
 
         // Write folder settings
@@ -374,7 +376,8 @@ public class StorageImporter {
     }
 
     private static void importIdentities(SharedPreferences.Editor editor, String uuid,
-            ImportedAccount account, boolean overwrite, Account existingAccount) {
+            ImportedAccount account, boolean overwrite, Account existingAccount,
+            Preferences prefs) throws InvalidSettingValueException {
 
         String accountKeyPrefix = uuid + ".";
 
@@ -391,17 +394,20 @@ public class StorageImporter {
         // Write identities
         for (ImportedIdentity identity : account.identities) {
             int writeIdentityIndex = nextIdentityIndex;
-            if (existingIdentities.size() > 0) {
+            boolean mergeSettings = false;
+            if (overwrite && existingIdentities.size() > 0) {
                 int identityIndex = findIdentity(identity, existingIdentities);
-                if (overwrite && identityIndex != -1) {
+                if (identityIndex != -1) {
                     writeIdentityIndex = identityIndex;
+                    mergeSettings = true;
                 }
             }
-            if (writeIdentityIndex == nextIdentityIndex) {
+            if (!mergeSettings) {
                 nextIdentityIndex++;
             }
 
-            String identityDescription = identity.description;
+            String identityDescription = (identity.description == null) ?
+                    "Imported" : identity.description;
             if (isIdentityDescriptionUsed(identityDescription, existingIdentities)) {
                 // Identity description is already in use. So generate a new one by appending
                 // " (x)", where x is the first number >= 1 that results in an unused identity
@@ -414,18 +420,45 @@ public class StorageImporter {
                 }
             }
 
-            editor.putString(accountKeyPrefix + Account.IDENTITY_NAME_KEY + "." +
-                    writeIdentityIndex, identity.name);
-            editor.putString(accountKeyPrefix + Account.IDENTITY_EMAIL_KEY + "." +
-                    writeIdentityIndex, identity.email);
-            editor.putString(accountKeyPrefix + Account.IDENTITY_DESCRIPTION_KEY + "." +
-                    writeIdentityIndex, identityDescription);
+            String identitySuffix = "." + writeIdentityIndex;
+
+            // Write name used in identity
+            String identityName = (identity.name == null) ? "" : identity.name;
+            editor.putString(accountKeyPrefix + Account.IDENTITY_NAME_KEY + identitySuffix,
+                    identityName);
+
+            // Validate email address
+            if (!IdentitySettings.isEmailAddressValid(identity.email)) {
+                throw new InvalidSettingValueException();
+            }
+
+            // Write email address
+            editor.putString(accountKeyPrefix + Account.IDENTITY_EMAIL_KEY + identitySuffix,
+                    identity.email);
+
+            // Write identity description
+            editor.putString(accountKeyPrefix + Account.IDENTITY_DESCRIPTION_KEY + identitySuffix,
+                    identityDescription);
+
+            // Validate identity settings
+            Map validatedSettings = IdentitySettings.validate(
+                    identity.settings.settings, !mergeSettings);
+
+            // Merge identity settings if necessary
+            Map writeSettings;
+            if (mergeSettings) {
+                writeSettings = new HashMap(IdentitySettings.getIdentitySettings(
+                        prefs.getPreferences(), uuid, writeIdentityIndex));
+                writeSettings.putAll(validatedSettings);
+            } else {
+                writeSettings = new HashMap(validatedSettings);
+            }
 
             // Write identity settings
-            for (Map.Entry setting : identity.settings.settings.entrySet()) {
-                String key = setting.getKey();
+            for (Map.Entry setting : writeSettings.entrySet()) {
+                String key = accountKeyPrefix + setting.getKey() + identitySuffix;
                 String value = setting.getValue();
-                editor.putString(accountKeyPrefix + key + "." + writeIdentityIndex, value);
+                editor.putString(key, value);
             }
         }
     }