fix: keep valueless query parameters when casting to a string
Valueless query parameters (for example ?foo) are stored as null, and has() reports them as present, but __toString() passed those null values to http_build_query() which skips them entirely, dropping the parameters from the output. Render null values as empty-valued parameters so they survive a round-trip.
This commit is contained in:
1 parent
857983643e
commit
d71aedae66
3 files changed
+94
-1
No files matched your search
@@ -0,0 +1,72 @@
|
||||
# fix: keep valueless query parameters when casting to a string
|
||||
|
||||
## Problem
|
||||
|
||||
A query parameter that has no value (for example `?foo` or `?foo=`) is parsed and
|
||||
stored, and `has()` / `getAllQueryParameters()` report it as present, but casting
|
||||
the bag (or the `Url`) back to a string silently drops it:
|
||||
|
||||
```php
|
||||
$url = Spatie\Url\Url::fromString('https://example.com/?foo=');
|
||||
$url->hasQueryParameter('foo'); // true
|
||||
(string) $url; // "https://example.com" <-- "foo" is gone
|
||||
```
|
||||
|
||||
So `hasQueryParameter('foo')` is `true` while the parameter disappears from the
|
||||
string representation, and the URL does not round-trip.
|
||||
|
||||
## Root cause
|
||||
|
||||
`src/QueryParameterBag.php`. `fromString()` deliberately stores valueless
|
||||
parameters as `null` (added in #11, and pinned by the "empty query parameters
|
||||
default to null" test):
|
||||
|
||||
```php
|
||||
$parameters = array_map(fn ($param) => $param !== '' ? $param : null, $parameters);
|
||||
```
|
||||
|
||||
but `__toString()` passes those `null` values straight to `http_build_query()`:
|
||||
|
||||
```php
|
||||
return http_build_query($this->parameters, '', '&', PHP_QUERY_RFC3986);
|
||||
```
|
||||
|
||||
`http_build_query()` skips `null` values entirely, so every valueless parameter
|
||||
is removed from the output.
|
||||
|
||||
## The fix
|
||||
|
||||
Render `null` values as empty-valued parameters before building the query string,
|
||||
so they survive the round-trip:
|
||||
|
||||
```php
|
||||
$parameters = array_map(fn ($value) => $value ?? '', $this->parameters);
|
||||
|
||||
return http_build_query($parameters, '', '&', PHP_QUERY_RFC3986);
|
||||
```
|
||||
|
||||
`?foo` / `?foo=` now serialize back to `foo=` (which parses to the same value),
|
||||
and parameters with an actual value or an array value are unaffected.
|
||||
|
||||
## Tests
|
||||
|
||||
Added two cases to `QueryParameterBagTest`:
|
||||
|
||||
- `it keeps valueless parameters when being casted to a string`
|
||||
(`fromString('offset')` -> `has('offset')` true and `__toString()` === `offset=`).
|
||||
- `it keeps valueless parameters alongside valued ones when being casted to a string`
|
||||
(`fromString('foo=&bar=baz')->__toString()` === `foo=&bar=baz`).
|
||||
|
||||
## Gate
|
||||
|
||||
```
|
||||
composer install && vendor/bin/pest
|
||||
```
|
||||
|
||||
(`phpunit` is intentionally blocked by this package in favour of Pest.)
|
||||
|
||||
Result: exit code `0` - `Tests: 110 passed`.
|
||||
|
||||
Counterfactual: reverting `__toString()` to pass `$this->parameters` directly to
|
||||
`http_build_query()` makes both new tests fail (the valueless parameters are
|
||||
dropped from the output).
|
||||
@@ -65,6 +65,12 @@ class QueryParameterBag implements \Stringable
|
||||
|
||||
public function __toString(): string
|
||||
{
|
||||
return http_build_query($this->parameters, '', '&', PHP_QUERY_RFC3986);
|
||||
// Parameters without a value are stored as null (for example "?foo"),
|
||||
// but http_build_query() skips null values entirely, which would drop
|
||||
// parameters that has() still reports as present. Render them with an
|
||||
// empty value so they survive a round-trip.
|
||||
$parameters = array_map(fn ($value) => $value ?? '', $this->parameters);
|
||||
|
||||
return http_build_query($parameters, '', '&', PHP_QUERY_RFC3986);
|
||||
}
|
||||
}
|
||||
@@ -89,6 +89,21 @@ it('can be casted to a string', function () {
|
||||
});
|
||||
|
||||
|
||||
it('keeps valueless parameters when being casted to a string', function () {
|
||||
$queryParameterBag = QueryParameterBag::fromString('offset');
|
||||
|
||||
expect($queryParameterBag)->has('offset')->toBeTrue();
|
||||
expect($queryParameterBag)->__toString()->toEqual('offset=');
|
||||
});
|
||||
|
||||
|
||||
it('keeps valueless parameters alongside valued ones when being casted to a string', function () {
|
||||
$queryParameterBag = QueryParameterBag::fromString('foo=&bar=baz');
|
||||
|
||||
expect($queryParameterBag)->__toString()->toEqual('foo=&bar=baz');
|
||||
});
|
||||
|
||||
|
||||
it('can be created from a string with url encoded values', function () {
|
||||
$queryParameterBag = QueryParameterBag::fromString(
|
||||
'category=storage%20furniture&discount=%3E40%25%20off&range%5B0%5D=10&range%5B1%5D=20'
|
||||
|
||||
Reference in new issue
Block a user