Differential D25239 Diff 1198 src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldPHIDs.php
Changeset View
Changeset View
Standalone View
Standalone View
src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldPHIDs.php
| Show All 34 Lines | abstract class PhabricatorStandardCustomFieldPHIDs | ||||||||||
| public function setValueFromStorage($value) { | public function setValueFromStorage($value) { | ||||||||||
| // NOTE: We're accepting either a JSON string (a real storage value) or | // NOTE: We're accepting either a JSON string (a real storage value) or | ||||||||||
| // an array (from HTTP parameter prefilling). This is a little hacky, but | // an array (from HTTP parameter prefilling). This is a little hacky, but | ||||||||||
| // should hold until this can get cleaned up more thoroughly. | // should hold until this can get cleaned up more thoroughly. | ||||||||||
| // TODO: Clean this up. | // TODO: Clean this up. | ||||||||||
| $result = array(); | $result = array(); | ||||||||||
| if ($value === null) { | |||||||||||
| return $this; | |||||||||||
| } | |||||||||||
| if (!is_array($value)) { | if (!is_array($value)) { | ||||||||||
| $value = json_decode($value, true); | $value = json_decode($value, true); | ||||||||||
valerio.bozzolan: The goal here is to decode a string. So, checking for "not array" is clearly not a good way to… | |||||||||||
| if (is_array($value)) { | if (is_array($value)) { | ||||||||||
| $result = array_values($value); | $result = array_values($value); | ||||||||||
valerio.bozzolanUnsubmitted Not Done Inline Actions🔴 Something seems terribly broken in the legacy code. The $result is never used. valerio.bozzolan: 🔴 Something seems terribly broken in the legacy code. The `$result` is never used. | |||||||||||
valerio.bozzolanUnsubmitted Not Done Inline Actions
valerio.bozzolan: | |||||||||||
| } | } | ||||||||||
| } | } | ||||||||||
| $this->setFieldValue($value); | $this->setFieldValue($value); | ||||||||||
| return $this; | return $this; | ||||||||||
| } | } | ||||||||||
| ▲ Show 20 Lines • Show All 221 Lines • Show Last 20 Lines | |||||||||||
Content licensed under Creative Commons Attribution-ShareAlike 4.0 (CC-BY-SA) unless otherwise noted; code licensed under Apache 2.0 or other open source licenses. · CC BY-SA 4.0 · Apache 2.0
The goal here is to decode a string. So, checking for "not array" is clearly not a good way to allow-list strings.
Interestingly, adopting is_string() also should fix the PHP 8.1 problem since NULL is not a string :D so it's never passed as json_decode(null).