From 25738e78a1f5ca20fc1147918c3c0a3b6f8a41be Mon Sep 17 00:00:00 2001 From: epriestley Date: Sat, 9 Mar 2013 13:51:58 -0800 Subject: [PATCH] Harden custom date fields against userland adventures Summary: Users do things like change the type of a field. Currently, we throw when this happens. Instead, recover somewhat-gracefully. Test Plan: Created a "string" field, then changed it to a "date" field. {F35241} Reviewers: btrahan, chad Reviewed By: chad CC: aran Differential Revision: https://secure.phabricator.com/D5310 --- ...hestAuxiliaryFieldDefaultSpecification.php | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/src/applications/maniphest/auxiliaryfield/ManiphestAuxiliaryFieldDefaultSpecification.php b/src/applications/maniphest/auxiliaryfield/ManiphestAuxiliaryFieldDefaultSpecification.php index 41d3009618..af020fcc91 100644 --- a/src/applications/maniphest/auxiliaryfield/ManiphestAuxiliaryFieldDefaultSpecification.php +++ b/src/applications/maniphest/auxiliaryfield/ManiphestAuxiliaryFieldDefaultSpecification.php @@ -195,6 +195,12 @@ class ManiphestAuxiliaryFieldDefaultSpecification $value = array(); } break; + case self::TYPE_DATE: + $value = (int)$value; + if ($value <= 0) { + return $this->setDefaultValue($value); + } + break; default: break; } @@ -336,11 +342,22 @@ class ManiphestAuxiliaryFieldDefaultSpecification } break; case self::TYPE_DATE: - $new_display = phabricator_datetime($new, $this->getUser()); + // NOTE: Although it should be impossible to get bad data in these + // fields normally, users can change the type of an existing field and + // leave us with uninterpretable data in old transactions. + if ((int)$new <= 0) { + $new_display = "(invalid epoch timestamp: {$new})"; + } else { + $new_display = phabricator_datetime($new, $this->getUser()); + } if ($old === null) { $desc = "set field '{$label}' to '{$new_display}'"; } else { - $old_display = phabricator_datetime($old, $this->getUser()); + if ((int)$old <= 0) { + $old_display = "(invalid epoch timestamp: {$old})"; + } else { + $old_display = phabricator_datetime($old, $this->getUser()); + } $desc = "changed field '{$label}' ". "from '{$old_display}' to '{$new_display}'"; }