Replace mock with laminas URI implementation

This also centralizes the logic to build path + query. Adding the whole URI to the exception message has been removed, because the string format is always displayed as absolute path, which is confusing.
amphp · May 10, 2020 · 3ae1c6c · 3ae1c6c
1 parent d52aaa7
commit 3ae1c6c
Show file tree

Hide file tree

Showing 6 changed files with 72 additions and 82 deletions.
diff --git a/composer.json b/composer.json
@@ -42,11 +42,12 @@
         "amphp/phpunit-util": "^1.1",
         "amphp/php-cs-fixer-config": "dev-master",
         "phpunit/phpunit": "^7 || ^8 || ^9",
-        "amphp/http-server": "^2-rc4",
+        "amphp/http-server": "^2",
         "kelunik/link-header-rfc5988": "^1.0",
         "clue/socks-react": "^1.0",
         "amphp/react-adapter": "^2.1",
-        "vimeo/psalm": "^3.9@dev"
+        "vimeo/psalm": "^3.9@dev",
+        "laminas/laminas-diactoros": "^2.3"
     },
     "suggest": {
         "ext-zlib": "Allows using compression for response bodies.",
@@ -56,7 +57,10 @@
     "autoload": {
         "psr-4": {
             "Amp\\Http\\Client\\": "src"
-        }
+        },
+        "files": [
+            "src/Internal/functions.php"
+        ]
     },
     "autoload-dev": {
         "psr-4": {

diff --git a/src/Connection/Http1Connection.php b/src/Connection/Http1Connection.php
@@ -36,6 +36,7 @@
 use function Amp\asyncCall;
 use function Amp\call;
 use function Amp\getCurrentTime;
+use function Amp\Http\Client\Internal\normalizeRequestPathWithQuery;
 
 /**
  * Socket client implementation.
@@ -292,7 +293,8 @@ private function readResponse(
                     ? Promise\timeout($this->socket->read(), $timeout)
                     : $this->socket->read()
             ) {
-                parseChunk: $response = $parser->parse($chunk);
+                parseChunk:
+                $response = $parser->parse($chunk);
                 if ($response === null) {
                     if ($this->socket === null) {
                         throw new SocketException('Socket closed prior to response completion');
@@ -414,7 +416,11 @@ private function readResponse(
                                 );
                             } catch (PromiseTimeoutException $e) {
                                 $this->close();
-                                throw new TimeoutException('Inactivity timeout exceeded, more than ' . $timeout . ' ms elapsed from last data received', 0, $e);
+                                throw new TimeoutException(
+                                    'Inactivity timeout exceeded, more than ' . $timeout . ' ms elapsed from last data received',
+                                    0,
+                                    $e
+                                );
                             }
 
                             $originalCancellation->throwIfRequested();
@@ -482,7 +488,11 @@ private function readResponse(
             throw $e;
         } catch (PromiseTimeoutException $e) {
             $this->close();
-            throw new TimeoutException('Inactivity timeout exceeded, more than ' . $timeout . ' ms elapsed from last data received', 0, $e);
+            throw new TimeoutException(
+                'Inactivity timeout exceeded, more than ' . $timeout . ' ms elapsed from last data received',
+                0,
+                $e
+            );
         } catch (\Throwable $e) {
             $this->close();
             throw new SocketException('Receiving the response headers failed: ' . $e->getMessage(), 0, $e);
@@ -692,18 +702,7 @@ private function writeRequest(
     private function generateRawHeader(Request $request, string $protocolVersion): string
     {
         $uri = $request->getUri();
-        $path = $uri->getPath();
-        if (($path[0] ?? '/') !== '/') {
-            throw new InvalidRequestException(
-                $request,
-                'Relative path (' . $path . ') is not allowed in the request URI: ' . $uri
-            );
-        }
-        $requestUri = $path ?: '/';
-
-        if ('' !== $query = $uri->getQuery()) {
-            $requestUri .= '?' . $query;
-        }
+        $requestUri = normalizeRequestPathWithQuery($request);
 
         $method = $request->getMethod();
 

diff --git a/src/Connection/Internal/Http2ConnectionProcessor.php b/src/Connection/Internal/Http2ConnectionProcessor.php
@@ -41,6 +41,7 @@
 use League\Uri;
 use function Amp\asyncCall;
 use function Amp\call;
+use function Amp\Http\Client\Internal\normalizeRequestPathWithQuery;
 
 /** @internal */
 final class Http2ConnectionProcessor implements Http2Processor
@@ -1543,28 +1544,14 @@ private function shutdown(?int $lastId = null, ?HttpException $reason = null): P
 
     /**
      * @param Request $request
+     *
      * @return array
      * @throws InvalidRequestException
      */
     private function generateHeaders(Request $request): array
     {
         $uri = $request->getUri();
-
-        $path = $uri->getPath();
-        if (($path[0] ?? '/') !== '/') {
-            throw new InvalidRequestException(
-                $request,
-                'Relative path (' . $path . ') is not allowed in the request URI: ' . $uri
-            );
-        }
-        if ($path === '') {
-            $path = '/';
-        }
-
-        $query = $uri->getQuery();
-        if ($query !== '') {
-            $path .= '?' . $query;
-        }
+        $path = normalizeRequestPathWithQuery($request);
 
         $authority = $uri->getHost();
         if ($port = $uri->getPort()) {

diff --git a/src/Internal/functions.php b/src/Internal/functions.php
@@ -0,0 +1,33 @@
+<?php
+
+namespace Amp\Http\Client\Internal;
+
+use Amp\Http\Client\InvalidRequestException;
+use Amp\Http\Client\Request;
+
+/**
+ * @param Request $request
+ *
+ * @return string
+ * @throws InvalidRequestException
+ *
+ * @internal
+ */
+function normalizeRequestPathWithQuery(Request $request): string
+{
+    $path = $request->getUri()->getPath();
+    $query = $request->getUri()->getQuery();
+
+    if ($path === '') {
+        return '/' . ($query !== '' ? '?' . $query : '');
+    }
+
+    if ($path[0] !== '/') {
+        throw new InvalidRequestException(
+            $request,
+            'Relative path (' . $path . ') is not allowed in the request URI'
+        );
+    }
+
+    return $path . ($query !== '' ? '?' . $query : '');
+}
diff --git a/test/Connection/Http1ConnectionTest.php b/test/Connection/Http1ConnectionTest.php
@@ -16,9 +16,8 @@
 use Amp\Promise;
 use Amp\Socket;
 use Amp\Success;
+use Laminas\Diactoros\Uri as LaminasUri;
 use League\Uri;
-use Psr\Http\Message\UriInterface;
-
 use function Amp\delay;
 
 class Http1ConnectionTest extends AsyncTestCase
@@ -106,7 +105,10 @@ public function testUpgrade(): \Generator
         $socketData = "Data that should be sent after the upgrade response";
 
         $invoked = false;
-        $callback = function (Socket\EncryptableSocket $socket, Request $request, Response $response) use (&$invoked, $socketData) {
+        $callback = function (Socket\EncryptableSocket $socket, Request $request, Response $response) use (
+            &$invoked,
+            $socketData
+        ) {
             $invoked = true;
             $this->assertSame(101, $response->getStatus());
             $this->assertSame($socketData, yield $socket->read());
@@ -203,38 +205,22 @@ public function testWritingRequestWithRelativeUriPathFails(): \Generator
         [$client] = Socket\createPair();
 
         $connection = new Http1Connection($client, 5000);
-        $uri = $this->createMock(UriInterface::class);
-        $uri->method('getScheme')->willReturn('http');
-        $uri->method('getAuthority')->willReturn('');
-        $uri->method('getUserInfo')->willReturn('');
-        $uri->method('getHost')->willReturn('localhost');
-        $uri->method('getQuery')->willReturn('');
-        $uri->method('getFragment')->willReturn('');
-        $uri->method('withScheme')->willReturnSelf();
-        $uri->method('withUserInfo')->willReturnSelf();
-        $uri->method('withHost')->willReturnSelf();
-        $uri->method('withPort')->willReturnSelf();
-        $uri->method('withQuery')->willReturnSelf();
-        $uri->method('withFragment')->willReturnSelf();
-        $uri->method('__toString')->willReturn('http://localhost/foo');
-
-        $uri
-            ->expects(self::never()) // ensure that path is left untouched
-            ->method('withPath')
-            ->willReturnSelf();
-        $uri->method('getPath')->willReturn('foo');
-        $request = new Request($uri);
+
+        $request = new Request(new LaminasUri('foo'));
+
         /** @var Stream $stream */
         $stream = yield $connection->getStream($request);
 
         $this->expectException(InvalidRequestException::class);
-        $this->expectExceptionMessage('Relative path (foo) is not allowed in the request URI: http://localhost/foo');
+        $this->expectExceptionMessage('Relative path (foo) is not allowed in the request URI');
+
         yield $stream->request($request, new NullCancellationToken);
     }
 
     /**
      * @param string $requestPath
      * @param string $expectedPath
+     *
      * @return \Generator
      * @throws Socket\SocketException
      * @dataProvider providerValidUriPaths

diff --git a/test/Connection/Http2ConnectionTest.php b/test/Connection/Http2ConnectionTest.php
@@ -17,9 +17,9 @@
 use Amp\Promise;
 use Amp\Socket;
 use Amp\TimeoutCancellationToken;
+use Laminas\Diactoros\Uri as LaminasUri;
 use League\Uri;
 
-use Psr\Http\Message\UriInterface;
 use function Amp\asyncCall;
 use function Amp\delay;
 use function Amp\Http\formatDateHeader;
@@ -427,34 +427,15 @@ public function testWritingRequestWithRelativeUriPathFails(): \Generator
         $server->write($frame = self::packFrame('', Http2Parser::SETTINGS, 0, 0));
         yield $connection->initialize();
 
-        $uri = $this->createMock(UriInterface::class);
-        $uri->method('getScheme')->willReturn('http');
-        $uri->method('getAuthority')->willReturn('');
-        $uri->method('getUserInfo')->willReturn('');
-        $uri->method('getHost')->willReturn('localhost');
-        $uri->method('getQuery')->willReturn('');
-        $uri->method('getFragment')->willReturn('');
-        $uri->method('withScheme')->willReturnSelf();
-        $uri->method('withUserInfo')->willReturnSelf();
-        $uri->method('withHost')->willReturnSelf();
-        $uri->method('withPort')->willReturnSelf();
-        $uri->method('withQuery')->willReturnSelf();
-        $uri->method('withFragment')->willReturnSelf();
-        $uri->method('__toString')->willReturn('http://localhost/foo');
-
-        $uri
-            ->expects(self::never()) // ensure that path is left untouched
-            ->method('withPath')
-            ->willReturnSelf();
-        $uri->method('getPath')->willReturn('foo');
-        $request = new Request($uri);
+        $request = new Request(new LaminasUri('foo'));
         $request->setInactivityTimeout(500);
 
         /** @var Stream $stream */
         $stream = yield $connection->getStream($request);
 
         $this->expectException(InvalidRequestException::class);
-        $this->expectExceptionMessage('Relative path (foo) is not allowed in the request URI: http://localhost/foo');
+        $this->expectExceptionMessage('Relative path (foo) is not allowed in the request URI');
+
         yield $stream->request($request, new NullCancellationToken());
     }