From bb1a190071125585e3a5bcedea1ba363df3763d2 Mon Sep 17 00:00:00 2001 From: Zach Borboa Date: Mon, 26 Jul 2021 00:48:18 -0400 Subject: [PATCH 1/5] Use strict types --- src/Curl/ArrayUtil.php | 2 +- src/Curl/CaseInsensitiveArray.php | 2 +- src/Curl/Curl.php | 2 +- src/Curl/Decoder.php | 2 +- src/Curl/Encoder.php | 2 +- src/Curl/MultiCurl.php | 2 +- src/Curl/StringUtil.php | 2 +- src/Curl/Url.php | 2 +- tests/PHPCurlClass/ArrayUtilTest.php | 2 +- tests/PHPCurlClass/ContentRangeServer.php | 2 +- tests/PHPCurlClass/Helper.php | 2 +- tests/PHPCurlClass/PHPCurlClassTest.php | 2 +- tests/PHPCurlClass/PHPMultiCurlClassTest.php | 2 +- tests/PHPCurlClass/RangeHeader.php | 2 +- tests/PHPCurlClass/UrlTest.php | 2 +- tests/PHPCurlClass/User.php | 2 +- tests/PHPCurlClass/server.php | 2 +- 17 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/Curl/ArrayUtil.php b/src/Curl/ArrayUtil.php index 61834de..00277b2 100644 --- a/src/Curl/ArrayUtil.php +++ b/src/Curl/ArrayUtil.php @@ -1,4 +1,4 @@ - Date: Mon, 26 Jul 2021 10:08:44 -0400 Subject: [PATCH 2/5] Fix some errors caused by using strict types --- src/Curl/Curl.php | 2 +- src/Curl/MultiCurl.php | 2 +- src/Curl/Url.php | 2 +- tests/PHPCurlClass/Helper.php | 2 +- tests/PHPCurlClass/PHPMultiCurlClassTest.php | 2 +- tests/PHPCurlClass/server.php | 4 ++-- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Curl/Curl.php b/src/Curl/Curl.php index 334dfd7..3fe29ec 100644 --- a/src/Curl/Curl.php +++ b/src/Curl/Curl.php @@ -1884,7 +1884,7 @@ class Curl */ private function parseHeaders($raw_headers) { - $raw_headers = preg_split('/\r\n/', $raw_headers, null, PREG_SPLIT_NO_EMPTY); + $raw_headers = preg_split('/\r\n/', (string) $raw_headers, -1, PREG_SPLIT_NO_EMPTY); $http_headers = new CaseInsensitiveArray(); $raw_headers_count = count($raw_headers); diff --git a/src/Curl/MultiCurl.php b/src/Curl/MultiCurl.php index 13b9f8c..ee0e42b 100755 --- a/src/Curl/MultiCurl.php +++ b/src/Curl/MultiCurl.php @@ -1270,7 +1270,7 @@ class MultiCurl $sleep_seconds = $sleep_until - microtime(true); // Avoid using time_sleep_until() as it appears to be less precise and not sleep long enough. - usleep($sleep_seconds * 1000000); + usleep((int) $sleep_seconds * 1000000); // Ensure that enough time has passed as usleep() may not have waited long enough. $this->currentStartTime = microtime(true); diff --git a/src/Curl/Url.php b/src/Curl/Url.php index d811fc9..227d21b 100644 --- a/src/Curl/Url.php +++ b/src/Curl/Url.php @@ -194,7 +194,7 @@ class Url // $7 = (query) // $8 = #Related (ignore) // $9 = Related (fragment) - preg_match('/^(([^:\/?#]+):)?(\/\/([^\/?#]*))?([^?#]*)(\?([^#]*))?(#(.*))?/', $url, $output_array); + preg_match('/^(([^:\/?#]+):)?(\/\/([^\/?#]*))?([^?#]*)(\?([^#]*))?(#(.*))?/', (string) $url, $output_array); $parts = []; if (isset($output_array['1']) && $output_array['1'] !== '') { diff --git a/tests/PHPCurlClass/Helper.php b/tests/PHPCurlClass/Helper.php index 79ded31..ac6eeaf 100644 --- a/tests/PHPCurlClass/Helper.php +++ b/tests/PHPCurlClass/Helper.php @@ -103,7 +103,7 @@ function get_tmp_file_path() // Return temporary file path without creating file. $tmp_file_path = rtrim(sys_get_temp_dir(), DIRECTORY_SEPARATOR) . - DIRECTORY_SEPARATOR . 'php-curl-class.' . uniqid(rand(), true); + DIRECTORY_SEPARATOR . 'php-curl-class.' . uniqid((string) rand(), true); return $tmp_file_path; } diff --git a/tests/PHPCurlClass/PHPMultiCurlClassTest.php b/tests/PHPCurlClass/PHPMultiCurlClassTest.php index 92bf42c..a43d4d2 100644 --- a/tests/PHPCurlClass/PHPMultiCurlClassTest.php +++ b/tests/PHPCurlClass/PHPMultiCurlClassTest.php @@ -2946,7 +2946,7 @@ class MultiCurlTest extends \PHPUnit\Framework\TestCase $urls = []; $copy_of_urls = []; for ($i = 0; $i < 10; $i++) { - $url = Test::TEST_URL . '?' . md5(mt_rand()); + $url = Test::TEST_URL . '?' . md5((string) mt_rand()); $urls[] = $url; $copy_of_urls[] = $url; } diff --git a/tests/PHPCurlClass/server.php b/tests/PHPCurlClass/server.php index 2f75d20..62a6ad9 100644 --- a/tests/PHPCurlClass/server.php +++ b/tests/PHPCurlClass/server.php @@ -264,7 +264,7 @@ if ($test === 'http_basic_auth') { exit; } elseif ($test === 'download_file_size') { $bytes = isset($_GET['bytes']) ? $_GET['bytes'] : 1234; - $str = str_repeat('.', $bytes); + $str = str_repeat('.', (int) $bytes); header('Content-Type: application/octet-stream'); header('Content-Length: ' . strlen($str)); header('ETag: ' . md5($str)); @@ -292,7 +292,7 @@ if ($test === 'http_basic_auth') { $dots_to_print = floor($elapsed) - $dots_printed; if ($dots_to_print) { - echo str_repeat('.', $dots_to_print); + echo str_repeat('.', (int) $dots_to_print); $dots_printed += $dots_to_print; } From 285b4f02e6f795b8949fa3baa9513aee783a1339 Mon Sep 17 00:00:00 2001 From: Zach Borboa Date: Mon, 26 Jul 2021 10:29:24 -0400 Subject: [PATCH 3/5] Update tests to more closely match one another --- tests/PHPCurlClass/PHPCurlClassTest.php | 3 +- tests/PHPCurlClass/PHPMultiCurlClassTest.php | 75 +++++++++----------- 2 files changed, 37 insertions(+), 41 deletions(-) diff --git a/tests/PHPCurlClass/PHPCurlClassTest.php b/tests/PHPCurlClass/PHPCurlClassTest.php index 5c994ca..132a2ab 100644 --- a/tests/PHPCurlClass/PHPCurlClassTest.php +++ b/tests/PHPCurlClass/PHPCurlClassTest.php @@ -643,7 +643,7 @@ class CurlTest extends \PHPUnit\Framework\TestCase $this->assertEquals('OPTIONS', $test->curl->responseHeaders['X-REQUEST-METHOD']); } - public function testDownload() + public function testDownloadToFile() { // Create and upload a file. $upload_file_path = \Helper\get_png(); @@ -705,6 +705,7 @@ class CurlTest extends \PHPUnit\Framework\TestCase unlink($upload_file_path); $this->assertFalse(file_exists($upload_file_path)); + $this->assertFalse(file_exists($upload_file_path)); } public function testDownloadRange() diff --git a/tests/PHPCurlClass/PHPMultiCurlClassTest.php b/tests/PHPCurlClass/PHPMultiCurlClassTest.php index a43d4d2..c302c92 100644 --- a/tests/PHPCurlClass/PHPMultiCurlClassTest.php +++ b/tests/PHPCurlClass/PHPMultiCurlClassTest.php @@ -2561,7 +2561,7 @@ class MultiCurlTest extends \PHPUnit\Framework\TestCase $multi_curl->start(); } - public function testDownload() + public function testDownloadToFile() { // Create and upload a file. $upload_file_path = \Helper\get_png(); @@ -2593,6 +2593,40 @@ class MultiCurlTest extends \PHPUnit\Framework\TestCase $this->assertFalse(file_exists($downloaded_file_path)); } + public function testDownloadCallback() + { + // Upload a file. + $upload_file_path = \Helper\get_png(); + $uploaded_file_path = \Helper\upload_file_to_server($upload_file_path); + + // Download the file. + $download_callback_called = false; + $multi_curl = new MultiCurl(); + $multi_curl->setHeader('X-DEBUG-TEST', 'download_response'); + $multi_curl->addDownload(Test::TEST_URL . '?' . http_build_query([ + 'file_path' => $uploaded_file_path, + ]), function ($instance, $fh) use (&$download_callback_called) { + \PHPUnit\Framework\Assert::assertFalse($download_callback_called); + \PHPUnit\Framework\Assert::assertInstanceOf('Curl\Curl', $instance); + \PHPUnit\Framework\Assert::assertTrue(is_resource($fh)); + \PHPUnit\Framework\Assert::assertEquals('stream', get_resource_type($fh)); + \PHPUnit\Framework\Assert::assertGreaterThan(0, strlen(stream_get_contents($fh))); + \PHPUnit\Framework\Assert::assertEquals(0, strlen(stream_get_contents($fh))); + \PHPUnit\Framework\Assert::assertTrue(fclose($fh)); + $download_callback_called = true; + }); + $multi_curl->start(); + $this->assertTrue($download_callback_called); + + // Remove server file. + \Helper\remove_file_from_server($uploaded_file_path); + + unlink($upload_file_path); + $this->assertFalse(file_exists($upload_file_path)); + $this->assertFalse(file_exists($uploaded_file_path)); + } + + public function testDownloadRange() { // Create and upload a file. @@ -2721,45 +2755,6 @@ class MultiCurlTest extends \PHPUnit\Framework\TestCase $multi_curl->start(); } - public function testDownloadCallback() - { - // Upload a file. - $upload_file_path = \Helper\get_png(); - $upload_test = new Test(); - $upload_test->server('upload_response', 'POST', [ - 'image' => '@' . $upload_file_path, - ]); - $uploaded_file_path = $upload_test->curl->response->file_path; - - // Download the file. - $download_callback_called = false; - $multi_curl = new MultiCurl(); - $multi_curl->setHeader('X-DEBUG-TEST', 'download_response'); - $multi_curl->addDownload(Test::TEST_URL . '?' . http_build_query([ - 'file_path' => $uploaded_file_path, - ]), function ($instance, $fh) use (&$download_callback_called) { - \PHPUnit\Framework\Assert::assertFalse($download_callback_called); - \PHPUnit\Framework\Assert::assertInstanceOf('Curl\Curl', $instance); - \PHPUnit\Framework\Assert::assertTrue(is_resource($fh)); - \PHPUnit\Framework\Assert::assertEquals('stream', get_resource_type($fh)); - \PHPUnit\Framework\Assert::assertGreaterThan(0, strlen(stream_get_contents($fh))); - \PHPUnit\Framework\Assert::assertEquals(0, strlen(stream_get_contents($fh))); - \PHPUnit\Framework\Assert::assertTrue(fclose($fh)); - $download_callback_called = true; - }); - $multi_curl->start(); - $this->assertTrue($download_callback_called); - - // Remove server file. - $this->assertEquals('true', $upload_test->server('upload_cleanup', 'POST', [ - 'file_path' => $uploaded_file_path, - ])); - - unlink($upload_file_path); - $this->assertFalse(file_exists($upload_file_path)); - $this->assertFalse(file_exists($uploaded_file_path)); - } - public function testDownloadCallbackError() { $download_before_send_called = false; From cd825550b21a4bf581441ad4fa3b4b50e48fb257 Mon Sep 17 00:00:00 2001 From: Zach Borboa Date: Mon, 26 Jul 2021 11:16:24 -0400 Subject: [PATCH 4/5] Fix Curl::downloadFileName not being correctly set --- src/Curl/Curl.php | 2 +- src/Curl/MultiCurl.php | 2 +- tests/PHPCurlClass/PHPCurlClassTest.php | 30 +++++++++++++++----- tests/PHPCurlClass/PHPMultiCurlClassTest.php | 13 +++++---- 4 files changed, 32 insertions(+), 15 deletions(-) diff --git a/src/Curl/Curl.php b/src/Curl/Curl.php index 3fe29ec..21dea01 100644 --- a/src/Curl/Curl.php +++ b/src/Curl/Curl.php @@ -1845,7 +1845,7 @@ class Curl */ private function downloadComplete($fh) { - if ($this->error && is_file($this->downloadFileName)) { + if ($this->error && is_file((string) $this->downloadFileName)) { @unlink($this->downloadFileName); } elseif (!$this->error && $this->downloadCompleteCallback) { rewind($fh); diff --git a/src/Curl/MultiCurl.php b/src/Curl/MultiCurl.php index ee0e42b..06d47e8 100755 --- a/src/Curl/MultiCurl.php +++ b/src/Curl/MultiCurl.php @@ -117,7 +117,7 @@ class MultiCurl // path. The download request will include header "Range: bytes=$filesize-" which is syntactically valid, // but unsatisfiable. $download_filename = $filename . '.pccdownload'; - $this->downloadFileName = $download_filename; + $curl->downloadFileName = $download_filename; // Attempt to resume download only when a temporary download file exists and is not empty. if (is_file($download_filename) && $filesize = filesize($download_filename)) { diff --git a/tests/PHPCurlClass/PHPCurlClassTest.php b/tests/PHPCurlClass/PHPCurlClassTest.php index 132a2ab..33d5a8f 100644 --- a/tests/PHPCurlClass/PHPCurlClassTest.php +++ b/tests/PHPCurlClass/PHPCurlClassTest.php @@ -661,6 +661,7 @@ class CurlTest extends \PHPUnit\Framework\TestCase $this->assertEquals(filesize($upload_file_path), filesize($downloaded_file_path)); $this->assertEquals(md5_file($upload_file_path), md5_file($downloaded_file_path)); $this->assertEquals(md5_file($upload_file_path), $download_test->curl->responseHeaders['ETag']); + $this->assertEquals($download_test->curl->downloadFileName, $downloaded_file_path . '.pccdownload'); // Ensure successive requests set the appropriate values. $this->assertEquals('GET', $download_test->server('request_method', 'GET')); @@ -683,29 +684,28 @@ class CurlTest extends \PHPUnit\Framework\TestCase $uploaded_file_path = \Helper\upload_file_to_server($upload_file_path); // Download the file. - $callback_called = false; + $download_callback_called = false; $curl = new Curl(); $curl->setHeader('X-DEBUG-TEST', 'download_response'); $curl->download(Test::TEST_URL . '?' . http_build_query([ 'file_path' => $uploaded_file_path, - ]), function ($instance, $fh) use (&$callback_called) { - \PHPUnit\Framework\Assert::assertFalse($callback_called); + ]), function ($instance, $fh) use (&$download_callback_called) { + \PHPUnit\Framework\Assert::assertFalse($download_callback_called); \PHPUnit\Framework\Assert::assertInstanceOf('Curl\Curl', $instance); \PHPUnit\Framework\Assert::assertTrue(is_resource($fh)); \PHPUnit\Framework\Assert::assertEquals('stream', get_resource_type($fh)); \PHPUnit\Framework\Assert::assertGreaterThan(0, strlen(stream_get_contents($fh))); \PHPUnit\Framework\Assert::assertEquals(0, strlen(stream_get_contents($fh))); \PHPUnit\Framework\Assert::assertTrue(fclose($fh)); - $callback_called = true; + $download_callback_called = true; }); - $this->assertTrue($callback_called); + $this->assertTrue($download_callback_called); // Remove server file. \Helper\remove_file_from_server($uploaded_file_path); unlink($upload_file_path); $this->assertFalse(file_exists($upload_file_path)); - $this->assertFalse(file_exists($upload_file_path)); } public function testDownloadRange() @@ -825,10 +825,26 @@ class CurlTest extends \PHPUnit\Framework\TestCase $test->curl->setHeader('X-DEBUG-TEST', '404'); $test->curl->download(Test::TEST_URL, $destination); - $this->assertFalse(file_exists($test->curl->getDownloadFileName())); + $this->assertFalse(file_exists($test->curl->downloadFileName)); $this->assertFalse(file_exists($destination)); } + public function testDownloadCallbackError() + { + $download_before_send_called = false; + $download_callback_called = false; + $curl = new Curl(); + $curl->beforeSend(function ($instance) use (&$download_before_send_called) { + \PHPUnit\Framework\Assert::assertFalse($download_before_send_called); + $download_before_send_called = true; + }); + $curl->download(Test::ERROR_URL, function ($instance, $fh) use (&$download_callback_called) { + $download_callback_called = true; + }); + $this->assertTrue($download_before_send_called); + $this->assertFalse($download_callback_called); + } + public function testMaxFilesize() { $tests = [ diff --git a/tests/PHPCurlClass/PHPMultiCurlClassTest.php b/tests/PHPCurlClass/PHPMultiCurlClassTest.php index c302c92..6811334 100644 --- a/tests/PHPCurlClass/PHPMultiCurlClassTest.php +++ b/tests/PHPCurlClass/PHPMultiCurlClassTest.php @@ -2574,9 +2574,12 @@ class MultiCurlTest extends \PHPUnit\Framework\TestCase $multi_curl->addDownload(Test::TEST_URL . '?' . http_build_query([ 'file_path' => $uploaded_file_path, ]), $downloaded_file_path); - $multi_curl->complete(function ($instance) use ($upload_file_path) { - \PHPUnit\Framework\Assert::assertFalse($instance->error); + $multi_curl->complete(function ($instance) use ($upload_file_path, $downloaded_file_path) { \PHPUnit\Framework\Assert::assertEquals(md5_file($upload_file_path), $instance->responseHeaders['ETag']); + \PHPUnit\Framework\Assert::assertEquals( + $instance->downloadFileName, + $downloaded_file_path . '.pccdownload' + ); }); $multi_curl->start(); $this->assertNotEquals($uploaded_file_path, $downloaded_file_path); @@ -2595,7 +2598,7 @@ class MultiCurlTest extends \PHPUnit\Framework\TestCase public function testDownloadCallback() { - // Upload a file. + // Create and upload a file. $upload_file_path = \Helper\get_png(); $uploaded_file_path = \Helper\upload_file_to_server($upload_file_path); @@ -2623,10 +2626,8 @@ class MultiCurlTest extends \PHPUnit\Framework\TestCase unlink($upload_file_path); $this->assertFalse(file_exists($upload_file_path)); - $this->assertFalse(file_exists($uploaded_file_path)); } - public function testDownloadRange() { // Create and upload a file. @@ -2749,7 +2750,7 @@ class MultiCurlTest extends \PHPUnit\Framework\TestCase $multi_curl->setHeader('X-DEBUG-TEST', '404'); $multi_curl->addDownload(Test::TEST_URL, $destination); $multi_curl->complete(function ($instance) use ($destination) { - \PHPUnit\Framework\Assert::assertFalse(file_exists($instance->getDownloadFileName())); + \PHPUnit\Framework\Assert::assertFalse(file_exists($instance->downloadFileName)); \PHPUnit\Framework\Assert::assertFalse(file_exists($destination)); }); $multi_curl->start(); From c51b1757488f2191a41043378018fd1810ba0a66 Mon Sep 17 00:00:00 2001 From: Zach Borboa Date: Mon, 26 Jul 2021 20:44:33 -0400 Subject: [PATCH 5/5] Increase amount of time drift allowed (noisy neighbors?) --- tests/PHPCurlClass/PHPMultiCurlClassTest.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/PHPCurlClass/PHPMultiCurlClassTest.php b/tests/PHPCurlClass/PHPMultiCurlClassTest.php index 6811334..82c9fc3 100644 --- a/tests/PHPCurlClass/PHPMultiCurlClassTest.php +++ b/tests/PHPCurlClass/PHPMultiCurlClassTest.php @@ -3774,7 +3774,7 @@ class MultiCurlTest extends \PHPUnit\Framework\TestCase $this->assertLessThanOrEqual(10.5, $request_stats['4']['relative_start']); // Assert R4 ends around 11. $this->assertGreaterThanOrEqual(10.8, $request_stats['4']['relative_stop']); - $this->assertLessThanOrEqual(11.5, $request_stats['4']['relative_stop']); + $this->assertLessThanOrEqual(11.5 + 1, $request_stats['4']['relative_stop']); } public function testSetRateLimitPerSecond2() @@ -3843,7 +3843,7 @@ class MultiCurlTest extends \PHPUnit\Framework\TestCase $this->assertLessThanOrEqual(10.5, $request_stats['4']['relative_start']); // Assert R4 ends around 11. $this->assertGreaterThanOrEqual(10.8, $request_stats['4']['relative_stop']); - $this->assertLessThanOrEqual(11.5, $request_stats['4']['relative_stop']); + $this->assertLessThanOrEqual(11.5 + 1, $request_stats['4']['relative_stop']); } public function testSetRateLimitPerSecond3() @@ -4045,7 +4045,7 @@ class MultiCurlTest extends \PHPUnit\Framework\TestCase $this->assertLessThanOrEqual(10.5, $request_stats['4']['relative_start']); // Assert R4 ends around 12. $this->assertGreaterThanOrEqual(11.8, $request_stats['4']['relative_stop']); - $this->assertLessThanOrEqual(12.5, $request_stats['4']['relative_stop']); + $this->assertLessThanOrEqual(12.5 + 1, $request_stats['4']['relative_stop']); } public function testSetRateLimitPerSecond6() @@ -4113,7 +4113,7 @@ class MultiCurlTest extends \PHPUnit\Framework\TestCase $this->assertLessThanOrEqual(10.5, $request_stats['4']['relative_start']); // Assert R4 ends around 12. $this->assertGreaterThanOrEqual(11.8, $request_stats['4']['relative_stop']); - $this->assertLessThanOrEqual(12.5, $request_stats['4']['relative_stop']); + $this->assertLessThanOrEqual(12.5 + 1, $request_stats['4']['relative_stop']); } public function testSetRateLimitPerSecond7()