From f6214f060e780ecf7b565c5a0edbd28d85c03275 Mon Sep 17 00:00:00 2001 From: Christopher Speck Date: Sun, 7 May 2023 18:37:34 -0400 Subject: [PATCH] Addressing some PHP 8 incompatibilities Summary: Starting with a new instance running PHP 8.2, address all exceptions that come up through some basic browsing/usage. For `strlen(null)` issues I generally tried to resolve if the value should be non-null at the point of issue, and attempt to address at earlier call-site. There were not many of these that I could determine. In the rest of those cases I would replace with a null-and-strlen check, or use `phutil_nonempty_string` if I was certain the value was a string and it was more convenient. Hitting all code-paths is challenging, so I would search for `strlen` within radius of files I was modifying and evaluate to address those uses in the same manner. Notes: - `AphrontRequest::getStr` only ever returns a string, and is safe to use `phutil_nonempty_string`. - `PhabricatorEnv::getEnvConfig` can return non-string things so any values coming from there should never use `phutil_nonempty_string`. - `AphrontRequest::getHTTPHeader` indicates it could return wild so `phutil_nonempty_string` should not be used. - `AphrontRequest::getURIData` isn't clear if it could return non-string data, so never use `phutil_nonempty_string`. Refs T13588 Test Plan: I'm running an instance on 8.2 and went through the basic setup/installation, startup and usage, including setup issues and configurations/settings. Reviewers: #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: aklapper, Korvin, epriestley Maniphest Tasks: T13588 Differential Revision: https://secure.phabricator.com/D21862 --- .gitignore | 2 ++ src/aphront/AphrontRequest.php | 11 +++++------ src/aphront/response/AphrontFileResponse.php | 7 ++++--- .../response/AphrontWebpageResponse.php | 2 +- .../auth/constants/PhabricatorCookies.php | 2 +- .../controller/PhabricatorAuthController.php | 2 +- .../PhabricatorAuthRegisterController.php | 12 ++++++------ .../PhabricatorAuthStartController.php | 8 ++++---- .../PhabricatorAuthPasswordException.php | 2 +- .../base/controller/PhabricatorController.php | 2 +- .../controller/CelerityResourceController.php | 2 +- .../check/PhabricatorBaseURISetupCheck.php | 2 +- .../check/PhabricatorDaemonsSetupCheck.php | 2 +- .../PhabricatorPHPPreflightSetupCheck.php | 2 +- .../config/check/PhabricatorSetupCheck.php | 2 +- .../check/PhabricatorStorageSetupCheck.php | 8 ++++---- .../check/PhabricatorWebServerSetupCheck.php | 2 +- .../PhabricatorConfigConsoleController.php | 4 ++-- ...abricatorConfigDatabaseStatusController.php | 2 +- ...PhabricatorConfigSettingsListController.php | 2 +- .../controller/DarkConsoleController.php | 4 ++-- .../console/core/DarkConsoleCore.php | 3 +++ .../DiffusionHistoryQueryConduitAPIMethod.php | 2 +- .../controller/DiffusionController.php | 6 +++--- ...habricatorFilesComposeAvatarBuiltinFile.php | 10 +++++----- .../conduit/FileAllocateConduitAPIMethod.php | 2 +- .../PhabricatorFileDataController.php | 4 ++-- .../PhabricatorFileLightboxController.php | 2 +- .../PhabricatorFileViewController.php | 4 ++-- .../PhabricatorJupyterDocumentEngine.php | 2 +- .../PhabricatorDocumentRenderingEngine.php | 8 ++++---- .../engine/PhabricatorS3FileStorageEngine.php | 12 ++++++------ .../PhabricatorEmbedFileRemarkupRule.php | 5 ++++- .../markup/PhabricatorImageRemarkupRule.php | 7 ++++--- .../files/storage/PhabricatorFile.php | 10 +++++----- .../view/PhabricatorGlobalUploadTargetView.php | 2 +- .../herald/phid/HeraldTranscriptPHIDType.php | 2 +- .../meta/query/PhabricatorAppSearchEngine.php | 2 +- .../PhabricatorNotificationServerRef.php | 18 +++++++++++++++--- ...habricatorNotificationServersConfigType.php | 2 +- .../PhabricatorUserProfileImageCacheType.php | 4 ++++ .../people/view/PhabricatorUserLogView.php | 7 ++++++- .../compiler/PhutilSearchQueryCompiler.php | 3 +++ .../PhabricatorApplicationSearchController.php | 8 ++++---- .../PhabricatorApplicationSearchEngine.php | 12 ++++++------ .../engine/PhabricatorProfileMenuEngine.php | 4 ++-- .../engine/PhabricatorProfileMenuItemView.php | 8 ++++---- .../field/PhabricatorSearchDateField.php | 4 ++-- .../editengine/PhabricatorEditEngine.php | 12 ++++++------ .../PhabricatorEditEngineSubtype.php | 3 +++ .../editfield/PhabricatorEditField.php | 2 +- .../editfield/PhabricatorTextEditField.php | 2 +- ...PhabricatorApplicationTransactionEditor.php | 2 +- ...torTypeaheadModularDatasourceController.php | 2 +- ...PhabricatorTypeaheadCompositeDatasource.php | 6 +++++- .../PhabricatorTypeaheadDatasource.php | 5 ++++- .../cluster/PhabricatorDatabaseRef.php | 2 +- .../field/PhabricatorCustomFieldList.php | 2 +- src/infrastructure/env/PhabricatorEnv.php | 2 +- src/infrastructure/javelin/markup.php | 5 ++++- src/view/control/AphrontTableView.php | 2 +- src/view/form/control/AphrontFormControl.php | 15 +++++++++------ .../control/AphrontFormTokenizerControl.php | 4 ++-- src/view/layout/AphrontSideNavFilterView.php | 5 ++--- src/view/page/PhabricatorStandardPageView.php | 2 +- src/view/page/menu/PhabricatorMainMenuView.php | 2 +- src/view/phui/PHUIInfoView.php | 6 +++--- src/view/phui/PHUIObjectItemListView.php | 4 ++-- src/view/phui/PHUIObjectItemView.php | 18 +++++++++++------- src/view/phui/PHUITagView.php | 6 +++++- src/view/widget/AphrontStackTraceView.php | 2 +- 71 files changed, 201 insertions(+), 145 deletions(-) diff --git a/.gitignore b/.gitignore index d9f44b6bab..b7f6818b8f 100644 --- a/.gitignore +++ b/.gitignore @@ -36,6 +36,8 @@ # NPM local packages /support/aphlict/server/node_modules/ +/support/aphlict/server/package.json +/support/aphlict/server/package-lock.json # Places for users to add custom resources. /resources/cows/custom/* diff --git a/src/aphront/AphrontRequest.php b/src/aphront/AphrontRequest.php index f35f3f1279..56d9e85cc5 100644 --- a/src/aphront/AphrontRequest.php +++ b/src/aphront/AphrontRequest.php @@ -66,7 +66,7 @@ final class AphrontRequest extends Phobject { } public static function parseURILineRange($range, $limit) { - if (!strlen($range)) { + if ($range === null || !strlen($range)) { return null; } @@ -448,11 +448,10 @@ final class AphrontRequest extends Phobject { } private function getPrefixedCookieName($name) { - if (strlen($this->cookiePrefix)) { + if ($this->cookiePrefix !== null && strlen($this->cookiePrefix)) { return $this->cookiePrefix.'_'.$name; - } else { - return $name; } + return $name; } public function getCookie($name, $default = null) { @@ -499,7 +498,7 @@ final class AphrontRequest extends Phobject { // domain is. This makes setup easier, and we'll tell administrators to // configure a base domain during the setup process. $base_uri = PhabricatorEnv::getEnvConfig('phabricator.base-uri'); - if (!strlen($base_uri)) { + if ($base_uri === null || !strlen($base_uri)) { return new PhutilURI('http://'.$host.'/'); } @@ -956,7 +955,7 @@ final class AphrontRequest extends Phobject { $submit_cookie = PhabricatorCookies::COOKIE_SUBMIT; $submit_key = $this->getCookie($submit_cookie); - if (strlen($submit_key)) { + if ($submit_key !== null && strlen($submit_key)) { $this->clearCookie($submit_cookie); $this->submitKey = $submit_key; } diff --git a/src/aphront/response/AphrontFileResponse.php b/src/aphront/response/AphrontFileResponse.php index 6bae4c808f..e48a30c84c 100644 --- a/src/aphront/response/AphrontFileResponse.php +++ b/src/aphront/response/AphrontFileResponse.php @@ -19,7 +19,7 @@ final class AphrontFileResponse extends AphrontResponse { } public function setDownload($download) { - if (!strlen($download)) { + if ($download === null || !strlen($download)) { $download = 'untitled'; } $this->download = $download; @@ -113,7 +113,8 @@ final class AphrontFileResponse extends AphrontResponse { $headers[] = array('Content-Length', $content_len); } - if (strlen($this->getDownload())) { + $download = $this->getDownload(); + if ($download !== null && strlen($download)) { $headers[] = array('X-Download-Options', 'noopen'); $filename = $this->getDownload(); @@ -150,7 +151,7 @@ final class AphrontFileResponse extends AphrontResponse { $begin = (int)$matches[1]; // The "Range" may be "200-299" or "200-", meaning "until end of file". - if (strlen($matches[2])) { + if ($matches[2] !== null && strlen($matches[2])) { $range_end = (int)$matches[2]; $end = $range_end + 1; } else { diff --git a/src/aphront/response/AphrontWebpageResponse.php b/src/aphront/response/AphrontWebpageResponse.php index de642f9b19..cfdfc249e1 100644 --- a/src/aphront/response/AphrontWebpageResponse.php +++ b/src/aphront/response/AphrontWebpageResponse.php @@ -21,7 +21,7 @@ final class AphrontWebpageResponse extends AphrontHTMLResponse { public function buildResponseString() { $unexpected_output = $this->getUnexpectedOutput(); - if (strlen($unexpected_output)) { + if ($unexpected_output !== null && strlen($unexpected_output)) { $style = array( 'background: linear-gradient(180deg, #eeddff, #ddbbff);', 'white-space: pre-wrap;', diff --git a/src/applications/auth/constants/PhabricatorCookies.php b/src/applications/auth/constants/PhabricatorCookies.php index 2573bf3ec6..31bdf3704c 100644 --- a/src/applications/auth/constants/PhabricatorCookies.php +++ b/src/applications/auth/constants/PhabricatorCookies.php @@ -89,7 +89,7 @@ final class PhabricatorCookies extends Phobject { // temporary and clearing it when users log out. $value = $request->getCookie(self::COOKIE_CLIENTID); - if (!strlen($value)) { + if ($value === null || !strlen($value)) { $request->setTemporaryCookie( self::COOKIE_CLIENTID, Filesystem::readRandomCharacters(16)); diff --git a/src/applications/auth/controller/PhabricatorAuthController.php b/src/applications/auth/controller/PhabricatorAuthController.php index aee93b98d6..5eee9d580f 100644 --- a/src/applications/auth/controller/PhabricatorAuthController.php +++ b/src/applications/auth/controller/PhabricatorAuthController.php @@ -282,7 +282,7 @@ abstract class PhabricatorAuthController extends PhabricatorController { $viewer, PhabricatorAuthLoginMessageType::MESSAGEKEY); - if (!strlen($text)) { + if ($text === null || !strlen($text)) { return null; } diff --git a/src/applications/auth/controller/PhabricatorAuthRegisterController.php b/src/applications/auth/controller/PhabricatorAuthRegisterController.php index 9fbf30d092..05405b42c7 100644 --- a/src/applications/auth/controller/PhabricatorAuthRegisterController.php +++ b/src/applications/auth/controller/PhabricatorAuthRegisterController.php @@ -18,7 +18,7 @@ final class PhabricatorAuthRegisterController $invite = $this->loadInvite(); $is_setup = false; - if (strlen($account_key)) { + if ($account_key !== null && strlen($account_key)) { $result = $this->loadAccountForRegistrationOrLinking($account_key); list($account, $provider, $response) = $result; $is_default = false; @@ -244,9 +244,9 @@ final class PhabricatorAuthRegisterController $require_real_name = PhabricatorEnv::getEnvConfig('user.require-real-name'); - $e_username = strlen($value_username) ? null : true; + $e_username = phutil_nonempty_string($value_username) ? null : true; $e_realname = $require_real_name ? true : null; - $e_email = strlen($value_email) ? null : true; + $e_email = phutil_nonempty_string($value_email) ? null : true; $e_password = true; $e_captcha = true; @@ -288,7 +288,7 @@ final class PhabricatorAuthRegisterController if ($can_edit_username) { $value_username = $request->getStr('username'); - if (!strlen($value_username)) { + if (!phutil_nonempty_string($value_username)) { $e_username = pht('Required'); $errors[] = pht('Username is required.'); } else if (!PhabricatorUser::validateUsername($value_username)) { @@ -323,7 +323,7 @@ final class PhabricatorAuthRegisterController if ($can_edit_email) { $value_email = $request->getStr('email'); - if (!strlen($value_email)) { + if (!phutil_nonempty_string($value_email)) { $e_email = pht('Required'); $errors[] = pht('Email is required.'); } else if (!PhabricatorUserEmail::isValidAddress($value_email)) { @@ -339,7 +339,7 @@ final class PhabricatorAuthRegisterController if ($can_edit_realname) { $value_realname = $request->getStr('realName'); - if (!strlen($value_realname) && $require_real_name) { + if (!phutil_nonempty_string($value_realname) && $require_real_name) { $e_realname = pht('Required'); $errors[] = pht('Real name is required.'); } else { diff --git a/src/applications/auth/controller/PhabricatorAuthStartController.php b/src/applications/auth/controller/PhabricatorAuthStartController.php index 5789740e14..e9beb2b403 100644 --- a/src/applications/auth/controller/PhabricatorAuthStartController.php +++ b/src/applications/auth/controller/PhabricatorAuthStartController.php @@ -31,7 +31,7 @@ final class PhabricatorAuthStartController $session_token = $request->getCookie(PhabricatorCookies::COOKIE_SESSION); $did_clear = $request->getStr('cleared'); - if (strlen($session_token)) { + if ($session_token !== null && strlen($session_token)) { $kind = PhabricatorAuthSessionEngine::getSessionKindFromToken( $session_token); switch ($kind) { @@ -98,7 +98,7 @@ final class PhabricatorAuthStartController } $next_uri = $request->getStr('next'); - if (!strlen($next_uri)) { + if (phutil_nonempty_string($next_uri)) { if ($this->getDelegatingController()) { // Only set a next URI from the request path if this controller was // delegated to, which happens when a user tries to view a page which @@ -112,7 +112,7 @@ final class PhabricatorAuthStartController } if (!$request->isFormPost()) { - if (strlen($next_uri)) { + if (phutil_nonempty_string($next_uri)) { PhabricatorCookies::setNextURICookie($request, $next_uri); } PhabricatorCookies::setClientIDCookie($request); @@ -226,7 +226,7 @@ final class PhabricatorAuthStartController $via_header = AphrontRequest::getViaHeaderName(); $via_uri = AphrontRequest::getHTTPHeader($via_header); - if (strlen($via_uri)) { + if ($via_uri !== null && strlen($via_uri)) { PhabricatorCookies::setNextURICookie($request, $via_uri, $force = true); } diff --git a/src/applications/auth/password/PhabricatorAuthPasswordException.php b/src/applications/auth/password/PhabricatorAuthPasswordException.php index d4b13bb10c..fa749722af 100644 --- a/src/applications/auth/password/PhabricatorAuthPasswordException.php +++ b/src/applications/auth/password/PhabricatorAuthPasswordException.php @@ -4,7 +4,7 @@ final class PhabricatorAuthPasswordException extends Exception { private $passwordError; - private $confirmErorr; + private $confirmError; public function __construct( $message, diff --git a/src/applications/base/controller/PhabricatorController.php b/src/applications/base/controller/PhabricatorController.php index db9d456094..3457e0df46 100644 --- a/src/applications/base/controller/PhabricatorController.php +++ b/src/applications/base/controller/PhabricatorController.php @@ -74,7 +74,7 @@ abstract class PhabricatorController extends AphrontController { $session_engine = new PhabricatorAuthSessionEngine(); $phsid = $request->getCookie(PhabricatorCookies::COOKIE_SESSION); - if (strlen($phsid)) { + if ($phsid !== null && strlen($phsid)) { $session_user = $session_engine->loadUserForSession( PhabricatorAuthSession::TYPE_WEB, $phsid); diff --git a/src/applications/celerity/controller/CelerityResourceController.php b/src/applications/celerity/controller/CelerityResourceController.php index 702fcefb2c..4fc05ede3a 100644 --- a/src/applications/celerity/controller/CelerityResourceController.php +++ b/src/applications/celerity/controller/CelerityResourceController.php @@ -113,7 +113,7 @@ abstract class CelerityResourceController extends PhabricatorController { $range = AphrontRequest::getHTTPHeader('Range'); - if (strlen($range)) { + if ($range !== null && strlen($range)) { $response->setContentLength(strlen($data)); list($range_begin, $range_end) = $response->parseHTTPRange($range); diff --git a/src/applications/config/check/PhabricatorBaseURISetupCheck.php b/src/applications/config/check/PhabricatorBaseURISetupCheck.php index 92e46641d7..65176318e2 100644 --- a/src/applications/config/check/PhabricatorBaseURISetupCheck.php +++ b/src/applications/config/check/PhabricatorBaseURISetupCheck.php @@ -11,7 +11,7 @@ final class PhabricatorBaseURISetupCheck extends PhabricatorSetupCheck { $host_header = AphrontRequest::getHTTPHeader('Host'); if (strpos($host_header, '.') === false) { - if (!strlen(trim($host_header))) { + if ($host_header === null || !strlen(trim($host_header))) { $name = pht('No "Host" Header'); $summary = pht('No "Host" header present in request.'); $message = pht( diff --git a/src/applications/config/check/PhabricatorDaemonsSetupCheck.php b/src/applications/config/check/PhabricatorDaemonsSetupCheck.php index 608bac675e..1d52137d82 100644 --- a/src/applications/config/check/PhabricatorDaemonsSetupCheck.php +++ b/src/applications/config/check/PhabricatorDaemonsSetupCheck.php @@ -53,7 +53,7 @@ final class PhabricatorDaemonsSetupCheck extends PhabricatorSetupCheck { } $expect_user = PhabricatorEnv::getEnvConfig('phd.user'); - if (strlen($expect_user)) { + if ($expect_user !== null && strlen($expect_user)) { try { $all_daemons = id(new PhabricatorDaemonLogQuery()) diff --git a/src/applications/config/check/PhabricatorPHPPreflightSetupCheck.php b/src/applications/config/check/PhabricatorPHPPreflightSetupCheck.php index 02c1134dc3..763f9d3b7a 100644 --- a/src/applications/config/check/PhabricatorPHPPreflightSetupCheck.php +++ b/src/applications/config/check/PhabricatorPHPPreflightSetupCheck.php @@ -124,7 +124,7 @@ final class PhabricatorPHPPreflightSetupCheck extends PhabricatorSetupCheck { } $open_basedir = ini_get('open_basedir'); - if (strlen($open_basedir)) { + if ($open_basedir !== null && strlen($open_basedir)) { // If `open_basedir` is set, just fatal. It's technically possible for // us to run with certain values of `open_basedir`, but: we can only // raise fatal errors from preflight steps, so we'd have to do this check diff --git a/src/applications/config/check/PhabricatorSetupCheck.php b/src/applications/config/check/PhabricatorSetupCheck.php index e0574b7636..6707416424 100644 --- a/src/applications/config/check/PhabricatorSetupCheck.php +++ b/src/applications/config/check/PhabricatorSetupCheck.php @@ -122,7 +122,7 @@ abstract class PhabricatorSetupCheck extends Phobject { $db_cache = new PhabricatorKeyValueDatabaseCache(); try { $value = $db_cache->getKey('phabricator.setup.issue-keys'); - if (!strlen($value)) { + if ($value === null || !strlen($value)) { return null; } return phutil_json_decode($value); diff --git a/src/applications/config/check/PhabricatorStorageSetupCheck.php b/src/applications/config/check/PhabricatorStorageSetupCheck.php index 86aa1f2ebd..3a8aea08ce 100644 --- a/src/applications/config/check/PhabricatorStorageSetupCheck.php +++ b/src/applications/config/check/PhabricatorStorageSetupCheck.php @@ -151,19 +151,19 @@ final class PhabricatorStorageSetupCheck extends PhabricatorSetupCheck { $how_many = 0; - if (strlen($access_key)) { + if ($access_key !== null && strlen($access_key)) { $how_many++; } - if (strlen($secret_key)) { + if ($secret_key !== null && strlen($secret_key)) { $how_many++; } - if (strlen($region)) { + if ($region !== null && strlen($region)) { $how_many++; } - if (strlen($endpoint)) { + if ($endpoint !== null && strlen($endpoint)) { $how_many++; } diff --git a/src/applications/config/check/PhabricatorWebServerSetupCheck.php b/src/applications/config/check/PhabricatorWebServerSetupCheck.php index cc9660325c..61cbb2085c 100644 --- a/src/applications/config/check/PhabricatorWebServerSetupCheck.php +++ b/src/applications/config/check/PhabricatorWebServerSetupCheck.php @@ -23,7 +23,7 @@ final class PhabricatorWebServerSetupCheck extends PhabricatorSetupCheck { } $base_uri = PhabricatorEnv::getEnvConfig('phabricator.base-uri'); - if (!strlen($base_uri)) { + if ($base_uri === null || !strlen($base_uri)) { // If `phabricator.base-uri` is not set then we can't really do // anything. return; diff --git a/src/applications/config/controller/PhabricatorConfigConsoleController.php b/src/applications/config/controller/PhabricatorConfigConsoleController.php index bd23d6dde5..169906fa74 100644 --- a/src/applications/config/controller/PhabricatorConfigConsoleController.php +++ b/src/applications/config/controller/PhabricatorConfigConsoleController.php @@ -85,14 +85,14 @@ final class PhabricatorConfigConsoleController $rows = array(); foreach ($versions as $name => $info) { $branchpoint = $info['branchpoint']; - if (strlen($branchpoint)) { + if ($branchpoint !== null && strlen($branchpoint)) { $branchpoint = substr($branchpoint, 0, 12); } else { $branchpoint = null; } $version = $info['hash']; - if (strlen($version)) { + if ($version !== null && strlen($version)) { $version = substr($version, 0, 12); } else { $version = pht('Unknown'); diff --git a/src/applications/config/controller/services/PhabricatorConfigDatabaseStatusController.php b/src/applications/config/controller/services/PhabricatorConfigDatabaseStatusController.php index 09f4f344b5..652b875954 100644 --- a/src/applications/config/controller/services/PhabricatorConfigDatabaseStatusController.php +++ b/src/applications/config/controller/services/PhabricatorConfigDatabaseStatusController.php @@ -837,7 +837,7 @@ final class PhabricatorConfigDatabaseStatusController $parts = array(); foreach ($properties as $key => $property) { - if (!strlen($property)) { + if ($property === null || !strlen($property)) { continue; } diff --git a/src/applications/config/controller/settings/PhabricatorConfigSettingsListController.php b/src/applications/config/controller/settings/PhabricatorConfigSettingsListController.php index 8513150134..f793623ccd 100644 --- a/src/applications/config/controller/settings/PhabricatorConfigSettingsListController.php +++ b/src/applications/config/controller/settings/PhabricatorConfigSettingsListController.php @@ -7,7 +7,7 @@ final class PhabricatorConfigSettingsListController $viewer = $request->getViewer(); $filter = $request->getURIData('filter'); - if (!strlen($filter)) { + if ($filter === null || !strlen($filter)) { $filter = 'settings'; } diff --git a/src/applications/console/controller/DarkConsoleController.php b/src/applications/console/controller/DarkConsoleController.php index 3a677920ee..9f1289653e 100644 --- a/src/applications/console/controller/DarkConsoleController.php +++ b/src/applications/console/controller/DarkConsoleController.php @@ -26,7 +26,7 @@ final class DarkConsoleController extends PhabricatorController { } $visible = $request->getStr('visible'); - if (strlen($visible)) { + if (phutil_nonempty_string($visible)) { $this->writeDarkConsoleSetting( PhabricatorDarkConsoleVisibleSetting::SETTINGKEY, (int)$visible); @@ -34,7 +34,7 @@ final class DarkConsoleController extends PhabricatorController { } $tab = $request->getStr('tab'); - if (strlen($tab)) { + if (phutil_nonempty_string($tab)) { $this->writeDarkConsoleSetting( PhabricatorDarkConsoleTabSetting::SETTINGKEY, $tab); diff --git a/src/applications/console/core/DarkConsoleCore.php b/src/applications/console/core/DarkConsoleCore.php index 437d6f0be7..71d7febd19 100644 --- a/src/applications/console/core/DarkConsoleCore.php +++ b/src/applications/console/core/DarkConsoleCore.php @@ -114,6 +114,9 @@ final class DarkConsoleCore extends Phobject { * need to convert it to UTF-8. */ private function sanitizeForJSON($data) { + if ($data === null) { + return ''; + } if (is_object($data)) { return ''; } else if (is_array($data)) { diff --git a/src/applications/diffusion/conduit/DiffusionHistoryQueryConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionHistoryQueryConduitAPIMethod.php index c6ae6dbd2f..5eb20d2f7e 100644 --- a/src/applications/diffusion/conduit/DiffusionHistoryQueryConduitAPIMethod.php +++ b/src/applications/diffusion/conduit/DiffusionHistoryQueryConduitAPIMethod.php @@ -55,7 +55,7 @@ final class DiffusionHistoryQueryConduitAPIMethod $limit = $request->getValue('limit'); if (strlen($against_hash)) { - $commit_range = "${against_hash}..${commit_hash}"; + $commit_range = "{$against_hash}..{$commit_hash}"; } else { $commit_range = $commit_hash; } diff --git a/src/applications/diffusion/controller/DiffusionController.php b/src/applications/diffusion/controller/DiffusionController.php index b3595c1b72..e7d1f43956 100644 --- a/src/applications/diffusion/controller/DiffusionController.php +++ b/src/applications/diffusion/controller/DiffusionController.php @@ -97,19 +97,19 @@ abstract class DiffusionController extends PhabricatorController { AphrontRequest $request) { $short_name = $request->getURIData('repositoryShortName'); - if (strlen($short_name)) { + if ($short_name !== null && strlen($short_name)) { // If the short name ends in ".git", ignore it. $short_name = preg_replace('/\\.git\z/', '', $short_name); return $short_name; } $identifier = $request->getURIData('repositoryCallsign'); - if (strlen($identifier)) { + if ($identifier !== null && strlen($identifier)) { return $identifier; } $id = $request->getURIData('repositoryID'); - if (strlen($id)) { + if ($id !== null && strlen($id)) { return (int)$id; } diff --git a/src/applications/files/builtin/PhabricatorFilesComposeAvatarBuiltinFile.php b/src/applications/files/builtin/PhabricatorFilesComposeAvatarBuiltinFile.php index 4f827f7603..acebfcdfef 100644 --- a/src/applications/files/builtin/PhabricatorFilesComposeAvatarBuiltinFile.php +++ b/src/applications/files/builtin/PhabricatorFilesComposeAvatarBuiltinFile.php @@ -183,11 +183,11 @@ final class PhabricatorFilesComposeAvatarBuiltinFile 'image/png'); } - private static function rgba2gd($rgba) { - $r = $rgba[0]; - $g = $rgba[1]; - $b = $rgba[2]; - $a = $rgba[3]; + private static function rgba2gd(array $rgba) { + $r = (int)$rgba[0]; + $g = (int)$rgba[1]; + $b = (int)$rgba[2]; + $a = (int)$rgba[3]; $a = (1 - $a) * 255; return ($a << 24) | ($r << 16) | ($g << 8) | $b; } diff --git a/src/applications/files/conduit/FileAllocateConduitAPIMethod.php b/src/applications/files/conduit/FileAllocateConduitAPIMethod.php index c831eb2afe..1da759c907 100644 --- a/src/applications/files/conduit/FileAllocateConduitAPIMethod.php +++ b/src/applications/files/conduit/FileAllocateConduitAPIMethod.php @@ -65,7 +65,7 @@ final class FileAllocateConduitAPIMethod ->executeOne(); } - if (strlen($name) && !$hash && !$file) { + if ($name !== null && strlen($name) && !$hash && !$file) { if ($length > PhabricatorFileStorageEngine::getChunkThreshold()) { // If we don't have a hash, but this file is large enough to store in // chunks and thus may be resumable, try to find a partially uploaded diff --git a/src/applications/files/controller/PhabricatorFileDataController.php b/src/applications/files/controller/PhabricatorFileDataController.php index 8189b30a21..5920053f8f 100644 --- a/src/applications/files/controller/PhabricatorFileDataController.php +++ b/src/applications/files/controller/PhabricatorFileDataController.php @@ -29,7 +29,7 @@ final class PhabricatorFileDataController extends PhabricatorFileController { $request_kind = $request->getURIData('kind'); $is_download = ($request_kind === 'download'); - if (!strlen($alt) || $main_domain == $alt_domain) { + if (($alt === null || !strlen($alt)) || $main_domain == $alt_domain) { // No alternate domain. $should_redirect = false; $is_alternate_domain = false; @@ -69,7 +69,7 @@ final class PhabricatorFileDataController extends PhabricatorFileController { // an initial request for bytes 0-1 of the audio file, and things go south // if we can't respond with a 206 Partial Content. $range = $request->getHTTPHeader('range'); - if (strlen($range)) { + if ($range !== null && strlen($range)) { list($begin, $end) = $response->parseHTTPRange($range); } diff --git a/src/applications/files/controller/PhabricatorFileLightboxController.php b/src/applications/files/controller/PhabricatorFileLightboxController.php index 59a826dd42..078b9b3b12 100644 --- a/src/applications/files/controller/PhabricatorFileLightboxController.php +++ b/src/applications/files/controller/PhabricatorFileLightboxController.php @@ -20,7 +20,7 @@ final class PhabricatorFileLightboxController return new Aphront404Response(); } - if (strlen($comment)) { + if ($comment !== null && strlen($comment)) { $xactions = array(); $xactions[] = id(new PhabricatorFileTransaction()) ->setTransactionType(PhabricatorTransactions::TYPE_COMMENT) diff --git a/src/applications/files/controller/PhabricatorFileViewController.php b/src/applications/files/controller/PhabricatorFileViewController.php index 389fc66247..5c429d57d9 100644 --- a/src/applications/files/controller/PhabricatorFileViewController.php +++ b/src/applications/files/controller/PhabricatorFileViewController.php @@ -311,12 +311,12 @@ final class PhabricatorFileViewController extends PhabricatorFileController { $file->getStorageHandle()); $custom_alt = $file->getCustomAltText(); - if (strlen($custom_alt)) { + if ($custom_alt !== null && strlen($custom_alt)) { $finfo->addProperty(pht('Custom Alt Text'), $custom_alt); } $default_alt = $file->getDefaultAltText(); - if (strlen($default_alt)) { + if ($default_alt !== null && strlen($default_alt)) { $finfo->addProperty(pht('Default Alt Text'), $default_alt); } diff --git a/src/applications/files/document/PhabricatorJupyterDocumentEngine.php b/src/applications/files/document/PhabricatorJupyterDocumentEngine.php index 20474b047b..28816c36a6 100644 --- a/src/applications/files/document/PhabricatorJupyterDocumentEngine.php +++ b/src/applications/files/document/PhabricatorJupyterDocumentEngine.php @@ -323,7 +323,7 @@ final class PhabricatorJupyterDocumentEngine } $nbformat = idx($data, 'nbformat'); - if (!strlen($nbformat)) { + if ($nbformat == null || !strlen($nbformat)) { throw new Exception( pht( 'This document is missing an "nbformat" field. Jupyter notebooks '. diff --git a/src/applications/files/document/render/PhabricatorDocumentRenderingEngine.php b/src/applications/files/document/render/PhabricatorDocumentRenderingEngine.php index 5a4cb77340..c664e56b6d 100644 --- a/src/applications/files/document/render/PhabricatorDocumentRenderingEngine.php +++ b/src/applications/files/document/render/PhabricatorDocumentRenderingEngine.php @@ -60,12 +60,12 @@ abstract class PhabricatorDocumentRenderingEngine } $encode_setting = $request->getStr('encode'); - if (strlen($encode_setting)) { + if (phutil_nonempty_string($encode_setting)) { $engine->setEncodingConfiguration($encode_setting); } $highlight_setting = $request->getStr('highlight'); - if (strlen($highlight_setting)) { + if (phutil_nonempty_string($highlight_setting)) { $engine->setHighlightingConfiguration($highlight_setting); } @@ -208,12 +208,12 @@ abstract class PhabricatorDocumentRenderingEngine $this->activeEngine = $engine; $encode_setting = $request->getStr('encode'); - if (strlen($encode_setting)) { + if (phutil_nonempty_string($encode_setting)) { $engine->setEncodingConfiguration($encode_setting); } $highlight_setting = $request->getStr('highlight'); - if (strlen($highlight_setting)) { + if (phutil_nonempty_string($highlight_setting)) { $engine->setHighlightingConfiguration($highlight_setting); } diff --git a/src/applications/files/engine/PhabricatorS3FileStorageEngine.php b/src/applications/files/engine/PhabricatorS3FileStorageEngine.php index 443a5a9ddc..d78cf352e0 100644 --- a/src/applications/files/engine/PhabricatorS3FileStorageEngine.php +++ b/src/applications/files/engine/PhabricatorS3FileStorageEngine.php @@ -31,11 +31,11 @@ final class PhabricatorS3FileStorageEngine $endpoint = PhabricatorEnv::getEnvConfig('amazon-s3.endpoint'); $region = PhabricatorEnv::getEnvConfig('amazon-s3.region'); - return (strlen($bucket) && - strlen($access_key) && - strlen($secret_key) && - strlen($endpoint) && - strlen($region)); + return ($bucket !== null && strlen($bucket) && + $access_key !== null && strlen($access_key) && + $secret_key !== null && strlen($secret_key) && + $endpoint !== null && strlen($endpoint) && + $region !== null && strlen($region)); } @@ -57,7 +57,7 @@ final class PhabricatorS3FileStorageEngine $parts[] = 'phabricator'; $instance_name = PhabricatorEnv::getEnvConfig('cluster.instance'); - if (strlen($instance_name)) { + if ($instance_name !== null && strlen($instance_name)) { $parts[] = $instance_name; } diff --git a/src/applications/files/markup/PhabricatorEmbedFileRemarkupRule.php b/src/applications/files/markup/PhabricatorEmbedFileRemarkupRule.php index 9f37bdcaab..e1985d65de 100644 --- a/src/applications/files/markup/PhabricatorEmbedFileRemarkupRule.php +++ b/src/applications/files/markup/PhabricatorEmbedFileRemarkupRule.php @@ -197,7 +197,7 @@ final class PhabricatorEmbedFileRemarkupRule $alt = $options['alt']; } - if (!strlen($alt)) { + if ($alt === null || !strlen($alt)) { $alt = $file->getAltText(); } @@ -346,6 +346,9 @@ final class PhabricatorEmbedFileRemarkupRule } private function parseDimension($string) { + if ($string === null || !strlen($string)) { + return null; + } $string = trim($string); if (preg_match('/^(?:\d*\\.)?\d+%?$/', $string)) { diff --git a/src/applications/files/markup/PhabricatorImageRemarkupRule.php b/src/applications/files/markup/PhabricatorImageRemarkupRule.php index 57ad75bbc5..b06bd8671d 100644 --- a/src/applications/files/markup/PhabricatorImageRemarkupRule.php +++ b/src/applications/files/markup/PhabricatorImageRemarkupRule.php @@ -49,7 +49,8 @@ final class PhabricatorImageRemarkupRule extends PhutilRemarkupRule { $args += $defaults; - if (!strlen($args['uri'])) { + $uri_arg = $args['uri']; + if ($uri_arg === null || !strlen($uri_arg)) { return $matches[0]; } @@ -57,9 +58,9 @@ final class PhabricatorImageRemarkupRule extends PhutilRemarkupRule { // validate it more carefully before proxying it, but if whatever the user // has typed isn't even close, just decline to activate the rule behavior. try { - $uri = new PhutilURI($args['uri']); + $uri = new PhutilURI($uri_arg); - if (!strlen($uri->getProtocol())) { + if ($uri->getProtocol() === null || !strlen($uri->getProtocol())) { return $matches[0]; } diff --git a/src/applications/files/storage/PhabricatorFile.php b/src/applications/files/storage/PhabricatorFile.php index cc1b701dcd..32029886f0 100644 --- a/src/applications/files/storage/PhabricatorFile.php +++ b/src/applications/files/storage/PhabricatorFile.php @@ -288,7 +288,7 @@ final class PhabricatorFile extends PhabricatorFileDAO // update the parent file if a MIME type hasn't been provided. This matters // for large media files like video. $mime_type = idx($params, 'mime-type'); - if (!strlen($mime_type)) { + if ($mime_type === null || !strlen($mime_type)) { $file->setMimeType('application/octet-stream'); } @@ -856,7 +856,7 @@ final class PhabricatorFile extends PhabricatorFileDAO // instance identity in the path allows us to distinguish between requests // originating from different instances but served through the same CDN. $instance = PhabricatorEnv::getEnvConfig('cluster.instance'); - if (strlen($instance)) { + if ($instance !== null && strlen($instance)) { $parts[] = '@'.$instance; } @@ -903,7 +903,7 @@ final class PhabricatorFile extends PhabricatorFileDAO $parts[] = 'xform'; $instance = PhabricatorEnv::getEnvConfig('cluster.instance'); - if (strlen($instance)) { + if ($instance !== null && strlen($instance)) { $parts[] = '@'.$instance; } @@ -1278,7 +1278,7 @@ final class PhabricatorFile extends PhabricatorFileDAO public function getAltText() { $alt = $this->getCustomAltText(); - if (strlen($alt)) { + if ($alt !== null && strlen($alt)) { return $alt; } @@ -1309,7 +1309,7 @@ final class PhabricatorFile extends PhabricatorFileDAO $parts = array(); $name = $this->getName(); - if (strlen($name)) { + if ($name !== null && strlen($name)) { $parts[] = $name; } diff --git a/src/applications/files/view/PhabricatorGlobalUploadTargetView.php b/src/applications/files/view/PhabricatorGlobalUploadTargetView.php index 76aacbe36f..9fb902f68a 100644 --- a/src/applications/files/view/PhabricatorGlobalUploadTargetView.php +++ b/src/applications/files/view/PhabricatorGlobalUploadTargetView.php @@ -67,7 +67,7 @@ final class PhabricatorGlobalUploadTargetView extends AphrontView { require_celerity_resource('global-drag-and-drop-css'); $hint_text = $this->getHintText(); - if (!strlen($hint_text)) { + if ($hint_text === null || !strlen($hint_text)) { $hint_text = "\xE2\x87\xAA ".pht('Drop Files to Upload'); } diff --git a/src/applications/herald/phid/HeraldTranscriptPHIDType.php b/src/applications/herald/phid/HeraldTranscriptPHIDType.php index 8ba3a0254c..e10c4b02c7 100644 --- a/src/applications/herald/phid/HeraldTranscriptPHIDType.php +++ b/src/applications/herald/phid/HeraldTranscriptPHIDType.php @@ -35,7 +35,7 @@ final class HeraldTranscriptPHIDType extends PhabricatorPHIDType { $id = $xscript->getID(); $handle->setName(pht('Transcript %s', $id)); - $handle->setURI("/herald/transcript/${id}/"); + $handle->setURI("/herald/transcript/{$id}/"); } } diff --git a/src/applications/meta/query/PhabricatorAppSearchEngine.php b/src/applications/meta/query/PhabricatorAppSearchEngine.php index ee938abbbb..103f28dcb6 100644 --- a/src/applications/meta/query/PhabricatorAppSearchEngine.php +++ b/src/applications/meta/query/PhabricatorAppSearchEngine.php @@ -45,7 +45,7 @@ final class PhabricatorAppSearchEngine ->withUnlisted(false); $name = $saved->getParameter('name'); - if (strlen($name)) { + if ($name !== null && strlen($name)) { $query->withNameContains($name); } diff --git a/src/applications/notification/client/PhabricatorNotificationServerRef.php b/src/applications/notification/client/PhabricatorNotificationServerRef.php index 714ad5f5d7..7788bacaaf 100644 --- a/src/applications/notification/client/PhabricatorNotificationServerRef.php +++ b/src/applications/notification/client/PhabricatorNotificationServerRef.php @@ -144,7 +144,19 @@ final class PhabricatorNotificationServerRef } public function getURI($to_path = null) { - $full_path = rtrim($this->getPath(), '/').'/'.ltrim($to_path, '/'); + if ($to_path === null || !strlen($to_path)) { + $to_path = ''; + } else { + $to_path = '/'.ltrim($to_path, '/'); + } + + $base_path = $this->getPath(); + if ($base_path === null || !strlen($base_path)) { + $base_path = ''; + } else { + $base_path = rtrim($base_path, '/'); + } + $full_path = $base_path.$to_path; $uri = id(new PhutilURI('http://'.$this->getHost())) ->setProtocol($this->getProtocol()) @@ -152,7 +164,7 @@ final class PhabricatorNotificationServerRef ->setPath($full_path); $instance = PhabricatorEnv::getEnvConfig('cluster.instance'); - if (strlen($instance)) { + if ($instance !== null && strlen($instance)) { $uri->replaceQueryParam('instance', $instance); } @@ -161,7 +173,7 @@ final class PhabricatorNotificationServerRef public function getWebsocketURI($to_path = null) { $instance = PhabricatorEnv::getEnvConfig('cluster.instance'); - if (strlen($instance)) { + if ($instance !== null && strlen($instance)) { $to_path = $to_path.'~'.$instance.'/'; } diff --git a/src/applications/notification/config/PhabricatorNotificationServersConfigType.php b/src/applications/notification/config/PhabricatorNotificationServersConfigType.php index f13105a249..ee2a73221a 100644 --- a/src/applications/notification/config/PhabricatorNotificationServersConfigType.php +++ b/src/applications/notification/config/PhabricatorNotificationServersConfigType.php @@ -92,7 +92,7 @@ final class PhabricatorNotificationServersConfigType } $path = idx($spec, 'path'); - if ($type == 'admin' && strlen($path)) { + if ($type == 'admin' && $path !== null && strlen($path)) { throw $this->newException( pht( 'Notification server configuration describes an invalid host '. diff --git a/src/applications/people/cache/PhabricatorUserProfileImageCacheType.php b/src/applications/people/cache/PhabricatorUserProfileImageCacheType.php index 8babff859f..9e13a56b01 100644 --- a/src/applications/people/cache/PhabricatorUserProfileImageCacheType.php +++ b/src/applications/people/cache/PhabricatorUserProfileImageCacheType.php @@ -91,6 +91,10 @@ final class PhabricatorUserProfileImageCacheType } public function isRawCacheDataValid(PhabricatorUser $user, $key, $data) { + if ($data === null) { + return false; + } + $parts = explode(',', $data, 2); $version = reset($parts); return ($version === $this->getCacheVersion($user)); diff --git a/src/applications/people/view/PhabricatorUserLogView.php b/src/applications/people/view/PhabricatorUserLogView.php index cf728af973..db9b243a7f 100644 --- a/src/applications/people/view/PhabricatorUserLogView.php +++ b/src/applications/people/view/PhabricatorUserLogView.php @@ -36,7 +36,12 @@ final class PhabricatorUserLogView extends AphrontView { $rows = array(); foreach ($logs as $log) { - $session = substr($log->getSession(), 0, 6); + // Events such as "Login Failure" will not have an associated session. + $session = $log->getSession(); + if ($session === null) { + $session = ''; + } + $session = substr($session, 0, 6); $actor_phid = $log->getActorPHID(); $user_phid = $log->getUserPHID(); diff --git a/src/applications/search/compiler/PhutilSearchQueryCompiler.php b/src/applications/search/compiler/PhutilSearchQueryCompiler.php index c3f93e16c9..79e246c117 100644 --- a/src/applications/search/compiler/PhutilSearchQueryCompiler.php +++ b/src/applications/search/compiler/PhutilSearchQueryCompiler.php @@ -104,6 +104,9 @@ final class PhutilSearchQueryCompiler private function tokenizeQuery($query) { $maximum_bytes = 1024; + if ($query === null) { + $query = ''; + } $query_bytes = strlen($query); if ($query_bytes > $maximum_bytes) { throw new PhutilSearchQueryCompilerSyntaxException( diff --git a/src/applications/search/controller/PhabricatorApplicationSearchController.php b/src/applications/search/controller/PhabricatorApplicationSearchController.php index 290623e149..a06fca7583 100644 --- a/src/applications/search/controller/PhabricatorApplicationSearchController.php +++ b/src/applications/search/controller/PhabricatorApplicationSearchController.php @@ -112,10 +112,10 @@ final class PhabricatorApplicationSearchController $named_query = null; $run_query = true; $query_key = $this->queryKey; - if ($this->queryKey == 'advanced') { + if ($query_key == 'advanced') { $run_query = false; $query_key = $request->getStr('query'); - } else if (!strlen($this->queryKey)) { + } else if ($query_key === null || !strlen($query_key)) { $found_query_data = false; if ($request->isHTTPGet() || $request->isQuicksand()) { @@ -775,7 +775,7 @@ final class PhabricatorApplicationSearchController $force_nux) { // Don't render NUX if the user has clicked away from the default page. - if (strlen($this->getQueryKey())) { + if ($this->getQueryKey() !== null && strlen($this->getQueryKey())) { return null; } @@ -1022,7 +1022,7 @@ final class PhabricatorApplicationSearchController $object_name = pht('%s %s', $object->getMonogram(), $object->getName()); // Likewise, the context object can only be a dashboard. - if (strlen($context_phid)) { + if ($context_phid !== null && !strlen($context_phid)) { $context = id(new PhabricatorDashboardQuery()) ->setViewer($viewer) ->withPHIDs(array($context_phid)) diff --git a/src/applications/search/engine/PhabricatorApplicationSearchEngine.php b/src/applications/search/engine/PhabricatorApplicationSearchEngine.php index 95f1ffafa3..36f90fb57d 100644 --- a/src/applications/search/engine/PhabricatorApplicationSearchEngine.php +++ b/src/applications/search/engine/PhabricatorApplicationSearchEngine.php @@ -179,7 +179,7 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject { $order = $saved->getParameter('order'); $builtin = $query->getBuiltinOrderAliasMap(); - if (strlen($order) && isset($builtin[$order])) { + if ($order !== null && strlen($order) && isset($builtin[$order])) { $query->setOrder($order); } else { // If the order is invalid or not available, we choose the first @@ -872,7 +872,7 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject { protected function readBoolFromRequest( AphrontRequest $request, $key) { - if (!strlen($request->getStr($key))) { + if (!phutil_nonempty_string($request->getStr($key))) { return null; } return $request->getBool($key); @@ -895,7 +895,7 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject { * @task dates */ protected function parseDateTime($date_time) { - if (!strlen($date_time)) { + if ($date_time === null || !strlen($date_time)) { return null; } @@ -916,7 +916,7 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject { $start_str = $saved_query->getParameter($start_key); $start = null; - if (strlen($start_str)) { + if ($start_str !== null && strlen($start_str)) { $start = $this->parseDateTime($start_str); if (!$start) { $this->addError( @@ -929,7 +929,7 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject { $end_str = $saved_query->getParameter($end_key); $end = null; - if (strlen($end_str)) { + if ($end_str !== null && strlen($end_str)) { $end = $this->parseDateTime($end_str); if (!$end) { $this->addError( @@ -1137,7 +1137,7 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject { $viewer = $this->requireViewer(); $query_key = $request->getValue('queryKey'); - if (!strlen($query_key)) { + if ($query_key === null || !strlen($query_key)) { $saved_query = new PhabricatorSavedQuery(); } else if ($this->isBuiltinQuery($query_key)) { $saved_query = $this->buildSavedQueryFromBuiltin($query_key); diff --git a/src/applications/search/engine/PhabricatorProfileMenuEngine.php b/src/applications/search/engine/PhabricatorProfileMenuEngine.php index aedbcc787f..b3c2df191b 100644 --- a/src/applications/search/engine/PhabricatorProfileMenuEngine.php +++ b/src/applications/search/engine/PhabricatorProfileMenuEngine.php @@ -135,7 +135,7 @@ abstract class PhabricatorProfileMenuEngine extends Phobject { if ($is_view) { $selected_item = $this->selectViewItem($view_list, $item_id); } else { - if (!strlen($item_id)) { + if ($item_id === null || !strlen($item_id)) { $item_id = self::ITEM_MANAGE; } $selected_item = $this->selectEditItem($view_list, $item_id); @@ -1308,7 +1308,7 @@ abstract class PhabricatorProfileMenuEngine extends Phobject { // render the default view instead. $selected_view = null; - if (strlen($item_id)) { + if ($item_id !== null && strlen($item_id)) { $item_views = $view_list->getViewsWithItemIdentifier($item_id); if ($item_views) { $selected_view = head($item_views); diff --git a/src/applications/search/engine/PhabricatorProfileMenuItemView.php b/src/applications/search/engine/PhabricatorProfileMenuItemView.php index d947afcba6..1ef41114b5 100644 --- a/src/applications/search/engine/PhabricatorProfileMenuItemView.php +++ b/src/applications/search/engine/PhabricatorProfileMenuItemView.php @@ -110,7 +110,7 @@ final class PhabricatorProfileMenuItemView return $this->isExternalLink; } - public function setIsLabel($is_label) { + public function setIsLabel() { return $this->setSpecialType('label'); } @@ -118,7 +118,7 @@ final class PhabricatorProfileMenuItemView return $this->isSpecialType('label'); } - public function setIsDivider($is_divider) { + public function setIsDivider() { return $this->setSpecialType('divider'); } @@ -140,7 +140,7 @@ final class PhabricatorProfileMenuItemView ->setName($this->getName()); $uri = $this->getURI(); - if (strlen($uri)) { + if ($uri !== null && strlen($uri)) { if ($this->getIsExternalLink()) { if (!PhabricatorEnv::isValidURIForLink($uri)) { $uri = '#'; @@ -176,7 +176,7 @@ final class PhabricatorProfileMenuItemView } $tooltip = $this->getTooltip(); - if (strlen($tooltip)) { + if ($tooltip !== null && strlen($tooltip)) { $view->setTooltip($tooltip); } diff --git a/src/applications/search/field/PhabricatorSearchDateField.php b/src/applications/search/field/PhabricatorSearchDateField.php index 41decd9503..331b616fcf 100644 --- a/src/applications/search/field/PhabricatorSearchDateField.php +++ b/src/applications/search/field/PhabricatorSearchDateField.php @@ -17,7 +17,7 @@ final class PhabricatorSearchDateField } protected function validateControlValue($value) { - if (!strlen($value)) { + if ($value === null || !strlen($value)) { return; } @@ -32,7 +32,7 @@ final class PhabricatorSearchDateField } protected function parseDateTime($value) { - if (!strlen($value)) { + if ($value === null || !strlen($value)) { return null; } diff --git a/src/applications/transactions/editengine/PhabricatorEditEngine.php b/src/applications/transactions/editengine/PhabricatorEditEngine.php index 60ccdaa401..556aa83298 100644 --- a/src/applications/transactions/editengine/PhabricatorEditEngine.php +++ b/src/applications/transactions/editengine/PhabricatorEditEngine.php @@ -941,7 +941,7 @@ abstract class PhabricatorEditEngine } } else { $form_key = $request->getURIData('formKey'); - if (strlen($form_key)) { + if ($form_key !== null && strlen($form_key)) { $config = $this->loadEditEngineConfigurationWithIdentifier($form_key); if (!$config) { @@ -971,14 +971,14 @@ abstract class PhabricatorEditEngine } $page_key = $request->getURIData('pageKey'); - if (!strlen($page_key)) { + if ($page_key === null || !strlen($page_key)) { $pages = $this->getPages($object); if ($pages) { $page_key = head_key($pages); } } - if (strlen($page_key)) { + if ($page_key !== null && strlen($page_key)) { $page = $this->selectPage($object, $page_key); if (!$page) { return new Aphront404Response(); @@ -1169,7 +1169,7 @@ abstract class PhabricatorEditEngine if ($this->getIsCreate()) { $template = $request->getStr('template'); - if (strlen($template)) { + if (phutil_nonempty_string($template)) { $template_object = $this->newObjectFromIdentifier( $template, array( @@ -1909,7 +1909,7 @@ abstract class PhabricatorEditEngine $comment_text = $request->getStr('comment'); $comment_metadata = $request->getStr('comment_metadata'); - if (strlen($comment_metadata)) { + if (phutil_nonempty_string($comment_metadata)) { $comment_metadata = phutil_json_decode($comment_metadata); } @@ -2009,7 +2009,7 @@ abstract class PhabricatorEditEngine $xactions[] = $xaction; } - if (strlen($comment_text) || !$xactions) { + if (($comment_text !== null && strlen($comment_text)) || !$xactions) { $xactions[] = id(clone $template) ->setTransactionType(PhabricatorTransactions::TYPE_COMMENT) ->setMetadataValue('remarkup.control', $comment_metadata) diff --git a/src/applications/transactions/editengine/PhabricatorEditEngineSubtype.php b/src/applications/transactions/editengine/PhabricatorEditEngineSubtype.php index d177595a2b..74108059e5 100644 --- a/src/applications/transactions/editengine/PhabricatorEditEngineSubtype.php +++ b/src/applications/transactions/editengine/PhabricatorEditEngineSubtype.php @@ -89,6 +89,9 @@ final class PhabricatorEditEngineSubtype } public function hasTagView() { + if ($this->getTagText() === null) { + return false; + } return (bool)strlen($this->getTagText()); } diff --git a/src/applications/transactions/editfield/PhabricatorEditField.php b/src/applications/transactions/editfield/PhabricatorEditField.php index 7eafbc60cf..efea3f4615 100644 --- a/src/applications/transactions/editfield/PhabricatorEditField.php +++ b/src/applications/transactions/editfield/PhabricatorEditField.php @@ -418,7 +418,7 @@ abstract class PhabricatorEditField extends Phobject { } $instructions = $this->getControlInstructions(); - if (strlen($instructions)) { + if ($instructions !== null && strlen($instructions)) { $form->appendRemarkupInstructions($instructions); } diff --git a/src/applications/transactions/editfield/PhabricatorTextEditField.php b/src/applications/transactions/editfield/PhabricatorTextEditField.php index 68854cf2e2..354f211588 100644 --- a/src/applications/transactions/editfield/PhabricatorTextEditField.php +++ b/src/applications/transactions/editfield/PhabricatorTextEditField.php @@ -18,7 +18,7 @@ final class PhabricatorTextEditField $control = new AphrontFormTextControl(); $placeholder = $this->getPlaceholder(); - if (strlen($placeholder)) { + if ($placeholder !== null && strlen($placeholder)) { $control->setPlaceholder($placeholder); } diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php index 77b5fbfbb6..17e9a86abb 100644 --- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php +++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php @@ -617,7 +617,7 @@ abstract class PhabricatorApplicationTransactionEditor return true; case PhabricatorTransactions::TYPE_SPACE: $space_phid = $xaction->getNewValue(); - if (!strlen($space_phid)) { + if ($space_phid === null || !strlen($space_phid)) { // If an install has no Spaces or the Spaces controls are not visible // to the viewer, we might end up with the empty string here instead // of a strict `null`, because some controller just used `getStr()` diff --git a/src/applications/typeahead/controller/PhabricatorTypeaheadModularDatasourceController.php b/src/applications/typeahead/controller/PhabricatorTypeaheadModularDatasourceController.php index 2d55c5f663..5753c3cded 100644 --- a/src/applications/typeahead/controller/PhabricatorTypeaheadModularDatasourceController.php +++ b/src/applications/typeahead/controller/PhabricatorTypeaheadModularDatasourceController.php @@ -39,7 +39,7 @@ final class PhabricatorTypeaheadModularDatasourceController $parameters = array(); $raw_parameters = $request->getStr('parameters'); - if (strlen($raw_parameters)) { + if (phutil_nonempty_string($raw_parameters)) { try { $parameters = phutil_json_decode($raw_parameters); } catch (PhutilJSONParserException $ex) { diff --git a/src/applications/typeahead/datasource/PhabricatorTypeaheadCompositeDatasource.php b/src/applications/typeahead/datasource/PhabricatorTypeaheadCompositeDatasource.php index 5594044c42..6b0183e35f 100644 --- a/src/applications/typeahead/datasource/PhabricatorTypeaheadCompositeDatasource.php +++ b/src/applications/typeahead/datasource/PhabricatorTypeaheadCompositeDatasource.php @@ -28,7 +28,8 @@ abstract class PhabricatorTypeaheadCompositeDatasource // We only need to do a prefix phase query if there's an actual query // string. If the user didn't type anything, nothing can possibly match it. - if (strlen($this->getRawQuery())) { + $raw_query = $this->getRawQuery(); + if ($raw_query !== null && strlen($raw_query)) { $phases[] = self::PHASE_PREFIX; } @@ -214,6 +215,9 @@ abstract class PhabricatorTypeaheadCompositeDatasource if (!$limit) { $limit = count($results); } + if (!$offset) { + $offset = 0; + } $results = array_slice($results, $offset, $limit, $preserve_keys = true); } diff --git a/src/applications/typeahead/datasource/PhabricatorTypeaheadDatasource.php b/src/applications/typeahead/datasource/PhabricatorTypeaheadDatasource.php index 29a8d4b7ba..60183776c5 100644 --- a/src/applications/typeahead/datasource/PhabricatorTypeaheadDatasource.php +++ b/src/applications/typeahead/datasource/PhabricatorTypeaheadDatasource.php @@ -147,6 +147,9 @@ abstract class PhabricatorTypeaheadDatasource extends Phobject { } public static function tokenizeString($string) { + if ($string === null) { + return array(); + } $string = phutil_utf8_strtolower($string); $string = trim($string); if (!strlen($string)) { @@ -464,7 +467,7 @@ abstract class PhabricatorTypeaheadDatasource extends Phobject { // We're looking for a "(" so that a string like "members(q" is identified // and parsed as a function call. This allows us to start generating // results immediately, before the user fully types out "members(quack)". - return (strpos($token, '(') !== false); + return ($token !== null && strpos($token, '(') !== false); } diff --git a/src/infrastructure/cluster/PhabricatorDatabaseRef.php b/src/infrastructure/cluster/PhabricatorDatabaseRef.php index 1eb232ad86..106576cc2e 100644 --- a/src/infrastructure/cluster/PhabricatorDatabaseRef.php +++ b/src/infrastructure/cluster/PhabricatorDatabaseRef.php @@ -229,7 +229,7 @@ final class PhabricatorDatabaseRef $host = $this->getHost(); $port = $this->getPort(); - if (strlen($port)) { + if ($port !== null && strlen($port)) { return "{$host}:{$port}"; } diff --git a/src/infrastructure/customfield/field/PhabricatorCustomFieldList.php b/src/infrastructure/customfield/field/PhabricatorCustomFieldList.php index 8ae5529f95..d59ccb1144 100644 --- a/src/infrastructure/customfield/field/PhabricatorCustomFieldList.php +++ b/src/infrastructure/customfield/field/PhabricatorCustomFieldList.php @@ -89,7 +89,7 @@ final class PhabricatorCustomFieldList extends Phobject { $field_handles = array_select_keys($handles, $phids[$field_key]); $instructions = $field->getInstructionsForEdit(); - if (strlen($instructions)) { + if ($instructions !== null && strlen($instructions)) { $form->appendRemarkupInstructions($instructions); } diff --git a/src/infrastructure/env/PhabricatorEnv.php b/src/infrastructure/env/PhabricatorEnv.php index 352e9aeb5e..602ee2ec79 100644 --- a/src/infrastructure/env/PhabricatorEnv.php +++ b/src/infrastructure/env/PhabricatorEnv.php @@ -125,7 +125,7 @@ final class PhabricatorEnv extends Phobject { // If an instance identifier is defined, write it into the environment so // it's available to subprocesses. $instance = self::getEnvConfig('cluster.instance'); - if (strlen($instance)) { + if ($instance !== null && strlen($instance)) { putenv('PHABRICATOR_INSTANCE='.$instance); $_ENV['PHABRICATOR_INSTANCE'] = $instance; } diff --git a/src/infrastructure/javelin/markup.php b/src/infrastructure/javelin/markup.php index 533baad65e..1a0c98d983 100644 --- a/src/infrastructure/javelin/markup.php +++ b/src/infrastructure/javelin/markup.php @@ -77,7 +77,10 @@ function phabricator_form(PhabricatorUser $user, $attributes, $content) { $is_post = (strcasecmp($http_method, 'POST') === 0); $http_action = idx($attributes, 'action'); - $is_absolute_uri = preg_match('#^(https?:|//)#', $http_action); + $is_absolute_uri = false; + if ($http_action != null) { + $is_absolute_uri = preg_match('#^(https?:|//)#', $http_action); + } if ($is_post) { diff --git a/src/view/control/AphrontTableView.php b/src/view/control/AphrontTableView.php index a3c0a49be4..beafcaeb71 100644 --- a/src/view/control/AphrontTableView.php +++ b/src/view/control/AphrontTableView.php @@ -135,7 +135,7 @@ final class AphrontTableView extends AphrontView { $col_classes = array(); foreach ($this->columnClasses as $key => $class) { - if (strlen($class)) { + if ($class !== null && strlen($class)) { $col_classes[] = $class; } else { $col_classes[] = null; diff --git a/src/view/form/control/AphrontFormControl.php b/src/view/form/control/AphrontFormControl.php index 0125fa87b3..a21cf8164c 100644 --- a/src/view/form/control/AphrontFormControl.php +++ b/src/view/form/control/AphrontFormControl.php @@ -109,7 +109,7 @@ abstract class AphrontFormControl extends AphrontView { } public function isEmpty() { - return !strlen($this->getValue()); + return $this->getValue() === null || !strlen($this->getValue()); } public function getSerializedValue() { @@ -171,9 +171,8 @@ abstract class AphrontFormControl extends AphrontView { array('class' => 'aphront-form-input'), $this->renderInput()); - $error = null; - if (strlen($this->getError())) { - $error = $this->getError(); + $error = $this->error; + if ($error !== null && strlen($error)) { if ($error === true) { $error = phutil_tag( 'span', @@ -185,9 +184,12 @@ abstract class AphrontFormControl extends AphrontView { array('class' => 'aphront-form-error'), $error); } + } else { + $error = null; } - if (strlen($this->getLabel())) { + $label = $this->label; + if ($label !== null && strlen($label)) { $label = phutil_tag( 'label', array( @@ -203,7 +205,8 @@ abstract class AphrontFormControl extends AphrontView { $custom_class .= ' aphront-form-control-nolabel'; } - if (strlen($this->getCaption())) { + $caption = $this->caption; + if ($caption !== null && strlen($caption)) { $caption = phutil_tag( 'div', array('class' => 'aphront-form-caption'), diff --git a/src/view/form/control/AphrontFormTokenizerControl.php b/src/view/form/control/AphrontFormTokenizerControl.php index fe80c86f81..0b1dec6ec7 100644 --- a/src/view/form/control/AphrontFormTokenizerControl.php +++ b/src/view/form/control/AphrontFormTokenizerControl.php @@ -67,8 +67,8 @@ final class AphrontFormTokenizerControl extends AphrontFormControl { } $datasource->setViewer($this->getUser()); - $placeholder = null; - if (!strlen($this->placeholder)) { + $placeholder = $this->placeholder; + if ($placeholder === null || !strlen($placeholder)) { $placeholder = $datasource->getPlaceholderText(); } diff --git a/src/view/layout/AphrontSideNavFilterView.php b/src/view/layout/AphrontSideNavFilterView.php index b2d5a716d4..6d84d41d7d 100644 --- a/src/view/layout/AphrontSideNavFilterView.php +++ b/src/view/layout/AphrontSideNavFilterView.php @@ -97,11 +97,10 @@ final class AphrontSideNavFilterView extends AphrontView { ->setName($name) ->setType($type); - if (strlen($icon)) { + if ($icon !== null) { $item->setIcon($icon); } - if (strlen($key)) { $item->setKey($key); } @@ -145,7 +144,7 @@ final class AphrontSideNavFilterView extends AphrontView { public function selectFilter($key, $default = null) { $this->selectedFilter = $default; - if ($this->menu->getItem($key) && strlen($key)) { + if ($key !== null && strlen($key) && $this->menu->getItem($key)) { $this->selectedFilter = $key; } return $this->selectedFilter; diff --git a/src/view/page/PhabricatorStandardPageView.php b/src/view/page/PhabricatorStandardPageView.php index 6f6ce56cad..8949d7ae60 100644 --- a/src/view/page/PhabricatorStandardPageView.php +++ b/src/view/page/PhabricatorStandardPageView.php @@ -188,7 +188,7 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView } } - if (strlen($prefix)) { + if ($prefix !== null && strlen($prefix)) { $title = $prefix.' '.$title; } diff --git a/src/view/page/menu/PhabricatorMainMenuView.php b/src/view/page/menu/PhabricatorMainMenuView.php index 29a259b44c..28d3d91f58 100644 --- a/src/view/page/menu/PhabricatorMainMenuView.php +++ b/src/view/page/menu/PhabricatorMainMenuView.php @@ -333,7 +333,7 @@ final class PhabricatorMainMenuView extends AphrontView { $wordmark_text = PhabricatorCustomLogoConfigType::getLogoWordmark(); - if (!strlen($wordmark_text)) { + if ($wordmark_text === null || !strlen($wordmark_text)) { $wordmark_text = PlatformSymbols::getPlatformServerName(); } diff --git a/src/view/phui/PHUIInfoView.php b/src/view/phui/PHUIInfoView.php index cefeedbe5f..91d89afe41 100644 --- a/src/view/phui/PHUIInfoView.php +++ b/src/view/phui/PHUIInfoView.php @@ -44,8 +44,8 @@ final class PHUIInfoView extends AphrontTagView { return $this; } - public function setIsHidden($bool) { - $this->isHidden = $bool; + public function setIsHidden($is_hidden) { + $this->isHidden = $is_hidden; return $this; } @@ -147,7 +147,7 @@ final class PHUIInfoView extends AphrontTagView { } $title = $this->title; - if ($title || strlen($title)) { + if ($title !== null && strlen($title)) { $title = phutil_tag( 'h1', array( diff --git a/src/view/phui/PHUIObjectItemListView.php b/src/view/phui/PHUIObjectItemListView.php index fbc3904586..3cf703de4c 100644 --- a/src/view/phui/PHUIObjectItemListView.php +++ b/src/view/phui/PHUIObjectItemListView.php @@ -120,7 +120,7 @@ final class PHUIObjectItemListView extends AphrontTagView { require_celerity_resource('phui-oi-color-css'); $header = null; - if (strlen($this->header)) { + if ($this->header !== null && strlen($this->header)) { $header = phutil_tag( 'h1', array( @@ -141,7 +141,7 @@ final class PHUIObjectItemListView extends AphrontTagView { } $items = $this->items; - } else if ($this->allowEmptyList) { + } else if ($this->getAllowEmptyList()) { $items = null; } else { $string = nonempty($this->noDataString, pht('No data.')); diff --git a/src/view/phui/PHUIObjectItemView.php b/src/view/phui/PHUIObjectItemView.php index 4bb9c3ea6a..c246d9bb1d 100644 --- a/src/view/phui/PHUIObjectItemView.php +++ b/src/view/phui/PHUIObjectItemView.php @@ -97,6 +97,10 @@ final class PHUIObjectItemView extends AphrontTagView { return $this; } + public function getHeader() { + return $this->header; + } + public function setSubHead($subhead) { $this->subhead = $subhead; return $this; @@ -122,10 +126,6 @@ final class PHUIObjectItemView extends AphrontTagView { return $this->titleText; } - public function getHeader() { - return $this->header; - } - public function addByline($byline) { $this->bylines[] = $byline; return $this; @@ -659,8 +659,12 @@ final class PHUIObjectItemView extends AphrontTagView { $this->getImageIcon()); } - if ($image && (strlen($this->href) || strlen($this->imageHref))) { - $image_href = ($this->imageHref) ? $this->imageHref : $this->href; + $image_href = $this->href; + if ($image_href === null || !strlen($image_href)) { + $image_href = $this->imageHref; + } + + if ($image && $image_href !== null && strlen($image_href)) { $image = phutil_tag( 'a', array( @@ -873,7 +877,7 @@ final class PHUIObjectItemView extends AphrontTagView { 'class' => 'phui-oi-status-icon', ); - if (strlen($label)) { + if ($label !== null && strlen($label)) { $options['sigil'] = 'has-tooltip'; $options['meta'] = array('tip' => $label, 'size' => 300); } diff --git a/src/view/phui/PHUITagView.php b/src/view/phui/PHUITagView.php index cd6321a852..0c41be87da 100644 --- a/src/view/phui/PHUITagView.php +++ b/src/view/phui/PHUITagView.php @@ -105,6 +105,10 @@ final class PHUITagView extends AphrontTagView { return $this; } + public function getHref() { + return $this->href; + } + public function setClosed($closed) { $this->closed = $closed; return $this; @@ -126,7 +130,7 @@ final class PHUITagView extends AphrontTagView { } protected function getTagName() { - return strlen($this->href) ? 'a' : 'span'; + return ($this->href !== null && strlen($this->href)) ? 'a' : 'span'; } public function setContextObject($context_object) { diff --git a/src/view/widget/AphrontStackTraceView.php b/src/view/widget/AphrontStackTraceView.php index ff8cac43bc..ae6030d14f 100644 --- a/src/view/widget/AphrontStackTraceView.php +++ b/src/view/widget/AphrontStackTraceView.php @@ -30,7 +30,7 @@ final class AphrontStackTraceView extends AphrontView { $relative = $file; foreach ($libraries as $library) { $root = phutil_get_library_root($library); - if (Filesystem::isDescendant($file, $root)) { + if ($file !== null && Filesystem::isDescendant($file, $root)) { $lib = $library; $relative = Filesystem::readablePath($file, $root); break;