Addressing some PHP8 incompatibilities - Diffusion & Differential

Summary: Running through setting up and using Diffusion repositories and addressing PHP8 issues that come up.

Test Plan:
I set up a hosted mercurial repository and pushed commits to it over ssh.
I set up a hosted git repository and pushed commits to it over ssh.
I set up a mirrored mercurial repository over ssh.
I set up a mirrored git repository over ssh.

I created a diff on a git repository and landed it.
I created a diff on a mercurial repository and landed it.

I cloned and pushed a commit to a mercurial repo over http.
I cloned and pushed a commit to a git repo over http.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D21864

scripts/ssh/ssh-auth.php

@@ -36,7 +36,7 @@ $authstruct_raw = $cache->getKey($authstruct_key);
 
 $authstruct = null;
 
-if (strlen($authstruct_raw)) {
+if ($authstruct_raw !== null && strlen($authstruct_raw)) {
   try {
     $authstruct = phutil_json_decode($authstruct_raw);
   } catch (Exception $ex) {
@@ -135,7 +135,7 @@ foreach ($authstruct['keys'] as $key_struct) {
 
   $cmd = csprintf('%s %Ls', $bin, $key_argv);
 
-  if (strlen($instance)) {
+  if ($instance !== null && strlen($instance)) {
     $cmd = csprintf('PHABRICATOR_INSTANCE=%s %C', $instance, $cmd);
   }
 

src/applications/almanac/util/AlmanacKeys.php

@@ -58,7 +58,7 @@ final class AlmanacKeys extends Phobject {
 
   public static function getClusterSSHUser() {
     $username = PhabricatorEnv::getEnvConfig('diffusion.ssh-user');
-    if (strlen($username)) {
+    if ($username !== null && strlen($username)) {
       return $username;
     }
 

src/applications/conduit/query/PhabricatorConduitSearchEngine.php

@@ -39,7 +39,7 @@ final class PhabricatorConduitSearchEngine
     $query->withIsInternal(false);
 
     $contains = $saved->getParameter('nameContains');
-    if (strlen($contains)) {
+    if ($contains !== null && strlen($contains)) {
       $query->withNameContains($contains);
     }
 

src/applications/config/controller/module/PhabricatorConfigModuleController.php

@@ -9,7 +9,7 @@ final class PhabricatorConfigModuleController
 
     $all_modules = PhabricatorConfigModule::getAllModules();
 
-    if (!strlen($key)) {
+    if ($key === null || !strlen($key)) {
       $key = head_key($all_modules);
     }
 

src/applications/differential/conduit/DifferentialGetCommitMessageConduitAPIMethod.php

@@ -56,7 +56,7 @@ final class DifferentialGetCommitMessageConduitAPIMethod
     // show "Field:" templates for some fields even if they are empty.
     $edit_mode = $request->getValue('edit');
 
-    $is_any_edit = (bool)strlen($edit_mode);
+    $is_any_edit = $edit_mode !== null && (bool)strlen($edit_mode);
     $is_create = ($edit_mode == 'create');
 
     $field_list = DifferentialCommitMessageField::newEnabledFields($viewer);
@@ -115,7 +115,7 @@ final class DifferentialGetCommitMessageConduitAPIMethod
 
       $is_title = ($field_key == $key_title);
 
-      if (!strlen($value)) {
+      if ($value === null || !strlen($value)) {
         if ($is_template) {
           $commit_message[] = $label.': ';
         }

src/applications/differential/customfield/DifferentialBranchField.php

@@ -39,13 +39,20 @@ final class DifferentialBranchField
     $branch = $diff->getBranch();
     $bookmark = $diff->getBookmark();
 
+    if ($branch === null) {
+      $branch = '';
+    }
+    if ($bookmark === null) {
+      $bookmark = '';
+    }
+
     if (strlen($branch) && strlen($bookmark)) {
       return pht('%s (bookmark) on %s (branch)', $bookmark, $branch);
     } else if (strlen($bookmark)) {
       return pht('%s (bookmark)', $bookmark);
     } else if (strlen($branch)) {
       $onto = $diff->loadTargetBranch();
-      if (strlen($onto) && ($onto !== $branch)) {
+      if ($onto !== null && strlen($onto) && ($onto !== $branch)) {
         return pht(
           '%s (branched from %s)',
           $branch,

src/applications/differential/field/DifferentialCommitMessageField.php

@@ -60,7 +60,7 @@ abstract class DifferentialCommitMessageField
   }
 
   public function renderFieldValue($value) {
-    if (!strlen($value)) {
+    if ($value === null || !strlen($value)) {
       return null;
     }
 

src/applications/differential/field/DifferentialRevisionIDCommitMessageField.php

@@ -72,7 +72,7 @@ final class DifferentialRevisionIDCommitMessageField
   }
 
   public function renderFieldValue($value) {
-    if (!strlen($value)) {
+    if ($value === null || !strlen($value)) {
       return null;
     }
 

src/applications/differential/storage/DifferentialChangeset.php

@@ -325,7 +325,7 @@ final class DifferentialChangeset
 
   public function getOldStatePathVector() {
     $path = $this->getOldFile();
-    if (!strlen($path)) {
+    if ($path === null || !strlen($path)) {
       $path = $this->getFilename();
     }
 

src/applications/differential/view/DifferentialRevisionUpdateHistoryView.php

@@ -251,6 +251,7 @@ final class DifferentialRevisionUpdateHistoryView extends AphrontView {
     $content = phabricator_form(
       $this->getUser(),
       array(
+        'method' => 'GET',
         'action' => '/D'.$revision_id.'#toc',
       ),
       array(

src/applications/diffusion/conduit/DiffusionBranchQueryConduitAPIMethod.php

@@ -30,7 +30,7 @@ final class DiffusionBranchQueryConduitAPIMethod
     $repository = $drequest->getRepository();
 
     $contains = $request->getValue('contains');
-    if (strlen($contains)) {
+    if ($contains !== null && strlen($contains)) {
 
       // See PHI958 (and, earlier, PHI720). If "patterns" are provided, pass
       // them to "git branch ..." to let callers test for reachability from
@@ -80,7 +80,7 @@ final class DiffusionBranchQueryConduitAPIMethod
       ->setRepository($repository);
 
     $contains = $request->getValue('contains');
-    if (strlen($contains)) {
+    if ($contains !== null && strlen($contains)) {
       $query->withContainsCommit($contains);
     }
 

src/applications/diffusion/conduit/DiffusionBrowseQueryConduitAPIMethod.php

@@ -37,7 +37,7 @@ final class DiffusionBrowseQueryConduitAPIMethod
     $repository = $drequest->getRepository();
 
     $path = $request->getValue('path');
-    if (!strlen($path) || $path === '/') {
+    if ($path === null || !strlen($path) || $path === '/') {
       $path = null;
     }
 
@@ -282,8 +282,13 @@ final class DiffusionBrowseQueryConduitAPIMethod
 
     $results = array();
 
-    $match_against = trim($path, '/');
+    if ($path !== null) {
-    $match_len = strlen($match_against);
+      $match_against = trim($path, '/');
+      $match_len = strlen($match_against);
+    } else {
+      $match_against = '';
+      $match_len = 0;
+    }
 
     // For the root, don't trim. For other paths, trim the "/" after we match.
     // We need this because Mercurial's canonical paths have no leading "/",
@@ -295,7 +300,7 @@ final class DiffusionBrowseQueryConduitAPIMethod
       if (strncmp($path, $match_against, $match_len)) {
         continue;
       }
-      if (!strlen($path)) {
+      if ($path === null || !strlen($path)) {
         continue;
       }
       $remainder = substr($path, $trim_len);

src/applications/diffusion/conduit/DiffusionHistoryQueryConduitAPIMethod.php

@@ -45,16 +45,15 @@ final class DiffusionHistoryQueryConduitAPIMethod
     $repository = $drequest->getRepository();
     $commit_hash = $request->getValue('commit');
     $against_hash = $request->getValue('against');
-
-    $path = $request->getValue('path');
-    if (!strlen($path)) {
-      $path = null;
-    }
-
     $offset = $request->getValue('offset');
     $limit = $request->getValue('limit');
 
-    if (strlen($against_hash)) {
+    $path = $request->getValue('path');
+    if ($path === null || !strlen($path)) {
+      $path = null;
+    }
+
+    if ($against_hash !== null && strlen($against_hash)) {
       $commit_range = "{$against_hash}..{$commit_hash}";
     } else {
       $commit_range = $commit_hash;

src/applications/diffusion/controller/DiffusionBranchTableController.php

@@ -26,7 +26,7 @@ final class DiffusionBranchTableController extends DiffusionController {
     );
 
     $contains = $drequest->getSymbolicCommit();
-    if (strlen($contains)) {
+    if ($contains !== null && strlen($contains)) {
       $params['contains'] = $contains;
     }
 

src/applications/diffusion/controller/DiffusionBrowseController.php

@@ -22,7 +22,7 @@ final class DiffusionBrowseController extends DiffusionController {
     // list.
 
     $grep = $request->getStr('grep');
-    if (strlen($grep)) {
+    if (phutil_nonempty_string($grep)) {
       return $this->browseSearch();
     }
 
@@ -290,6 +290,11 @@ final class DiffusionBrowseController extends DiffusionController {
     $header = $this->buildHeaderView($drequest);
     $header->setHeaderIcon('fa-folder-open');
 
+    $title = '/';
+    if ($drequest->getPath() !== null) {
+      $title = nonempty(basename($drequest->getPath()), '/');
+    }
+
     $empty_result = null;
     $browse_panel = null;
     if (!$results->isValidResults()) {
@@ -303,7 +308,6 @@ final class DiffusionBrowseController extends DiffusionController {
         ->setPaths($results->getPaths())
         ->setUser($request->getUser());
 
-      $title = nonempty(basename($drequest->getPath()), '/');
       $icon = 'fa-folder-open';
       $browse_header = $this->buildPanelHeaderView($title, $icon);
 
@@ -351,7 +355,7 @@ final class DiffusionBrowseController extends DiffusionController {
 
     return $this->newPage()
       ->setTitle(array(
-          nonempty(basename($drequest->getPath()), '/'),
+          $title,
           $repository->getDisplayName(),
         ))
       ->setCrumbs($crumbs)

src/applications/diffusion/controller/DiffusionCommitController.php

@@ -27,8 +27,11 @@ final class DiffusionCommitController extends DiffusionController {
 
     // If this page is being accessed via "/source/xyz/commit/...", redirect
     // to the canonical URI.
-    $has_callsign = strlen($request->getURIData('repositoryCallsign'));
+    $repo_callsign = $request->getURIData('repositoryCallsign');
-    $has_id = strlen($request->getURIData('repositoryID'));
+    $has_callsign = $repo_callsign !== null && strlen($repo_callsign);
+    $repo_id = $request->getURIData('repositoryID');
+    $has_id = $repo_id !== null && strlen($repo_id);
+
     if (!$has_callsign && !$has_id) {
       $canonical_uri = $repository->getCommitURI($commit_identifier);
       return id(new AphrontRedirectResponse())
@@ -922,7 +925,7 @@ final class DiffusionCommitController extends DiffusionController {
 
   private function linkBugtraq($corpus) {
     $url = PhabricatorEnv::getEnvConfig('bugtraq.url');
-    if (!strlen($url)) {
+    if ($url === null || !strlen($url)) {
       return $corpus;
     }
 

src/applications/diffusion/controller/DiffusionController.php

@@ -265,7 +265,10 @@ abstract class DiffusionController extends PhabricatorController {
 
   protected function renderPathLinks(DiffusionRequest $drequest, $action) {
     $path = $drequest->getPath();
-    $path_parts = array_filter(explode('/', trim($path, '/')));
+    $path_parts = array();
+    if ($path !== null && strlen($path)) {
+      $path_parts = array_filter(explode('/', trim($path, '/')));
+    }
 
     $divider = phutil_tag(
       'span',

src/applications/diffusion/controller/DiffusionHistoryController.php

@@ -50,7 +50,8 @@ final class DiffusionHistoryController extends DiffusionController {
     // ancestors appropriately, but this would currrently be prohibitively
     // expensive in the general case.
 
-    $show_graph = !strlen($drequest->getPath());
+    $show_graph = ($drequest->getPath() === null
+      || !strlen($drequest->getPath()));
     if ($show_graph) {
       $history_list
         ->setParents($history_results['parents'])
@@ -98,7 +99,7 @@ final class DiffusionHistoryController extends DiffusionController {
     $viewer = $this->getViewer();
     $repository = $drequest->getRepository();
 
-    $no_path = !strlen($drequest->getPath());
+    $no_path = $drequest->getPath() === null || !strlen($drequest->getPath());
     if ($no_path) {
       $header_text = pht('History');
     } else {

src/applications/diffusion/controller/DiffusionRepositoryManagePanelsController.php

@@ -40,7 +40,7 @@ final class DiffusionRepositoryManagePanelsController
     }
 
     $selected = $request->getURIData('panel');
-    if (!strlen($selected)) {
+    if ($selected === null || !strlen($selected)) {
       $selected = head_key($panels);
     }
 

src/applications/diffusion/controller/DiffusionServeController.php

@@ -183,11 +183,13 @@ final class DiffusionServeController extends DiffusionController {
     // won't prompt users who provide a username but no password otherwise.
     // See T10797 for discussion.
 
-    $have_user = strlen(idx($_SERVER, 'PHP_AUTH_USER'));
+    $http_user = idx($_SERVER, 'PHP_AUTH_USER');
-    $have_pass = strlen(idx($_SERVER, 'PHP_AUTH_PW'));
+    $http_pass = idx($_SERVER, 'PHP_AUTH_PW');
+    $have_user = $http_user !== null && strlen($http_user);
+    $have_pass = $http_pass !== null && strlen($http_pass);
     if ($have_user && $have_pass) {
-      $username = $_SERVER['PHP_AUTH_USER'];
+      $username = $http_user;
-      $password = new PhutilOpaqueEnvelope($_SERVER['PHP_AUTH_PW']);
+      $password = new PhutilOpaqueEnvelope($http_pass);
 
       // Try Git LFS auth first since we can usually reject it without doing
       // any queries, since the username won't match the one we expect or the
@@ -524,9 +526,15 @@ final class DiffusionServeController extends DiffusionController {
         break;
       case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL:
         $cmd = $request->getStr('cmd');
+        if ($cmd === null) {
+          return false;
+        }
         if ($cmd == 'batch') {
           $cmds = idx($this->getMercurialArguments(), 'cmds');
-          return DiffusionMercurialWireProtocol::isReadOnlyBatchCommand($cmds);
+          if ($cmds !== null) {
+            return DiffusionMercurialWireProtocol::isReadOnlyBatchCommand(
+              $cmds);
+          }
         }
         return DiffusionMercurialWireProtocol::isReadOnlyCommand($cmd);
       case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:

src/applications/diffusion/controller/DiffusionTagListController.php

@@ -25,7 +25,9 @@ final class DiffusionTagListController extends DiffusionController {
       'offset' => $pager->getOffset(),
     );
 
-    if (strlen($drequest->getSymbolicCommit())) {
+    if ($drequest->getSymbolicCommit() !== null
+      && strlen($drequest->getSymbolicCommit())) {
+
       $is_commit = true;
       $params['commit'] = $drequest->getSymbolicCommit();
     } else {

src/applications/diffusion/data/DiffusionCommitRef.php

@@ -131,6 +131,13 @@ final class DiffusionCommitRef extends Phobject {
   }
 
   private function formatUser($name, $email) {
+    if ($name === null) {
+      $name = '';
+    }
+    if ($email === null) {
+      $email = '';
+    }
+
     if (strlen($name) && strlen($email)) {
       return "{$name} <{$email}>";
     } else if (strlen($email)) {

src/applications/diffusion/document/DiffusionDocumentRenderingEngine.php

@@ -87,7 +87,7 @@ final class DiffusionDocumentRenderingEngine
     $ref->setSymbolMetadata($this->getSymbolMetadata());
 
     $coverage = $drequest->loadCoverage();
-    if (strlen($coverage)) {
+    if ($coverage !== null && strlen($coverage)) {
       $ref->addCoverage($coverage);
     }
   }

src/applications/diffusion/engine/DiffusionCommitHookEngine.php

@@ -1133,7 +1133,7 @@ final class DiffusionCommitHookEngine extends Phobject {
       ->setHookWait(phutil_microseconds_since($hook_start));
 
     $identifier = $this->getRequestIdentifier();
-    if (strlen($identifier)) {
+    if ($identifier !== null && strlen($identifier)) {
       $event->setRequestIdentifier($identifier);
     }
 

src/applications/diffusion/query/DiffusionFileFutureQuery.php

@@ -92,7 +92,10 @@ abstract class DiffusionFileFutureQuery
 
     $drequest = $this->getRequest();
 
-    $name = basename($drequest->getPath());
+    $name = '';
+    if ($drequest->getPath() !== null) {
+      $name = basename($drequest->getPath());
+    }
     $relative_ttl = phutil_units('48 hours in seconds');
 
     try {

src/applications/diffusion/query/lowlevel/DiffusionLowLevelMercurialPathsQuery.php

@@ -32,8 +32,12 @@ final class DiffusionLowLevelMercurialPathsQuery
       $hg_paths_command = 'locate --print0 --rev %s -I %s';
     }
 
-    $match_against = trim($path, '/');
+    if ($path !== null) {
-    $prefix = trim('./'.$match_against, '/');
+      $match_against = trim($path, '/');
+      $prefix = trim('./'.$match_against, '/');
+    } else {
+      $prefix = '.';
+    }
     list($entire_manifest) = $repository->execxLocalCommand(
       $hg_paths_command,
       hgsprintf('%s', $commit),

src/applications/diffusion/query/pathid/DiffusionPathIDQuery.php

@@ -48,6 +48,10 @@ final class DiffusionPathIDQuery extends Phobject {
    */
   public static function normalizePath($path) {
 
+    if ($path === null) {
+      return '/';
+    }
+
     // Normalize to single slashes, e.g. "///" => "/".
     $path = preg_replace('@[/]{2,}@', '/', $path);
 

src/applications/diffusion/query/rawdiff/DiffusionGitRawDiffQuery.php

@@ -35,7 +35,7 @@ final class DiffusionGitRawDiffQuery extends DiffusionRawDiffQuery {
     }
 
     $path = $drequest->getPath();
-    if (!strlen($path)) {
+    if ($path === null || !strlen($path)) {
       $path = '.';
     }
 

src/applications/diffusion/request/DiffusionGitRequest.php

@@ -3,7 +3,7 @@
 final class DiffusionGitRequest extends DiffusionRequest {
 
   protected function isStableCommit($symbol) {
-    return preg_match('/^[a-f0-9]{40}\z/', $symbol);
+    return $symbol !== null && preg_match('/^[a-f0-9]{40}\z/', $symbol);
   }
 
   public function getBranch() {

src/applications/diffusion/request/DiffusionMercurialRequest.php

@@ -3,7 +3,7 @@
 final class DiffusionMercurialRequest extends DiffusionRequest {
 
   protected function isStableCommit($symbol) {
-    return preg_match('/^[a-f0-9]{40}\z/', $symbol);
+    return $symbol !== null && preg_match('/^[a-f0-9]{40}\z/', $symbol);
   }
 
   public function getBranch() {

src/applications/diffusion/request/DiffusionSvnRequest.php

@@ -3,7 +3,7 @@
 final class DiffusionSvnRequest extends DiffusionRequest {
 
   protected function isStableCommit($symbol) {
-    return preg_match('/^[1-9]\d*\z/', $symbol);
+    return $symbol !== null && preg_match('/^[1-9]\d*\z/', $symbol);
   }
 
   protected function didInitialize() {

src/applications/diffusion/view/DiffusionBrowseTableView.php

@@ -15,9 +15,13 @@ final class DiffusionBrowseTableView extends DiffusionView {
     $repository = $request->getRepository();
     require_celerity_resource('diffusion-css');
 
-    $base_path = trim($request->getPath(), '/');
+    if ($request->getPath() !== null) {
-    if ($base_path) {
+      $base_path = trim($request->getPath(), '/');
-      $base_path = $base_path.'/';
+      if ($base_path) {
+        $base_path = $base_path.'/';
+      }
+    } else {
+      $base_path = '';
     }
 
     $need_pull = array();

src/applications/diffusion/view/DiffusionView.php

@@ -71,7 +71,7 @@ abstract class DiffusionView extends AphrontView {
     $display_name = idx($details, 'name');
     unset($details['name']);
 
-    if (strlen($display_name)) {
+    if ($display_name !== null && strlen($display_name)) {
       $display_name = phutil_tag(
         'span',
         array(

src/applications/repository/editor/PhabricatorRepositoryEditor.php

@@ -50,7 +50,7 @@ final class PhabricatorRepositoryEditor
     // If the repository does not have a local path yet, assign it one based
     // on its ID. We can't do this earlier because we won't have an ID yet.
     $local_path = $object->getLocalPath();
-    if (!strlen($local_path)) {
+    if ($local_path === null || !strlen($local_path)) {
       $local_key = 'repository.default-local-path';
 
       $local_root = PhabricatorEnv::getEnvConfig($local_key);

src/applications/repository/engine/PhabricatorRepositoryPullEngine.php

@@ -238,7 +238,7 @@ final class PhabricatorRepositoryPullEngine
     $identifier = $repository->getPHID();
 
     $instance = PhabricatorEnv::getEnvConfig('cluster.instance');
-    if (strlen($instance)) {
+    if ($instance !== null && strlen($instance)) {
       $identifier = "{$identifier}:{$instance}";
     }
 
@@ -575,7 +575,7 @@ final class PhabricatorRepositoryPullEngine
       $ref_rules);
 
     // Empty repositories don't have any refs.
-    if (!strlen(rtrim($stdout))) {
+    if ($stdout === null || !strlen(rtrim($stdout))) {
       return array();
     }
 

src/applications/repository/management/PhabricatorRepositoryManagementRebuildIdentitiesWorkflow.php

@@ -220,7 +220,7 @@ final class PhabricatorRepositoryManagementRebuildIdentitiesWorkflow
 
       $committer_name = $data->getCommitterString();
       $committer_phid = $commit->getCommitterIdentityPHID();
-      if (strlen($committer_name)) {
+      if (phutil_nonempty_string($committer_name)) {
         $committer_identity = $this->getIdentityForCommit(
           $commit,
           $committer_name);

src/applications/repository/query/PhabricatorRepositoryRefCursorQuery.php

@@ -132,7 +132,7 @@ final class PhabricatorRepositoryRefCursorQuery
         $name_hashes);
     }
 
-    if (strlen($this->datasourceQuery)) {
+    if ($this->datasourceQuery !== null && strlen($this->datasourceQuery)) {
       $where[] = qsprintf(
         $conn,
         'refNameRaw LIKE %>',

src/applications/repository/storage/PhabricatorRepository.php

@@ -345,7 +345,7 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
 
     // Make some reasonable effort to produce reasonable default directory
     // names from repository names.
-    if (!strlen($name)) {
+    if ($name === null || !strlen($name)) {
       $name = $this->getName();
       $name = phutil_utf8_strtolower($name);
       $name = preg_replace('@[ -/:->]+@', '-', $name);
@@ -721,14 +721,14 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
     $head = idx($params, 'head');
     $against = idx($params, 'against');
 
-    if ($req_commit && !strlen($commit)) {
+    if ($req_commit && ($commit === null || !strlen($commit))) {
       throw new Exception(
         pht(
           'Diffusion URI action "%s" requires commit!',
           $action));
     }
 
-    if ($req_branch && !strlen($branch)) {
+    if ($req_branch && ($branch === null || !strlen($branch))) {
       throw new Exception(
         pht(
           'Diffusion URI action "%s" requires branch!',
@@ -779,20 +779,20 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
         break;
       case 'compare':
         $uri = $this->getPathURI("/{$action}/");
-        if (strlen($head)) {
+        if ($head !== null && strlen($head)) {
           $query['head'] = $head;
-        } else if (strlen($raw_commit)) {
+        } else if ($raw_commit !== null && strlen($raw_commit)) {
           $query['commit'] = $raw_commit;
-        } else if (strlen($raw_branch)) {
+        } else if ($raw_branch !== null && strlen($raw_branch)) {
           $query['head'] = $raw_branch;
         }
 
-        if (strlen($against)) {
+        if ($against !== null && strlen($against)) {
           $query['against'] = $against;
         }
         break;
       case 'branch':
-        if (strlen($path)) {
+        if ($path != null && strlen($path)) {
           $uri = $this->getPathURI("/repository/{$path}");
         } else {
           $uri = $this->getPathURI('/');
@@ -1160,7 +1160,7 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
    */
   public function getRemoteURIObject() {
     $raw_uri = $this->getDetail('remote-uri');
-    if (!strlen($raw_uri)) {
+    if ($raw_uri === null || !strlen($raw_uri)) {
       return new PhutilURI('');
     }
 
@@ -2818,7 +2818,7 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
     $permanent_rules = $this->getStringListForConduit($permanent_rules);
 
     $default_branch = $this->getDefaultBranch();
-    if (!strlen($default_branch)) {
+    if ($default_branch === null || !strlen($default_branch)) {
       $default_branch = null;
     }
 

src/applications/repository/storage/PhabricatorRepositoryCommit.php

@@ -478,7 +478,7 @@ final class PhabricatorRepositoryCommit
     }
 
     $author = $this->getRawAuthorStringForDisplay();
-    if (strlen($author)) {
+    if ($author !== null && strlen($author)) {
       return DiffusionView::renderName($author);
     }
 
@@ -493,7 +493,7 @@ final class PhabricatorRepositoryCommit
     }
 
     $committer = $this->getRawCommitterStringForDisplay();
-    if (strlen($committer)) {
+    if ($committer !== null && strlen($committer)) {
       return DiffusionView::renderName($committer);
     }
 

src/applications/repository/storage/PhabricatorRepositoryCommitData.php

@@ -97,7 +97,7 @@ final class PhabricatorRepositoryCommitData extends PhabricatorRepositoryDAO {
     $ref = $this->getCommitRef();
 
     $author = $ref->getAuthor();
-    if (strlen($author)) {
+    if ($author !== null && strlen($author)) {
       return $author;
     }
 
@@ -131,7 +131,7 @@ final class PhabricatorRepositoryCommitData extends PhabricatorRepositoryDAO {
     $ref = $this->getCommitRef();
 
     $committer = $ref->getCommitter();
-    if (strlen($committer)) {
+    if ($committer !== null && strlen($committer)) {
       return $committer;
     }
 

src/applications/repository/storage/PhabricatorRepositoryPushLog.php

@@ -158,6 +158,9 @@ final class PhabricatorRepositoryPushLog
   }
 
   public function getRefName() {
+    if ($this->getRefNameRaw() === null) {
+      return null;
+    }
     return $this->getUTF8StringFromStorage(
       $this->getRefNameRaw(),
       $this->getRefNameEncoding());
@@ -175,6 +178,9 @@ final class PhabricatorRepositoryPushLog
     if ($this->getRepository()->isSVN()) {
       return $this->getRefOld();
     }
+    if ($this->getRefOld() === null) {
+      return null;
+    }
     return substr($this->getRefOld(), 0, 12);
   }
 

src/applications/repository/storage/PhabricatorRepositoryURI.php

@@ -752,7 +752,7 @@ final class PhabricatorRepositoryURI
     // without requiring an index rebuild.
 
     $ssh_host = PhabricatorEnv::getEnvConfig('diffusion.ssh-host');
-    if (strlen($ssh_host)) {
+    if ($ssh_host !== null && strlen($ssh_host)) {
       $domain_map['<ssh-host>'] = $ssh_host;
     }
 

src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php

@@ -36,7 +36,7 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker
 
     $author = $ref->getAuthor();
     $committer = $ref->getCommitter();
-    $has_committer = (bool)strlen($committer);
+    $has_committer = $committer !== null && (bool)strlen($committer);
 
     $identity_engine = id(new DiffusionRepositoryIdentityEngine())
       ->setViewer($viewer)

src/applications/settings/panel/PhabricatorExternalEditorSettingsPanel.php

@@ -39,7 +39,7 @@ final class PhabricatorExternalEditorSettingsPanel
     $viewer = $this->getViewer();
     $pattern = $viewer->getUserSetting(PhabricatorEditorSetting::SETTINGKEY);
 
-    if (!strlen($pattern)) {
+    if ($pattern === null || !strlen($pattern)) {
       return null;
     }
 

src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php

@@ -2363,7 +2363,9 @@ abstract class PhabricatorApplicationTransactionEditor
         // Here, we don't care about processing only new mentions after an edit
         // because there is no way for an object to ever "unmention" itself on
         // another object, so we can ignore the old value.
-        $engine->markupText($change->getNewValue());
+        if ($change->getNewValue() !== null) {
+          $engine->markupText($change->getNewValue());
+        }
 
         $mentioned_phids += $engine->getTextMetadata(
           PhabricatorObjectRemarkupRule::KEY_MENTIONED_OBJECTS,

src/infrastructure/contentsource/PhabricatorUnknownContentSource.php

@@ -7,7 +7,7 @@ final class PhabricatorUnknownContentSource
 
   public function getSourceName() {
     $source = $this->getSource();
-    if (strlen($source)) {
+    if ($source !== null && strlen($source)) {
       return pht('Unknown ("%s")', $source);
     } else {
       return pht('Unknown');

src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldRemarkup.php

@@ -30,7 +30,7 @@ final class PhabricatorStandardCustomFieldRemarkup
   public function renderPropertyViewValue(array $handles) {
     $value = $this->getFieldValue();
 
-    if (!strlen($value)) {
+    if ($value === null || !strlen($value)) {
       return null;
     }
 

src/infrastructure/diff/view/PHUIDiffTableOfContentsItemView.php

@@ -139,7 +139,7 @@ final class PHUIDiffTableOfContentsItemView extends AphrontView {
     $not_applicable = '-';
 
     $coverage = $this->getCoverage();
-    if (!strlen($coverage)) {
+    if ($coverage === null || !strlen($coverage)) {
       return $not_applicable;
     }
 
@@ -157,7 +157,7 @@ final class PHUIDiffTableOfContentsItemView extends AphrontView {
     $not_applicable = '-';
 
     $coverage = $this->getCoverage();
-    if (!strlen($coverage)) {
+    if ($coverage === null || !strlen($coverage)) {
       return $not_applicable;
     }
 

src/infrastructure/editor/PhabricatorEditorURIEngine.php

@@ -16,7 +16,7 @@ final class PhabricatorEditorURIEngine
 
     $pattern = $viewer->getUserSetting(PhabricatorEditorSetting::SETTINGKEY);
 
-    if (!strlen(trim($pattern))) {
+    if ($pattern === null || !strlen(trim($pattern))) {
       return null;
     }
 

src/infrastructure/storage/lisk/PhabricatorLiskDAO.php

@@ -42,7 +42,7 @@ abstract class PhabricatorLiskDAO extends LiskDAO {
     if (!strlen($namespace)) {
       $namespace = self::getDefaultStorageNamespace();
     }
-    if (!strlen($namespace)) {
+    if ($namespace === null || !strlen($namespace)) {
       throw new Exception(pht('No storage namespace configured!'));
     }
     return $namespace;
@@ -295,7 +295,7 @@ abstract class PhabricatorLiskDAO extends LiskDAO {
     }
 
     if (function_exists('mb_detect_encoding')) {
-      if (strlen($encoding)) {
+      if ($encoding !== null && strlen($encoding)) {
         $try_encodings = array(
           $encoding,
         );

src/view/phui/PHUIPagerView.php

@@ -50,7 +50,12 @@ final class PHUIPagerView extends AphrontView {
   public function readFromRequest(AphrontRequest $request) {
     $this->uri = $request->getRequestURI();
     $this->pagingParameter = 'offset';
-    $this->offset = $request->getInt($this->pagingParameter);
+
+    $offset = $request->getInt($this->pagingParameter);
+    if ($offset === null) {
+      $offset = 0;
+    }
+    $this->offset = $offset;
     return $this;
   }