From 3ae1c6c8d016e6b25bd97f486a0c19e01395843b Mon Sep 17 00:00:00 2001 From: Niklas Keller Date: Sun, 10 May 2020 21:01:10 +0200 Subject: [PATCH] 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. --- composer.json | 10 ++++-- src/Connection/Http1Connection.php | 29 ++++++++------- .../Internal/Http2ConnectionProcessor.php | 19 ++-------- src/Internal/functions.php | 33 +++++++++++++++++ test/Connection/Http1ConnectionTest.php | 36 ++++++------------- test/Connection/Http2ConnectionTest.php | 27 +++----------- 6 files changed, 72 insertions(+), 82 deletions(-) create mode 100644 src/Internal/functions.php diff --git a/composer.json b/composer.json index 264448a9..687524fd 100644 --- 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 index fc808aa1..bbbaea0f 100644 --- 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 index 66f55030..32f0a0f8 100644 --- 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 new file mode 100644 index 00000000..db2ed858 --- /dev/null +++ b/src/Internal/functions.php @@ -0,0 +1,33 @@ +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 index 1b5577b4..262f7c7b 100644 --- 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 index 17dfe4fa..85f2037e 100644 --- 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()); }