From 794a967536b6b36176355004aba25fd12481f609 Mon Sep 17 00:00:00 2001 From: Phil Davis Date: Sat, 11 Feb 2023 13:20:42 +0545 Subject: [PATCH 01/10] Add phpstan-phpunit phpstan-strict-rules to phpstan analysis --- composer.json | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/composer.json b/composer.json index f33898f..f4cc383 100644 --- a/composer.json +++ b/composer.json @@ -15,6 +15,9 @@ "require-dev" : { "friendsofphp/php-cs-fixer": "^3.14", "phpstan/phpstan": "^1.9", + "phpstan/phpstan-phpunit": "^1.3", + "phpstan/phpstan-strict-rules": "^1.4", + "phpstan/extension-installer": "^1.2", "phpunit/phpunit" : "^9.6" }, "suggest" : { @@ -60,5 +63,10 @@ "composer cs-fixer", "composer phpunit" ] + }, + "config": { + "allow-plugins": { + "phpstan/extension-installer": true + } } } From f777afc237e3ee86a35cfc7ce3ee15ecdf8c36ce Mon Sep 17 00:00:00 2001 From: Phil Davis Date: Sat, 11 Feb 2023 13:21:35 +0545 Subject: [PATCH 02/10] Fixup test code for phpstan --- phpstan.neon | 3 +++ tests/HTTP/Auth/AWSTest.php | 8 ++++---- tests/HTTP/ClientTest.php | 18 +++++++++--------- tests/HTTP/SapiTest.php | 4 ++-- 4 files changed, 18 insertions(+), 15 deletions(-) diff --git a/phpstan.neon b/phpstan.neon index 5802f6f..f97fe7a 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -18,3 +18,6 @@ parameters: message: "#^Strict comparison using === between null and array will always evaluate to false.$#" count: 1 path: lib/functions.php + - + message: "#^.* will always evaluate to true\\.$#" + path: tests/* diff --git a/tests/HTTP/Auth/AWSTest.php b/tests/HTTP/Auth/AWSTest.php index 4d49a18..2f1c1a9 100644 --- a/tests/HTTP/Auth/AWSTest.php +++ b/tests/HTTP/Auth/AWSTest.php @@ -90,7 +90,7 @@ public function testFutureDate(): void $contentMD5 = base64_encode(md5($content, true)); $date = new \DateTime('@'.(time() + (60 * 20))); - $date->setTimeZone(new \DateTimeZone('GMT')); + $date->setTimezone(new \DateTimeZone('GMT')); $date = $date->format('D, d M Y H:i:s \\G\\M\\T'); $this->request->setMethod('POST'); @@ -117,7 +117,7 @@ public function testPastDate(): void $contentMD5 = base64_encode(md5($content, true)); $date = new \DateTime('@'.(time() - (60 * 20))); - $date->setTimeZone(new \DateTimeZone('GMT')); + $date->setTimezone(new \DateTimeZone('GMT')); $date = $date->format('D, d M Y H:i:s \\G\\M\\T'); $this->request->setMethod('POST'); @@ -145,7 +145,7 @@ public function testIncorrectSignature(): void $contentMD5 = base64_encode(md5($content, true)); $date = new \DateTime('now'); - $date->setTimeZone(new \DateTimeZone('GMT')); + $date->setTimezone(new \DateTimeZone('GMT')); $date = $date->format('D, d M Y H:i:s \\G\\M\\T'); $this->request->setUrl('/'); @@ -172,7 +172,7 @@ public function testValidRequest(): void $contentMD5 = base64_encode(md5($content, true)); $date = new \DateTime('now'); - $date->setTimeZone(new \DateTimeZone('GMT')); + $date->setTimezone(new \DateTimeZone('GMT')); $date = $date->format('D, d M Y H:i:s \\G\\M\\T'); $sig = base64_encode($this->hmacsha1($secretKey, diff --git a/tests/HTTP/ClientTest.php b/tests/HTTP/ClientTest.php index 58859e5..cb8cefa 100644 --- a/tests/HTTP/ClientTest.php +++ b/tests/HTTP/ClientTest.php @@ -209,7 +209,7 @@ public function testSendToGetLargeContent(): void { $url = $this->getAbsoluteUrl('/large.php'); if (!$url) { - $this->markTestSkipped('Set an environment value BASEURL to continue'); + self::markTestSkipped('Set an environment value BASEURL to continue'); } // Allow the peak memory usage limit to be specified externally, if needed. @@ -239,7 +239,7 @@ public function testSendAsync(): void { $url = $this->getAbsoluteUrl('/foo'); if (!$url) { - $this->markTestSkipped('Set an environment value BASEURL to continue'); + self::markTestSkipped('Set an environment value BASEURL to continue'); } $client = new Client(); @@ -251,7 +251,7 @@ public function testSendAsync(): void self::assertEquals(4, $response->getHeader('Content-Length')); }, function ($error) use ($request) { $url = $request->getUrl(); - $this->fail("Failed to GET $url"); + self::fail("Failed to GET $url"); }); $client->wait(); @@ -264,7 +264,7 @@ public function testSendAsynConsecutively(): void { $url = $this->getAbsoluteUrl('/foo'); if (!$url) { - $this->markTestSkipped('Set an environment value BASEURL to continue'); + self::markTestSkipped('Set an environment value BASEURL to continue'); } $client = new Client(); @@ -276,7 +276,7 @@ public function testSendAsynConsecutively(): void self::assertEquals(4, $response->getHeader('Content-Length')); }, function ($error) use ($request) { $url = $request->getUrl(); - $this->fail("Failed to get $url"); + self::fail("Failed to get $url"); }); $url = $this->getAbsoluteUrl('/bar.php'); @@ -287,7 +287,7 @@ public function testSendAsynConsecutively(): void self::assertEquals('Bar', $response->getHeader('X-Test')); }, function ($error) use ($request) { $url = $request->getUrl(); - $this->fail("Failed to get $url"); + self::fail("Failed to get $url"); }); $client->wait(); @@ -308,7 +308,7 @@ public function testSendClientError(): void try { $client->send($request); - $this->fail('send() should have thrown an exception'); + self::fail('send() should have thrown an exception'); } catch (ClientException $e) { } self::assertTrue($called); @@ -373,7 +373,7 @@ public function testHttpErrorException(): void try { $client->send($request); - $this->fail('An exception should have been thrown'); + self::fail('An exception should have been thrown'); } catch (ClientHttpException $e) { self::assertEquals(404, $e->getHttpStatus()); self::assertInstanceOf('Sabre\HTTP\Response', $e->getResponse()); @@ -490,7 +490,7 @@ public function testDoRequestCurlError(): void try { $response = $client->doRequest($request); - $this->fail('This should have thrown an exception'); + self::fail('This should have thrown an exception'); } catch (ClientException $e) { self::assertEquals(1, $e->getCode()); self::assertEquals('Curl error', $e->getMessage()); diff --git a/tests/HTTP/SapiTest.php b/tests/HTTP/SapiTest.php index 1deb941..1dbec85 100644 --- a/tests/HTTP/SapiTest.php +++ b/tests/HTTP/SapiTest.php @@ -114,7 +114,7 @@ public function testConstructRedirectAuth(): void public function testSend(): void { if (!function_exists('xdebug_get_headers')) { - $this->markTestSkipped('XDebug needs to be installed for this test to run'); + self::markTestSkipped('XDebug needs to be installed for this test to run'); } $response = new Response(204, ['Content-Type' => 'text/xml;charset=UTF-8']); @@ -221,7 +221,7 @@ public function testSendContentRangeStream( $ignoreAtStartLength = strlen($ignoreAtStart); $ignoreAtEndLength = strlen($ignoreAtEnd); $body = fopen('php://memory', 'w'); - if (!$contentLength) { + if (null === $contentLength) { $contentLength = strlen($partial); } fwrite($body, $ignoreAtStart); From 1294a66e8976a6adbef6859dbf76df82f82ecb27 Mon Sep 17 00:00:00 2001 From: Phil Davis Date: Sat, 11 Feb 2023 14:29:33 +0545 Subject: [PATCH 03/10] Adjust code for things reported by phpstan --- lib/Auth/AWS.php | 6 ++++-- lib/Auth/Basic.php | 2 +- lib/Auth/Bearer.php | 2 +- lib/Auth/Digest.php | 8 ++++---- lib/Client.php | 23 +++++++++++++---------- lib/Request.php | 2 +- lib/Response.php | 2 +- lib/Sapi.php | 2 +- lib/functions.php | 6 +++--- phpstan.neon | 4 ++++ 10 files changed, 33 insertions(+), 24 deletions(-) diff --git a/lib/Auth/AWS.php b/lib/Auth/AWS.php index 02df933..a23c3ae 100644 --- a/lib/Auth/AWS.php +++ b/lib/Auth/AWS.php @@ -84,7 +84,7 @@ public function validate(string $secretKey): bool { $contentMD5 = $this->request->getHeader('Content-MD5'); - if ($contentMD5) { + if (null !== $contentMD5) { // We need to validate the integrity of the request $body = $this->request->getBody(); $this->request->setBody($body); @@ -97,7 +97,9 @@ public function validate(string $secretKey): bool } } - if (!$requestDate = $this->request->getHeader('x-amz-date')) { + $requestDate = $this->request->getHeader('x-amz-date'); + + if (null === $requestDate) { $requestDate = $this->request->getHeader('Date'); } diff --git a/lib/Auth/Basic.php b/lib/Auth/Basic.php index 0962470..f50afa7 100644 --- a/lib/Auth/Basic.php +++ b/lib/Auth/Basic.php @@ -31,7 +31,7 @@ public function getCredentials(): ?array { $auth = $this->request->getHeader('Authorization'); - if (!$auth) { + if (null === $auth) { return null; } diff --git a/lib/Auth/Bearer.php b/lib/Auth/Bearer.php index 476dcf2..6f34876 100644 --- a/lib/Auth/Bearer.php +++ b/lib/Auth/Bearer.php @@ -28,7 +28,7 @@ public function getToken(): ?string { $auth = $this->request->getHeader('Authorization'); - if (!$auth) { + if (null === $auth) { return null; } diff --git a/lib/Auth/Digest.php b/lib/Auth/Digest.php index 2e35eb9..e54fad0 100644 --- a/lib/Auth/Digest.php +++ b/lib/Auth/Digest.php @@ -130,14 +130,14 @@ protected function validate(): bool if ('auth-int' === $this->digestParts['qop']) { // Making sure we support this qop value - if (!($this->qop & self::QOP_AUTHINT)) { + if (0 === ($this->qop & self::QOP_AUTHINT)) { return false; } // We need to add an MD5 of the entire request body to the A2 part of the hash $body = $this->request->getBody(); $this->request->setBody($body); $A2 .= ':'.md5($body); - } elseif (!($this->qop & self::QOP_AUTH)) { + } elseif (0 === ($this->qop & self::QOP_AUTH)) { return false; } @@ -200,10 +200,10 @@ protected function parseDigest(string $digest) preg_match_all('@(\w+)=(?:(?:")([^"]+)"|([^\s,$]+))@', $digest, $matches, PREG_SET_ORDER); foreach ($matches as $m) { - $data[$m[1]] = $m[2] ?: $m[3]; + $data[$m[1]] = ('' !== $m[2]) ? $m[2] : $m[3]; unset($needed_parts[$m[1]]); } - return $needed_parts ? false : $data; + return (count($needed_parts) > 0) ? false : $data; } } diff --git a/lib/Client.php b/lib/Client.php index f323237..00634aa 100644 --- a/lib/Client.php +++ b/lib/Client.php @@ -194,7 +194,7 @@ public function sendAsync(RequestInterface $request, callable $success = null, c public function poll(): bool { // nothing to do? - if (!$this->curlMultiMap) { + if (0 === count($this->curlMultiMap)) { return false; } @@ -239,7 +239,7 @@ public function poll(): bool $curlResult['request'] = $request; - if ($errorCallback) { + if (is_callable($errorCallback)) { $errorCallback($curlResult); } } elseif (self::STATUS_HTTPERROR === $curlResult['status']) { @@ -254,13 +254,13 @@ public function poll(): bool $curlResult['request'] = $request; - if ($errorCallback) { + if (is_callable($errorCallback)) { $errorCallback($curlResult); } } else { $this->emit('afterRequest', [$request, $curlResult['response']]); - if ($successCallback) { + if (is_callable($successCallback)) { $successCallback($curlResult['response']); } } @@ -314,7 +314,7 @@ protected function doRequest(RequestInterface $request): ResponseInterface { $settings = $this->createCurlSettingsArray($request); - if (!$this->curlHandle) { + if (null === $this->curlHandle) { $this->curlHandle = curl_init(); } else { curl_reset($this->curlHandle); @@ -475,7 +475,7 @@ protected function parseCurlResponse(array $headerLines, string $body, $curlHand $curlErrMsg ) = $this->curlStuff($curlHandle); - if ($curlErrNo) { + if (0 !== $curlErrNo) { return [ 'status' => self::STATUS_CURLERROR, 'curl_errno' => $curlErrNo, @@ -532,7 +532,7 @@ protected function parseCurlResult(string $response, $curlHandle): array $curlErrMsg ) = $this->curlStuff($curlHandle); - if ($curlErrNo) { + if (0 !== $curlErrNo) { return [ 'status' => self::STATUS_CURLERROR, 'curl_errno' => $curlErrNo, @@ -541,10 +541,13 @@ protected function parseCurlResult(string $response, $curlHandle): array } $headerBlob = substr($response, 0, $curlInfo['header_size']); - // In the case of 204 No Content, strlen($response) == $curlInfo['header_size]. + // In the case of 204 No Content, strlen($response) == $curlInfo['header_size']. // This will cause substr($response, $curlInfo['header_size']) return FALSE instead of NULL // An exception will be thrown when calling getBodyAsString then - $responseBody = substr($response, $curlInfo['header_size']) ?: ''; + $responseBody = substr($response, $curlInfo['header_size']); + if (false === $responseBody) { + $responseBody = ''; + } unset($response); @@ -570,7 +573,7 @@ protected function parseCurlResult(string $response, $curlHandle): array */ protected function sendAsyncInternal(RequestInterface $request, callable $success, callable $error, int $retryCount = 0): void { - if (!$this->curlMultiHandle) { + if (null === $this->curlMultiHandle) { $this->curlMultiHandle = curl_multi_init(); } $curl = curl_init(); diff --git a/lib/Request.php b/lib/Request.php index a05dbcd..d289d6e 100644 --- a/lib/Request.php +++ b/lib/Request.php @@ -108,7 +108,7 @@ public function setAbsoluteUrl(string $url): void */ public function getAbsoluteUrl(): string { - if (!$this->absoluteUrl) { + if ((null === $this->absoluteUrl) || ('' === $this->absoluteUrl)) { // Guessing we're a http endpoint. $this->absoluteUrl = 'http://'. ($this->getHeader('Host') ?? 'localhost'). diff --git a/lib/Response.php b/lib/Response.php index 19ac87b..f73c643 100644 --- a/lib/Response.php +++ b/lib/Response.php @@ -97,7 +97,7 @@ class Response extends Message implements ResponseInterface /** * Creates the response object. * - * @param string|int $status + * @param string|int|null $status * @param array|null $headers * @param resource|string|callable|null $body */ diff --git a/lib/Sapi.php b/lib/Sapi.php index f078e3a..4383fda 100644 --- a/lib/Sapi.php +++ b/lib/Sapi.php @@ -201,7 +201,7 @@ public static function createFromServerArray(array $serverArray): Request break; case 'HTTPS': - if (!empty($value) && 'off' !== $value) { + if ('' !== $value && 'off' !== $value) { $protocol = 'https'; } break; diff --git a/lib/functions.php b/lib/functions.php index 7ecb7ad..5ee4235 100644 --- a/lib/functions.php +++ b/lib/functions.php @@ -103,7 +103,7 @@ function toDate(\DateTime $dateTime): string */ function negotiateContentType(?string $acceptHeaderValue, array $availableOptions): ?string { - if (!$acceptHeaderValue) { + if ((null === $acceptHeaderValue) || ('' === $acceptHeaderValue)) { // Grabbing the first in the list. return reset($availableOptions); } @@ -261,7 +261,7 @@ function parsePrefer($input): array } else { $value = true; } - $output[strtolower($matches['name'])] = empty($value) ? true : $value; + $output[strtolower($matches['name'])] = ('' === $value) ? true : $value; break; } } @@ -291,7 +291,7 @@ function parsePrefer($input): array function getHeaderValues($values, $values2 = null): array { $values = (array) $values; - if ($values2) { + if (null !== $values2) { $values = array_merge($values, (array) $values2); } diff --git a/phpstan.neon b/phpstan.neon index f97fe7a..16a2fff 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -14,6 +14,10 @@ parameters: message: "#^Left side of || is always false.$#" count: 2 path: lib/Client.php + - + message: "#^Casting to string something that's already string.$#" + count: 1 + path: lib/Sapi.php - message: "#^Strict comparison using === between null and array will always evaluate to false.$#" count: 1 From 67489563dbce027476faf72f66205d81e621bce4 Mon Sep 17 00:00:00 2001 From: Phil Davis Date: Sat, 11 Feb 2023 15:45:44 +0545 Subject: [PATCH 04/10] Use strict mode for in_array --- lib/Client.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Client.php b/lib/Client.php index 00634aa..5c36f57 100644 --- a/lib/Client.php +++ b/lib/Client.php @@ -122,7 +122,7 @@ public function send(RequestInterface $request): ResponseInterface // open_basedir. // // https://github.com/fruux/sabre-http/issues/12 - if ($redirects < $this->maxRedirects && in_array($code, [301, 302, 307, 308])) { + if ($redirects < $this->maxRedirects && in_array($code, [301, 302, 307, 308], true)) { $oldLocation = $request->getUrl(); // Creating a new instance of the request object. From d0850a5adaffd6f5e3565833775ebe8daff954cb Mon Sep 17 00:00:00 2001 From: Phil Davis Date: Sat, 11 Feb 2023 15:50:30 +0545 Subject: [PATCH 05/10] Use strict mode for base64_decode --- lib/Auth/Basic.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/Auth/Basic.php b/lib/Auth/Basic.php index f50afa7..16af088 100644 --- a/lib/Auth/Basic.php +++ b/lib/Auth/Basic.php @@ -39,7 +39,13 @@ public function getCredentials(): ?array return null; } - $credentials = explode(':', base64_decode(substr($auth, 6)), 2); + $decodedAuth = base64_decode(substr($auth, 6), true); + + if (false === $decodedAuth) { + return null; + } + + $credentials = explode(':', $decodedAuth, 2); if (2 !== count($credentials)) { return null; From e271735526d52f19a1ebc48415f8d58258b55342 Mon Sep 17 00:00:00 2001 From: Phil Davis Date: Sat, 11 Feb 2023 15:57:57 +0545 Subject: [PATCH 06/10] Ignore Call to function array_key_exists() will always evaluate to true --- phpstan.neon | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/phpstan.neon b/phpstan.neon index 16a2fff..8488fb9 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -14,6 +14,10 @@ parameters: message: "#^Left side of || is always false.$#" count: 2 path: lib/Client.php + - + message: "#^.* will always evaluate to true\\.$#" + count: 1 + path: lib/Client.php - message: "#^Casting to string something that's already string.$#" count: 1 From 3a298fa6096657bf8fb5fd3a0eb3bcb9bcee1a07 Mon Sep 17 00:00:00 2001 From: Phil Davis Date: Tue, 27 Jun 2023 14:16:52 +0545 Subject: [PATCH 07/10] Adjust to get phpstan to pass --- phpstan.neon | 6 +----- tests/HTTP/SapiTest.php | 6 +++--- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/phpstan.neon b/phpstan.neon index 8488fb9..3b9bd27 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -12,11 +12,7 @@ parameters: path: lib/Client.php - message: "#^Left side of || is always false.$#" - count: 2 - path: lib/Client.php - - - message: "#^.* will always evaluate to true\\.$#" - count: 1 + count: 3 path: lib/Client.php - message: "#^Casting to string something that's already string.$#" diff --git a/tests/HTTP/SapiTest.php b/tests/HTTP/SapiTest.php index 1dbec85..9c6026c 100644 --- a/tests/HTTP/SapiTest.php +++ b/tests/HTTP/SapiTest.php @@ -305,7 +305,7 @@ public function testSendConnectionAborted(): void { $baseUrl = getenv('BASEURL'); if (!$baseUrl) { - $this->markTestSkipped('Set an environment value BASEURL to continue'); + self::markTestSkipped('Set an environment value BASEURL to continue'); } $url = rtrim($baseUrl, '/').'/connection_aborted.php'; @@ -328,7 +328,7 @@ public function testSendConnectionAborted(): void sleep(5); $bytes_read = file_get_contents(sys_get_temp_dir().'/dummy_stream_read_counter'); - $this->assertEquals($chunk_size * 2, $bytes_read); - $this->assertGreaterThanOrEqual($fetch_size, $bytes_read); + self::assertEquals($chunk_size * 2, $bytes_read); + self::assertGreaterThanOrEqual($fetch_size, $bytes_read); } } From 8d0dd5a0ce2709b673e3c2159237933ef05a4f56 Mon Sep 17 00:00:00 2001 From: Phil Davis Date: Tue, 27 Jun 2023 14:58:37 +0545 Subject: [PATCH 08/10] Declare types of vars coming from curlMultiMap array --- lib/Client.php | 7 +++++++ phpstan.neon | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/Client.php b/lib/Client.php index 5c36f57..e2d80a8 100644 --- a/lib/Client.php +++ b/lib/Client.php @@ -216,6 +216,13 @@ public function poll(): bool if ($status && CURLMSG_DONE === $status['msg']) { $resourceId = (int) $status['handle']; + + /** + * @var RequestInterface $request + * @var callable $successCallback + * @var callable $errorCallback + * @var int $retryCount + */ list( $request, $successCallback, diff --git a/phpstan.neon b/phpstan.neon index 3b9bd27..7eff5cc 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -12,7 +12,7 @@ parameters: path: lib/Client.php - message: "#^Left side of || is always false.$#" - count: 3 + count: 6 path: lib/Client.php - message: "#^Casting to string something that's already string.$#" From f4474d2b9d0808adb5d013a5aa53aa0b7b7ffcbd Mon Sep 17 00:00:00 2001 From: Phil Davis Date: Tue, 27 Jun 2023 14:59:36 +0545 Subject: [PATCH 09/10] Add count to ignoreErrors entry for tests --- phpstan.neon | 1 + 1 file changed, 1 insertion(+) diff --git a/phpstan.neon b/phpstan.neon index 7eff5cc..2d08f5f 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -24,4 +24,5 @@ parameters: path: lib/functions.php - message: "#^.* will always evaluate to true\\.$#" + count: 4 path: tests/* From dbc86e433508362355d3b00209c15a9b198b235a Mon Sep 17 00:00:00 2001 From: Phil Davis Date: Tue, 27 Jun 2023 15:50:56 +0545 Subject: [PATCH 10/10] Add array shape PHPdoc for curlMultiMap --- lib/Client.php | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/lib/Client.php b/lib/Client.php index e2d80a8..4c7e2b2 100644 --- a/lib/Client.php +++ b/lib/Client.php @@ -217,12 +217,6 @@ public function poll(): bool if ($status && CURLMSG_DONE === $status['msg']) { $resourceId = (int) $status['handle']; - /** - * @var RequestInterface $request - * @var callable $successCallback - * @var callable $errorCallback - * @var int $retryCount - */ list( $request, $successCallback, @@ -360,7 +354,7 @@ protected function doRequest(RequestInterface $request): ResponseInterface * Has a list of curl handles, as well as their associated success and * error callbacks. * - * @var array + * @var array */ private array $curlMultiMap = [];