From 1d620c51145bab9a37412828d881bb79777401d5 Mon Sep 17 00:00:00 2001 From: Sammyjo20 <29132017+Sammyjo20@users.noreply.github.com> Date: Mon, 12 Feb 2024 11:38:11 +0000 Subject: [PATCH 1/5] Fix | Better Query Parameter Parsing --- .phpunit.cache/test-results | 1 + src/Helpers/URLHelper.php | 24 +++++++++++++++++++ .../PendingRequest/ManagesPsrRequests.php | 3 ++- tests/Unit/PsrTest.php | 11 +++++++++ 4 files changed, 38 insertions(+), 1 deletion(-) create mode 100644 .phpunit.cache/test-results diff --git a/.phpunit.cache/test-results b/.phpunit.cache/test-results new file mode 100644 index 00000000..cad9b7af --- /dev/null +++ b/.phpunit.cache/test-results @@ -0,0 +1 @@ +{"version":"pest_2.33.4","defects":{"P\\Tests\\Unit\\PsrTest::__pest_evaluable_when_using_the_url_for_query_parameters_you_can_use_dots_and_semicolons":7},"times":{"P\\Tests\\Unit\\PsrTest::__pest_evaluable_a_psr_7_request_can_be_created_from_the_PendingRequest":0.019,"P\\Tests\\Unit\\PsrTest::__pest_evaluable_if_request_body_is_present_then_it_will_be_on_the_psr_7_request":0.002,"P\\Tests\\Unit\\PsrTest::__pest_evaluable_you_can_generate_a_uri_from_the_PendingRequest":0.001,"P\\Tests\\Unit\\PsrTest::__pest_evaluable_when_using_the_url_for_query_parameters_you_can_use_dots_and_semicolons":0.021,"P\\Tests\\Unit\\PsrTest::__pest_evaluable_when_using_the_url_for_query_parameters_you_can_use_dots_and_value_less_parameters":0.015}} \ No newline at end of file diff --git a/src/Helpers/URLHelper.php b/src/Helpers/URLHelper.php index 1e93f8e3..1e4e2692 100644 --- a/src/Helpers/URLHelper.php +++ b/src/Helpers/URLHelper.php @@ -46,4 +46,28 @@ public static function isValidUrl(string $url): bool { return ! empty(filter_var($url, FILTER_VALIDATE_URL)); } + + /** + * Parse a query string into an array + * + * @param string $query + * @return array + */ + public static function parseQueryString(string $query): array + { + if ($query === '') { + return []; + } + + $parameters = []; + + foreach (explode('&', $query) as $parameter) { + $name = strtok($parameter, '='); + $value = strtok('='); + + $parameters[$name] = $value !== false ? $value : ''; + } + + return $parameters; + } } diff --git a/src/Traits/PendingRequest/ManagesPsrRequests.php b/src/Traits/PendingRequest/ManagesPsrRequests.php index fd6b3b8e..41030538 100644 --- a/src/Traits/PendingRequest/ManagesPsrRequests.php +++ b/src/Traits/PendingRequest/ManagesPsrRequests.php @@ -8,6 +8,7 @@ use Saloon\Data\FactoryCollection; use Psr\Http\Message\RequestInterface; use Saloon\Contracts\Body\BodyRepository; +use Saloon\Helpers\URLHelper; trait ManagesPsrRequests { @@ -27,7 +28,7 @@ public function getUri(): UriInterface // and then we'll merge in Saloon's query parameters. Our query parameters will take // priority over any that were defined in the URL. - parse_str($uri->getQuery(), $existingQuery); + $existingQuery = URLHelper::parseQueryString($uri->getQuery()); return $uri->withQuery( http_build_query(array_merge($existingQuery, $this->query()->all())) diff --git a/tests/Unit/PsrTest.php b/tests/Unit/PsrTest.php index b1bdc01f..cc40b175 100644 --- a/tests/Unit/PsrTest.php +++ b/tests/Unit/PsrTest.php @@ -5,6 +5,7 @@ use Psr\Http\Message\UriInterface; use Psr\Http\Message\StreamInterface; use Psr\Http\Message\RequestInterface; +use Saloon\Helpers\URLHelper; use Saloon\Tests\Fixtures\Requests\UserRequest; use Saloon\Tests\Fixtures\Connectors\TestConnector; use Saloon\Tests\Fixtures\Requests\HasJsonBodyRequest; @@ -59,3 +60,13 @@ expect($uri->getQuery())->toEqual('include=hats&sort=first_name&per_page=100'); expect($uri->getFragment())->toEqual('fragment-123'); }); + +test('when using the url for query parameters you can use dots and value-less parameters', function () { + $connector = new TestConnector; + $request = new QueryParameterRequest('/user?account.id=1&checked&name=sam'); + + $pendingRequest = $connector->createPendingRequest($request); + $uri = $pendingRequest->getUri(); + + expect($uri->getQuery())->toEqual('account.id=1&checked=&name=sam&per_page=100'); +}); From fa06e4dd98aaaec7696bb7163b7b661b6d6da217 Mon Sep 17 00:00:00 2001 From: Sammyjo20 Date: Mon, 12 Feb 2024 11:40:10 +0000 Subject: [PATCH 2/5] =?UTF-8?q?=F0=9F=AA=84=20Code=20Style=20Fixes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/Helpers/URLHelper.php | 1 - src/Traits/PendingRequest/ManagesPsrRequests.php | 2 +- tests/Unit/PsrTest.php | 1 - 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Helpers/URLHelper.php b/src/Helpers/URLHelper.php index 1e4e2692..21c81ce2 100644 --- a/src/Helpers/URLHelper.php +++ b/src/Helpers/URLHelper.php @@ -50,7 +50,6 @@ public static function isValidUrl(string $url): bool /** * Parse a query string into an array * - * @param string $query * @return array */ public static function parseQueryString(string $query): array diff --git a/src/Traits/PendingRequest/ManagesPsrRequests.php b/src/Traits/PendingRequest/ManagesPsrRequests.php index 41030538..1cd8e155 100644 --- a/src/Traits/PendingRequest/ManagesPsrRequests.php +++ b/src/Traits/PendingRequest/ManagesPsrRequests.php @@ -4,11 +4,11 @@ namespace Saloon\Traits\PendingRequest; +use Saloon\Helpers\URLHelper; use Psr\Http\Message\UriInterface; use Saloon\Data\FactoryCollection; use Psr\Http\Message\RequestInterface; use Saloon\Contracts\Body\BodyRepository; -use Saloon\Helpers\URLHelper; trait ManagesPsrRequests { diff --git a/tests/Unit/PsrTest.php b/tests/Unit/PsrTest.php index cc40b175..05747ea1 100644 --- a/tests/Unit/PsrTest.php +++ b/tests/Unit/PsrTest.php @@ -5,7 +5,6 @@ use Psr\Http\Message\UriInterface; use Psr\Http\Message\StreamInterface; use Psr\Http\Message\RequestInterface; -use Saloon\Helpers\URLHelper; use Saloon\Tests\Fixtures\Requests\UserRequest; use Saloon\Tests\Fixtures\Connectors\TestConnector; use Saloon\Tests\Fixtures\Requests\HasJsonBodyRequest; From 29c10e501edac390cfe86d626f60c76c7598e213 Mon Sep 17 00:00:00 2001 From: Sammyjo20 <29132017+Sammyjo20@users.noreply.github.com> Date: Mon, 12 Feb 2024 11:40:48 +0000 Subject: [PATCH 3/5] Ignore phpunit cache --- .gitignore | 1 + .phpunit.cache/test-results | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) delete mode 100644 .phpunit.cache/test-results diff --git a/.gitignore b/.gitignore index 271dc0b3..37943bee 100644 --- a/.gitignore +++ b/.gitignore @@ -15,6 +15,7 @@ .php-cs-fixer.cache .phpunit.result.cache tests/Fixtures/Saloon/Testing +.phpunit.cache/test-results # environments/configs phpstan.neon diff --git a/.phpunit.cache/test-results b/.phpunit.cache/test-results deleted file mode 100644 index cad9b7af..00000000 --- a/.phpunit.cache/test-results +++ /dev/null @@ -1 +0,0 @@ -{"version":"pest_2.33.4","defects":{"P\\Tests\\Unit\\PsrTest::__pest_evaluable_when_using_the_url_for_query_parameters_you_can_use_dots_and_semicolons":7},"times":{"P\\Tests\\Unit\\PsrTest::__pest_evaluable_a_psr_7_request_can_be_created_from_the_PendingRequest":0.019,"P\\Tests\\Unit\\PsrTest::__pest_evaluable_if_request_body_is_present_then_it_will_be_on_the_psr_7_request":0.002,"P\\Tests\\Unit\\PsrTest::__pest_evaluable_you_can_generate_a_uri_from_the_PendingRequest":0.001,"P\\Tests\\Unit\\PsrTest::__pest_evaluable_when_using_the_url_for_query_parameters_you_can_use_dots_and_semicolons":0.021,"P\\Tests\\Unit\\PsrTest::__pest_evaluable_when_using_the_url_for_query_parameters_you_can_use_dots_and_value_less_parameters":0.015}} \ No newline at end of file From b881b89ec79ec9b88f7979066723a28443f263f9 Mon Sep 17 00:00:00 2001 From: Sammyjo20 <29132017+Sammyjo20@users.noreply.github.com> Date: Mon, 12 Feb 2024 11:46:01 +0000 Subject: [PATCH 4/5] Fixed PHPStan issues --- src/Helpers/URLHelper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Helpers/URLHelper.php b/src/Helpers/URLHelper.php index 21c81ce2..d8f79c65 100644 --- a/src/Helpers/URLHelper.php +++ b/src/Helpers/URLHelper.php @@ -50,7 +50,7 @@ public static function isValidUrl(string $url): bool /** * Parse a query string into an array * - * @return array + * @return array */ public static function parseQueryString(string $query): array { From 99b269523d1a94421987b834b7aa557e83369f5c Mon Sep 17 00:00:00 2001 From: Sammyjo20 <29132017+Sammyjo20@users.noreply.github.com> Date: Mon, 12 Feb 2024 13:48:10 +0000 Subject: [PATCH 5/5] Review comments --- src/Helpers/URLHelper.php | 12 ++++++++---- tests/Unit/URLHelperTest.php | 15 ++++++++++++++- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/src/Helpers/URLHelper.php b/src/Helpers/URLHelper.php index d8f79c65..fe62c961 100644 --- a/src/Helpers/URLHelper.php +++ b/src/Helpers/URLHelper.php @@ -50,7 +50,7 @@ public static function isValidUrl(string $url): bool /** * Parse a query string into an array * - * @return array + * @return array */ public static function parseQueryString(string $query): array { @@ -61,10 +61,14 @@ public static function parseQueryString(string $query): array $parameters = []; foreach (explode('&', $query) as $parameter) { - $name = strtok($parameter, '='); - $value = strtok('='); + $name = urldecode((string)strtok($parameter, '=')); + $value = urldecode((string)strtok('=')); - $parameters[$name] = $value !== false ? $value : ''; + if (! $name || str_starts_with($parameter, '=')) { + continue; + } + + $parameters[$name] = $value; } return $parameters; diff --git a/tests/Unit/URLHelperTest.php b/tests/Unit/URLHelperTest.php index c6bc89fa..36000428 100644 --- a/tests/Unit/URLHelperTest.php +++ b/tests/Unit/URLHelperTest.php @@ -5,7 +5,7 @@ use Saloon\Helpers\URLHelper; test('the URL helper will join two URLs together', function ($baseUrl, $endpoint, $expected) { - expect(URLHelper::join($baseUrl, $endpoint))->toEqual($expected); + expect(URLHelper::join($baseUrl, $endpoint))->toBe($expected); })->with([ ['https://google.com', '/search', 'https://google.com/search'], ['https://google.com', 'search', 'https://google.com/search'], @@ -16,3 +16,16 @@ ['', 'google.com/search', '/google.com/search'], ['https://google.com', 'https://api.google.com/search', 'https://api.google.com/search'], ]); + +test('the URL helper can parse a variety of query parameters', function (string $query, array $expected) { + expect(URLHelper::parseQueryString($query))->toBe($expected); +})->with([ + ['foo=bar', ['foo' => 'bar']], + ['foo=bar&name=sam', ['foo' => 'bar', 'name' => 'sam']], + ['foo==bar&name=sam', ['foo' => 'bar', 'name' => 'sam']], + ['=abc&name=sam', ['name' => 'sam']], + ['foo&name=sam', ['foo' => '', 'name' => 'sam']], + ['account.id=1', ['account.id' => '1']], + ['name=cowboy%20sam', ['name' => 'cowboy sam']], + ['name=sam&', ['name' => 'sam']], +]);