Drop PSR7 v1 and redact URI user info in errors (#3236)

* Redact URI user info in errors Co-Authored-By: Tim Düsterhus <209270+TimWolla@users.noreply.github.com> * guzzlehttp/psr7 `^2.7` * Update CurlFactory.php * Fizes * Update CurlFactory.php * Update CurlFactory.php * Update CurlFactory.php --------- Co-authored-by: Tim Düsterhus <209270+TimWolla@users.noreply.github.com>
guzzle · Jul 18, 2024 · 3bec073 · 3bec073
1 parent 740e191
commit 3bec073
Show file tree

Hide file tree

Showing 6 changed files with 47 additions and 38 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -52,19 +52,10 @@ jobs:
       max-parallel: 10
       matrix:
         php: ['7.2', '7.3', '7.4', '8.0', '8.1', '8.2', '8.3', '8.4']
-        psr7: ['^1.9.1', '^2.6.3']
+        psr7: ['^2.7.0']
         include:
           - php: '8.4'
-            psr7: '^2.6.3@dev'
-        exclude:
-          - php: '8.1'
-            psr7: '^1.9.1'
-          - php: '8.2'
-            psr7: '^1.9.1'
-          - php: '8.3'
-            psr7: '^1.9.1'
-          - php: '8.4'
-            psr7: '^1.9.1'
+            psr7: '^2.7.0@dev'
 
     steps:
       - name: Set up PHP

diff --git a/composer.json b/composer.json
@@ -82,7 +82,7 @@
         "php": "^7.2.5 || ^8.0",
         "ext-json": "*",
         "guzzlehttp/promises": "^1.5.3 || ^2.0.3",
-        "guzzlehttp/psr7": "^1.9.1 || ^2.6.3",
+        "guzzlehttp/psr7": "^2.7.0",
         "psr/http-client": "^1.0",
         "symfony/deprecation-contracts": "^2.2 || ^3.0"
     },

diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -35,11 +35,6 @@ parameters:
 			count: 3
 			path: src/Cookie/SetCookie.php
 
-		-
-			message: "#^Unsafe call to private method GuzzleHttp\\\\Exception\\\\RequestException\\:\\:obfuscateUri\\(\\) through static\\:\\:\\.$#"
-			count: 1
-			path: src/Exception/RequestException.php
-
 		-
 			message: "#^Cannot access offset 'features' on array\\|false\\.$#"
 			count: 3

diff --git a/src/Exception/RequestException.php b/src/Exception/RequestException.php
@@ -7,7 +7,6 @@
 use Psr\Http\Client\RequestExceptionInterface;
 use Psr\Http\Message\RequestInterface;
 use Psr\Http\Message\ResponseInterface;
-use Psr\Http\Message\UriInterface;
 
 /**
  * HTTP Request exception
@@ -90,8 +89,7 @@ public static function create(
             $className = __CLASS__;
         }
 
-        $uri = $request->getUri();
-        $uri = static::obfuscateUri($uri);
+        $uri = \GuzzleHttp\Psr7\Utils::redactUserInfo($request->getUri());
 
         // Client Error: `GET /` resulted in a `404 Not Found` response:
         // <html> ... (truncated)
@@ -113,20 +111,6 @@ public static function create(
         return new $className($message, $request, $response, $previous, $handlerContext);
     }
 
-    /**
-     * Obfuscates URI if there is a username and a password present
-     */
-    private static function obfuscateUri(UriInterface $uri): UriInterface
-    {
-        $userInfo = $uri->getUserInfo();
-
-        if (false !== ($pos = \strpos($userInfo, ':'))) {
-            return $uri->withUserInfo(\substr($userInfo, 0, $pos), '***');
-        }
-
-        return $uri;
-    }
-
     /**
      * Get the request that caused the exception
      */

diff --git a/src/Handler/CurlFactory.php b/src/Handler/CurlFactory.php
@@ -11,6 +11,7 @@
 use GuzzleHttp\TransferStats;
 use GuzzleHttp\Utils;
 use Psr\Http\Message\RequestInterface;
+use Psr\Http\Message\UriInterface;
 
 /**
  * Creates curl resources from a request
@@ -251,15 +252,22 @@ private static function createRejection(EasyHandle $easy, array $ctx): PromiseIn
             );
         }
 
+        $uri = $easy->request->getUri();
+
+        $sanitizedError = self::sanitizeCurlError($ctx['error'] ?? '', $uri);
+
         $message = \sprintf(
             'cURL error %s: %s (%s)',
             $ctx['errno'],
-            $ctx['error'],
+            $sanitizedError,
             'see https://curl.haxx.se/libcurl/c/libcurl-errors.html'
         );
-        $uriString = (string) $easy->request->getUri();
-        if ($uriString !== '' && false === \strpos($ctx['error'], $uriString)) {
-            $message .= \sprintf(' for %s', $uriString);
+
+        if ('' !== $sanitizedError) {
+            $redactedUriString = \GuzzleHttp\Psr7\Utils::redactUserInfo($uri)->__toString();
+            if ($redactedUriString !== '' && false === \strpos($sanitizedError, $redactedUriString)) {
+                $message .= \sprintf(' for %s', $redactedUriString);
+            }
         }
 
         // Create a connection exception if it was a specific error code.
@@ -270,6 +278,24 @@ private static function createRejection(EasyHandle $easy, array $ctx): PromiseIn
         return P\Create::rejectionFor($error);
     }
 
+    private static function sanitizeCurlError(string $error, UriInterface $uri): string
+    {
+        if ('' === $error) {
+            return $error;
+        }
+
+        $baseUri = $uri->withQuery('')->withFragment('');
+        $baseUriString = $baseUri->__toString();
+
+        if ('' === $baseUriString) {
+            return $error;
+        }
+
+        $redactedUriString = \GuzzleHttp\Psr7\Utils::redactUserInfo($baseUri)->__toString();
+
+        return str_replace($baseUriString, $redactedUriString, $error);
+    }
+
     /**
      * @return array<int|string, mixed>
      */

diff --git a/tests/Handler/CurlHandlerTest.php b/tests/Handler/CurlHandlerTest.php
@@ -32,6 +32,19 @@ public function testCreatesCurlErrors()
         $handler($request, ['timeout' => 0.001, 'connect_timeout' => 0.001])->wait();
     }
 
+    public function testRedactsUserInfoInErrors()
+    {
+        $handler = new CurlHandler();
+        $request = new Request('GET', 'http://my_user:secretPass@localhost:123');
+
+        try {
+            $handler($request, ['timeout' => 0.001, 'connect_timeout' => 0.001])->wait();
+            $this->fail('Must throw an Exception.');
+        } catch (\Throwable $e) {
+            $this->assertStringNotContainsString('secretPass', $e->getMessage());
+        }
+    }
+
     public function testReusesHandles()
     {
         Server::flush();