From b12f13efd8d0803b906a83ea5f689097a24b69c6 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 18 Jun 2015 15:11:01 -0700 Subject: [PATCH] Force date/time preferences to valid values Summary: Fixes T8601. To reproduce the problem: - Set your time preference to `""` (the empty string). This isn't possible from the modern UI, but can be done with "Right Click > Inspect Element", or users may have carried it forward from an older setting (this is the case with me and @hach-que on this install). - Load Calendar with some events. - This parses an epoch, which sets `valueTime` to `""` (since there are no format characters in the preference) and then `getEpoch()` fails because `strlen($time)` is 0. - Since `getEpoch()` failed, `getDateTime()` also fails. To fix this: - Only permit the date and time preferences to have valid values. Test Plan: - Loaded page before patch, saw fatal. - Applied patch. - No more fatal. - Viewed tooltips, dates/times, dates/times in other apps. - Changed my preferences, saw them respected. Reviewers: lpriestley Reviewed By: lpriestley Subscribers: epriestley, hach-que Maniphest Tasks: T8601 Differential Revision: https://secure.phabricator.com/D13346 --- src/__phutil_library_map__.php | 1 - .../people/storage/PhabricatorUser.php | 35 +++++++++++++ .../form/control/AphrontFormDateControl.php | 14 ++--- .../control/AphrontFormDateControlValue.php | 51 +++++++++---------- .../phui/calendar/PHUICalendarListView.php | 7 +-- src/view/viewutils.php | 19 ++----- 6 files changed, 68 insertions(+), 59 deletions(-) diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index fc465aeea6..5d9b7ad113 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -3290,7 +3290,6 @@ phutil_register_library_map(array( 'phabricator_format_local_time' => 'view/viewutils.php', 'phabricator_relative_date' => 'view/viewutils.php', 'phabricator_time' => 'view/viewutils.php', - 'phabricator_time_format' => 'view/viewutils.php', 'phid_get_subtype' => 'applications/phid/utils.php', 'phid_get_type' => 'applications/phid/utils.php', 'phid_group_by_type' => 'applications/phid/utils.php', diff --git a/src/applications/people/storage/PhabricatorUser.php b/src/applications/people/storage/PhabricatorUser.php index 55ade255a7..b4985bfaa5 100644 --- a/src/applications/people/storage/PhabricatorUser.php +++ b/src/applications/people/storage/PhabricatorUser.php @@ -738,6 +738,41 @@ final class PhabricatorUser return new DateTimeZone($this->getTimezoneIdentifier()); } + public function getPreference($key) { + $preferences = $this->loadPreferences(); + + // TODO: After T4103 and T7707 this should eventually be pushed down the + // stack into modular preference definitions and role profiles. This is + // just fixing T8601 and mildly anticipating those changes. + $value = $preferences->getPreference($key); + + $allowed_values = null; + switch ($key) { + case PhabricatorUserPreferences::PREFERENCE_TIME_FORMAT: + $allowed_values = array( + 'g:i A', + 'H:i', + ); + break; + case PhabricatorUserPreferences::PREFERENCE_DATE_FORMAT: + $allowed_values = array( + 'Y-m-d', + 'n/j/Y', + 'd-m-Y', + ); + break; + } + + if ($allowed_values !== null) { + $allowed_values = array_fuse($allowed_values); + if (empty($allowed_values[$value])) { + $value = head($allowed_values); + } + } + + return $value; + } + public function __toString() { return $this->getUsername(); } diff --git a/src/view/form/control/AphrontFormDateControl.php b/src/view/form/control/AphrontFormDateControl.php index 20172593e9..ca803c9bd1 100644 --- a/src/view/form/control/AphrontFormDateControl.php +++ b/src/view/form/control/AphrontFormDateControl.php @@ -137,19 +137,13 @@ final class AphrontFormDateControl extends AphrontFormControl { } private function getTimeFormat() { - $viewer = $this->getUser(); - $preferences = $viewer->loadPreferences(); - $pref_time_format = PhabricatorUserPreferences::PREFERENCE_TIME_FORMAT; - - return $preferences->getPreference($pref_time_format, 'g:i A'); + return $this->getUser() + ->getPreference(PhabricatorUserPreferences::PREFERENCE_TIME_FORMAT); } private function getDateFormat() { - $viewer = $this->getUser(); - $preferences = $viewer->loadPreferences(); - $pref_date_format = PhabricatorUserPreferences::PREFERENCE_DATE_FORMAT; - - return $preferences->getPreference($pref_date_format, 'Y-m-d'); + return $this->getUser() + ->getPreference(PhabricatorUserPreferences::PREFERENCE_DATE_FORMAT); } private function getTimeInputValue() { diff --git a/src/view/form/control/AphrontFormDateControlValue.php b/src/view/form/control/AphrontFormDateControlValue.php index c5d7e81852..f533bcaddb 100644 --- a/src/view/form/control/AphrontFormDateControlValue.php +++ b/src/view/form/control/AphrontFormDateControlValue.php @@ -10,7 +10,6 @@ final class AphrontFormDateControlValue extends Phobject { private $zone; private $optional; - public function getValueDate() { return $this->valueDate; } @@ -56,6 +55,10 @@ final class AphrontFormDateControlValue extends Phobject { return $this->optional; } + public function getViewer() { + return $this->viewer; + } + public static function newFromParts( PhabricatorUser $viewer, $year, @@ -71,8 +74,7 @@ final class AphrontFormDateControlValue extends Phobject { $year, $month, $day, - coalesce($time, '12:00 AM'), - $value); + coalesce($time, '12:00 AM')); $value->valueEnabled = $enabled; return $value; @@ -85,8 +87,7 @@ final class AphrontFormDateControlValue extends Phobject { list($value->valueDate, $value->valueTime) = $value->getFormattedDateFromDate( $request->getStr($key.'_d'), - $request->getStr($key.'_t'), - $value); + $request->getStr($key.'_t')); $value->valueEnabled = $request->getStr($key.'_e'); return $value; @@ -108,8 +109,7 @@ final class AphrontFormDateControlValue extends Phobject { $year, $month, $day, - $time, - $value); + $time); return $value; } @@ -123,8 +123,7 @@ final class AphrontFormDateControlValue extends Phobject { list($value->valueDate, $value->valueTime) = $value->getFormattedDateFromDate( idx($dictionary, 'd'), - idx($dictionary, 't'), - $value); + idx($dictionary, 't')); $value->valueEnabled = idx($dictionary, 'e'); @@ -205,29 +204,25 @@ final class AphrontFormDateControlValue extends Phobject { } private function getTimeFormat() { - $preferences = $this->viewer->loadPreferences(); - $pref_time_format = PhabricatorUserPreferences::PREFERENCE_TIME_FORMAT; - - return $preferences->getPreference($pref_time_format, 'g:i A'); + return $this->getViewer() + ->getPreference(PhabricatorUserPreferences::PREFERENCE_TIME_FORMAT); } private function getDateFormat() { - $preferences = $this->viewer->loadPreferences(); - $pref_date_format = PhabricatorUserPreferences::PREFERENCE_DATE_FORMAT; - - return $preferences->getPreference($pref_date_format, 'Y-m-d'); + return $this->getViewer() + ->getPreference(PhabricatorUserPreferences::PREFERENCE_DATE_FORMAT); } - private function getFormattedDateFromDate($date, $time, $value) { + private function getFormattedDateFromDate($date, $time) { $original_input = $date; - $zone = $value->getTimezone(); - $separator = $value->getFormatSeparator(); + $zone = $this->getTimezone(); + $separator = $this->getFormatSeparator(); $parts = preg_split('@[,./:-]@', $date); $date = implode($separator, $parts); $date = id(new DateTime($date, $zone)); if ($date) { - $date = $date->format($value->getDateFormat()); + $date = $date->format($this->getDateFormat()); } else { $date = $original_input; } @@ -235,8 +230,8 @@ final class AphrontFormDateControlValue extends Phobject { $date = id(new DateTime("{$date} {$time}", $zone)); return array( - $date->format($value->getDateFormat()), - $date->format($value->getTimeFormat()), + $date->format($this->getDateFormat()), + $date->format($this->getTimeFormat()), ); } @@ -244,14 +239,14 @@ final class AphrontFormDateControlValue extends Phobject { $year, $month, $day, - $time, - $value) { - $zone = $value->getTimezone(); + $time) { + + $zone = $this->getTimezone(); $date_time = id(new DateTime("{$year}-{$month}-{$day} {$time}", $zone)); return array( - $date_time->format($value->getDateFormat()), - $date_time->format($value->getTimeFormat()), + $date_time->format($this->getDateFormat()), + $date_time->format($this->getTimeFormat()), ); } diff --git a/src/view/phui/calendar/PHUICalendarListView.php b/src/view/phui/calendar/PHUICalendarListView.php index 80468dcb62..4de84ce3fd 100644 --- a/src/view/phui/calendar/PHUICalendarListView.php +++ b/src/view/phui/calendar/PHUICalendarListView.php @@ -141,11 +141,8 @@ final class PHUICalendarListView extends AphrontTagView { } private function getEventTooltip(AphrontCalendarEventView $event) { - $viewer = $this->getUser(); - $preferences = $viewer->loadPreferences(); - $time_pref = $preferences->getPreference( - PhabricatorUserPreferences::PREFERENCE_TIME_FORMAT, - 'g:i A'); + $time_pref = $this->getUser() + ->getPreference(PhabricatorUserPreferences::PREFERENCE_TIME_FORMAT); Javelin::initBehavior('phabricator-tooltips'); diff --git a/src/view/viewutils.php b/src/view/viewutils.php index 990c811c84..8d57cd02d0 100644 --- a/src/view/viewutils.php +++ b/src/view/viewutils.php @@ -31,32 +31,21 @@ function phabricator_relative_date($epoch, $user, $on = false) { } function phabricator_time($epoch, $user) { + $time_key = PhabricatorUserPreferences::PREFERENCE_TIME_FORMAT; return phabricator_format_local_time( $epoch, $user, - phabricator_time_format($user)); + $user->getPreference($time_key)); } function phabricator_datetime($epoch, $user) { + $time_key = PhabricatorUserPreferences::PREFERENCE_TIME_FORMAT; return phabricator_format_local_time( $epoch, $user, pht('%s, %s', phutil_date_format($epoch), - phabricator_time_format($user))); -} - -function phabricator_time_format($user) { - $prefs = $user->loadPreferences(); - - $pref = $prefs->getPreference( - PhabricatorUserPreferences::PREFERENCE_TIME_FORMAT); - - if (strlen($pref)) { - return $pref; - } - - return pht('g:i A'); + $user->getPreference($time_key))); } /**