Correct Aphlict websocket URI construction after PHP8 compatibility changes

epriestley · epriestley · commit 5720a38cfe95 · 2023-11-13T11:04:04.000-08:00
Summary: See D21862. Ref T13700. D21862 affected notification server URI generation behavior when a notification server client is configured with: 1. no "path" argument; and 2. "cluster.instance" not set. Condition (1) is not true default, and condition (2) is not true in my local environment, so it was easy for this to slip through the cracks. Apply the change suggested in D21862. Also fix a couple other string-null issues I caught locally. Test Plan: Generated notification server URIs under various simulated local conditions (no instance, path set) and everything seemed to be working correctly. Maniphest Tasks: T13700 Differential Revision: https://secure.phabricator.com/D21875
diff --git a/src/applications/auth/constants/PhabricatorCookies.php b/src/applications/auth/constants/PhabricatorCookies.php
@@ -164,7 +164,7 @@ private static function parseNextURICookie($cookie) {
     // Old cookies look like: /uri
     // New cookies look like: timestamp,/uri
 
-    if (!strlen($cookie)) {
+    if (!phutil_nonempty_string($cookie)) {
       return null;
     }
 
diff --git a/src/applications/auth/controller/PhabricatorAuthSetExternalController.php b/src/applications/auth/controller/PhabricatorAuthSetExternalController.php
@@ -40,7 +40,7 @@ public function handleRequest(AphrontRequest $request) {
     $text = PhabricatorAuthMessage::loadMessageText(
       $viewer,
       PhabricatorAuthLinkMessageType::MESSAGEKEY);
-    if (!strlen($text)) {
+    if (!phutil_nonempty_string($text)) {
       $text = pht(
         'You can link your %s account to an external account to '.
         'allow you to log in more easily in the future. To continue, choose '.
diff --git a/src/applications/notification/client/PhabricatorNotificationServerRef.php b/src/applications/notification/client/PhabricatorNotificationServerRef.php
@@ -147,7 +147,7 @@ public function getURI($to_path = null) {
     if ($to_path === null || !strlen($to_path)) {
       $to_path = '';
     } else {
-      $to_path = '/'.ltrim($to_path, '/');
+      $to_path = ltrim($to_path, '/');
     }
 
     $base_path = $this->getPath();
@@ -156,7 +156,7 @@ public function getURI($to_path = null) {
     } else {
       $base_path = rtrim($base_path, '/');
     }
-    $full_path = $base_path.$to_path;
+    $full_path = $base_path.'/'.$to_path;
 
     $uri = id(new PhutilURI('http://'.$this->getHost()))
       ->setProtocol($this->getProtocol())

Original file line number	Diff line number	Diff line change
`@@ -164,7 +164,7 @@ private static function parseNextURICookie($cookie) {`
`164`	`164`	`// Old cookies look like: /uri`
`165`	`165`	`// New cookies look like: timestamp,/uri`
`166`	`166`
`167`		`- if (!strlen($cookie)) {`
	`167`	`+ if (!phutil_nonempty_string($cookie)) {`
`168`	`168`	`return null;`
`169`	`169`	`}`
`170`	`170`