From 495092613019f8adaaff5d287d88cd8d8f886f40 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 7 Dec 2016 12:48:04 -0800
Subject: [PATCH] Validate settings before writing them to the user cache

Summary:
Fixes T11960. In D16998 I removed some code which validated settings on read to improve performance, but lost this replacement validation in shuffling the patch stack.

This restores similar validation before we write the cache. This has the same effect, it's just faster.

Also, bump the cache key to wipe out anything that got bitten (like my account on `secure` rendering dates wrong).

Test Plan:
  - Edited settings, verified the edits held.
  - Faked invalid settings, saw the check throw exceptions.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11960

Differential Revision: https://secure.phabricator.com/D17008
---
 .../cache/PhabricatorUserPreferencesCacheType.php    | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/src/applications/people/cache/PhabricatorUserPreferencesCacheType.php b/src/applications/people/cache/PhabricatorUserPreferencesCacheType.php
index 7fee680def..b4c8dc6a45 100644
--- a/src/applications/people/cache/PhabricatorUserPreferencesCacheType.php
+++ b/src/applications/people/cache/PhabricatorUserPreferencesCacheType.php
@@ -5,7 +5,7 @@ final class PhabricatorUserPreferencesCacheType
 
   const CACHETYPE = 'preferences';
 
-  const KEY_PREFERENCES = 'user.preferences.v1';
+  const KEY_PREFERENCES = 'user.preferences.v2';
 
   public function getAutoloadKeys() {
     return array(
@@ -47,6 +47,16 @@ final class PhabricatorUserPreferencesCacheType
       foreach ($all_settings as $key => $setting) {
         $value = $preference->getSettingValue($key);
 
+        try {
+          id(clone $setting)
+            ->setViewer($viewer)
+            ->assertValidValue($value);
+        } catch (Exception $ex) {
+          // If the saved value isn't valid, don't cache it: we'll use the
+          // default value instead.
+          continue;
+        }
+
         // As an optimization, we omit the value from the cache if it is
         // exactly the same as the hardcoded default.
         $default_value = id(clone $setting)