[RFC] [Discussion] Followup Improvements for ext/uri

Hi Everyone,

I'd like to introduce my latest RFC that I've been working on for a while now: https://wiki.php.net/rfc/uri_followup.

It proposes 5 followup improvements for ext/uri in the following areas:

• URI Building
• Query Parameter Manipulation
• Accessing Path Segments as an Array
• Host Type Detection
• URI Type Detection
• Percent-Encoding and Decoding Support

I did my best to write an RFC that was at least as extensive as https://wiki.php.net/rfc/url_parsing_api had become by the end. Despite my efforts,
there are still a couple things which need a final decision, or which need to be polished/improved. Some examples:

• How to support array/object values for constructing query strings? (https://wiki.php.net/rfc/uri_followup#type_support)
• How to make the UriQueryParams and UrlQueryParams classes more interoperable with the query string component (mainly with respect to percent-encoding)? (https://wiki.php.net/rfc/uri_followup#percent-encoding_and_decoding)
• Exactly how the advanced percent-decoding capabilities should work? Does it make sense to support all the possible modes (UriPercentEncodingMode) for percent-decoding as well (https://wiki.php.net/rfc/uri_followup#percent-encoding_and_decoding_support)
• etc.

Regards,
Máté

Hello!

For the builder methods, why not use the same wither method names? That would make switching to them really easy, over the current implementation.

— Rob

Hi Máté,
Once again thanks for this follow up RFC. While there are a lot to digest I
wanted to point out your reservation around implementing the
IteratorAggregate interface for Query Manipulation,

The UriQueryParams and UrlQueryParams classes could implement the

IteratorAggregate interface in theory. However, it's not possible to do so
due to query components that share the same name, e.g.:
param=foo&param=bar&param=baz. In this case, the same key (param) would be
repeated 3 times - and it's actually not possible to support with iterators.

When building the Query component in league URI I was able to use the
Countable and IteratorAggregate interface using a different representation
of the query pair see
https://uri.thephpleague.com/components/7.0/query/#countable-and-iteratoraggregate
TL;DR instead of using your proposed structure

[['name' => 'value'],...]

I used the following

[['name', 'value'], ...]

While both format IMHO can allow implementing the IteratorAggregate
interface, the latter allows for a more predictable API

$uri = new Uri('https://example.com?param=foo&param=bar&param=baz');
foreach ($uri->getQueryParams() as $key => $pair) {
    //first iteration $pair['param'] = 'foo'
    //second iteration $pair['param'] = 'bar'
    //third iteration $pair['param'] = 'baz'
}

The user needs to know beforehand the name of the pair which is counter
intuitive if you do not know the exact position
of the pair. In contrast, using the league URI query syntax you will have
the following:

$uri = new Uri('https://example.com?param=foo&param=bar&param=baz');
Query::fromUri($uri);
foreach (Query::fromUri($uri) as $key => $pair) {
//first iteration $pair[0] = 'param'; $pair[1] = 'foo'
//second iteration $pair[0] = 'param'; $pair[1] = 'baz'
//third iteration $pair[0] = 'param'; $pair[1] = 'bar'
}

The user will always get the parameter name using $pair[0] and the value
using $pair[1] regardless of their content and value.

What do you think ? This IMHO would solve your issue but it is indeed a
stronger departure to how query strings are parsed in PHP currently.

Best regards.



> Hi Everyone,
>
> I'd like to introduce my latest RFC that I've been working on for a while
> now: https://wiki.php.net/rfc/uri_followup.
>
> It proposes 5 followup improvements for ext/uri in the following areas:
> - URI Building
> - Query Parameter Manipulation
> - Accessing Path Segments as an Array
> - Host Type Detection
> - URI Type Detection
> - Percent-Encoding and Decoding Support
>
> I did my best to write an RFC that was at least as extensive as
> https://wiki.php.net/rfc/url_parsing_api had become by the end. Despite
> my efforts,
> there are still a couple things which need a final decision, or which
> need to be polished/improved. Some examples:
>
> - How to support array/object values for constructing query strings? (
> https://wiki.php.net/rfc/uri_followup#type_support)
> - How to make the UriQueryParams and UrlQueryParams classes more
> interoperable with the query string component (mainly with respect to
> percent-encoding)? (
> https://wiki.php.net/rfc/uri_followup#percent-encoding_and_decoding)
> - Exactly how the advanced percent-decoding capabilities should work? Does
> it make sense to support all the possible modes (UriPercentEncodingMode)
> for percent-decoding as well (
> https://wiki.php.net/rfc/uri_followup#percent-encoding_and_decoding_support
> )
> - etc.
>
> Regards,
> Máté

Hi Everyone,

I'd like to introduce my latest RFC that I've been working on for a
while now: https://wiki.php.net/rfc/uri_followup.

It proposes 5 followup improvements for ext/uri in the following areas:

• URI Building
• Query Parameter Manipulation
• Accessing Path Segments as an Array
• Host Type Detection
• URI Type Detection
• Percent-Encoding and Decoding Support

I did my best to write an RFC that was at least as extensive as
https://wiki.php.net/rfc/url_parsing_api had become by the end. Despite
my efforts,
there are still a couple things which need a final decision, or which
need to be polished/improved. Some examples:

• How to support array/object values for constructing query strings?
  (https://wiki.php.net/rfc/uri_followup#type_support)
• How to make the UriQueryParams and UrlQueryParams classes more
  interoperable with the query string component (mainly with respect to
  percent-encoding)?
  (https://wiki.php.net/rfc/uri_followup#percent-encoding_and_decoding)
• Exactly how the advanced percent-decoding capabilities should work?
  Does it make sense to support all the possible modes
  (UriPercentEncodingMode) for percent-decoding as well
  (https://wiki.php.net/rfc/uri_followup#percent-encoding_and_decoding_support)
• etc.

Regards,
Máté

Thanks, Máté.

Notes as I read through:

• I really, really hate the "set" prefix on all the methods. It's a builder object, surely the "set" is implied?

$builder->scheme('https')->host('example.com')->path('/foo/bar')->build();

That's nice and easy to read.

• It really feels like there's an interface to extract here from the Url/UriBuilder classes. There's literally only one type-specific method (build()).

• UriQueryParams::hasWithValue(), could that be just hasValue()? You still need to specify the key anyway, and that's self-evident from the signature.

• There's a count() method, so shouldn't Ur{i|l]QueryParams implement Countable?

• As above, there really is an interface lurking in UriQueryParams...

• Why both Uri getRawQueryParams() and getQueryParams()? It looks like they would return the same value, no? (If not, that should be explained.)

• The sort() method... should it take an optional user callback, or do we lock people in to lexical ordering?

• It would be quite convenient of set() and append() returned $this, allowing them to be chained.

• The fromArray() logic is... totally weird and unexpected and I hate it. :-) Why can't you support repeated query parameters using nested arrays rather than gumming up all calls with a wonky format?

• It's not clear how one would start a new query from scratch, with the private constructor. There doesn't seem to be a justification for the private. I can't see why new UriQueryParams()->set('foo', 'bar') is a bad thing.

• Type support: Looks reasonable to me.

• The HostType logic seems reasonable to me.

• Url::isSpecial() Could we come up with a better name here? "Special" could mean anything unless you know the RFC; it feels like "real escape string" all over again.

Some parts of this are over my head as I've not read the relevant RFCs, but overall I do like the direction.

--Larry Garfield

Hi Máté,
After quickly checking the proposed API for Percent-Encoding and Decoding
Support I wonder if the following would not
be more appropriate ?

namespace Uri\Rfc3986 {
    enum UriPercentEncoding
    {
        case UserInfo;
        case Host;
        case RelativeReferencePath;
        case RelativeReferenceFirstPathSegment;
        case Path;
        case PathSegment;
        case Query;
        case FormQuery;
        case Fragment;
        case AllReservedCharacters;
        case All;

        public function encode(string $input): string {}
        public function decode(string $input): string {}
    }
}

With the same logic being applied in the Uri\Whatwg namespace. This
would make for a better encapsulated feature. So we can

have a clear distinction between the Value Object, its builder and the
Encoding mechanism ? What do you think?

Best regards,

Ignace

Hi Everyone,

I'd like to introduce my latest RFC that I've been working on for a while
now: https://wiki.php.net/rfc/uri_followup.

It proposes 5 followup improvements for ext/uri in the following areas:

• URI Building
• Query Parameter Manipulation
• Accessing Path Segments as an Array
• Host Type Detection
• URI Type Detection
• Percent-Encoding and Decoding Support

I did my best to write an RFC that was at least as extensive as
https://wiki.php.net/rfc/url_parsing_api had become by the end. Despite
my efforts,
there are still a couple things which need a final decision, or which
need to be polished/improved. Some examples:

• How to support array/object values for constructing query strings? (
  https://wiki.php.net/rfc/uri_followup#type_support)
• How to make the UriQueryParams and UrlQueryParams classes more
  interoperable with the query string component (mainly with respect to
  percent-encoding)? (
  https://wiki.php.net/rfc/uri_followup#percent-encoding_and_decoding)
• Exactly how the advanced percent-decoding capabilities should work? Does
  it make sense to support all the possible modes (UriPercentEncodingMode)
  for percent-decoding as well (
  https://wiki.php.net/rfc/uri_followup#percent-encoding_and_decoding_support
  )
• etc.

Regards,
Máté

Hi Màté,

I read the Accessing Path Segments as an Array sub RFC and I have a couple
of remarks, suggestions.
In the RFC text it is said that:

The getter methods return null if the path is empty (https://example.com),
an empty array when the path
consists of a single slash (https://example.com/), and a non-empty array
otherwise.

This is suboptimal to me because it means that the signature for the getter
methods is array|null which would lead
developers to always add a check in the code whenever using the method to
distinguish the path state absolute or not.
Instead, I would rather always get a single type, the array as return
value. The issue you are facing is that
you want to convey via your return type if the path is absolute or not.
But, we already have access to this
information via the UriType Enum, at least in the case of the
Uri\Rfc3986\Uri class. For the Uri\WhatWg\Uri
the information is less crucial as the validation and normalization rules
of the WHATWG specifications
will autocorrect the path if needed. This leads me to propose the following
alternative:

For Uri\Rfc3986\Uri:

/** @return list<string> */
Uri::getPathSegments(): array {}
/** @return list<string> */
Uri::getRawPathSegments(): array {}
#[\NoDiscard(message: "as Uri\Rfc3986\Uri::withPathSegments() does not
modify the object itself")]
Uri::withPathSegments(array $segments, Uri\Rfc3986\UriType $uriType =
Uri\Rfc3986\UriType::RelativePathReference): static {}

(the default value for the $uriType parameter is TBD).

For Uri\WhatWg\Url:

/** @return list<string> */
Url::getPathSegments(): array {}
#[\NoDiscard(message: "as Uri\WhatWg\Url::withPathSegments() does not
modify the object itself")]
/**  @param list<UrlValidationError> $errors */
Url::withPathSegments(array $segments): static {}

with the following behaviour

The getter methods return the empty array if the path is empty
(https://example.com https://example.com), or a single slash
(https://example.com/ https://example.com/),and a non-empty array
otherwise. To distinguish between an absolute path and a relative path you
can refer to the Uri\Rfc3986\Uri::getUriType(),
method, in case of RFC 3986 URI, and the information does not matter
otherwise (ie: for WHATWG URL).

During update, for RFC 3986 URI, The additional $uriType argument would
serve to tell if a / should be prepended or not to the generated
string path. For the WHATWG URL, no soft errors are emitted, which show
that the starting slash does not really matter.

Best regards,
Ignace

Hi Everyone,

I'd like to introduce my latest RFC that I've been working on for a while
now: https://wiki.php.net/rfc/uri_followup.

It proposes 5 followup improvements for ext/uri in the following areas:

• URI Building
• Query Parameter Manipulation
• Accessing Path Segments as an Array
• Host Type Detection
• URI Type Detection
• Percent-Encoding and Decoding Support

I did my best to write an RFC that was at least as extensive as
https://wiki.php.net/rfc/url_parsing_api had become by the end. Despite
my efforts,
there are still a couple things which need a final decision, or which
need to be polished/improved. Some examples:

• How to support array/object values for constructing query strings? (
  https://wiki.php.net/rfc/uri_followup#type_support)
• How to make the UriQueryParams and UrlQueryParams classes more
  interoperable with the query string component (mainly with respect to
  percent-encoding)? (
  https://wiki.php.net/rfc/uri_followup#percent-encoding_and_decoding)
• Exactly how the advanced percent-decoding capabilities should work? Does
  it make sense to support all the possible modes (UriPercentEncodingMode)
  for percent-decoding as well (
  https://wiki.php.net/rfc/uri_followup#percent-encoding_and_decoding_support
  )
• etc.

Regards,
Máté

Hey,

I love to see this RFC. I’ve got a few notes which likely come from me not understanding the specifications. But I think most users will not have read them and will not know or care about the differences, right?

1. Query setters and types of their parameter. Quote from the builder example:

    ->setQuery("a=1&b=2"])

    ->setQueryParams(["a" => 1, "b" => 2]) // Has the same effect as the setQuery() call above

I’d expect the setQueryParams to set the particular params that I specified instead of overriding all the query (which I’d expect setQuery to do).

Maybe it would be possible for setQuery to accept string, array and instances of *QueryParams? And setQueryParams could either be left out or override just the selected params? As far as I see, there is no equivalent withQueryParams so there’s no precedent on how such a method should work, right?

Btw I wasn’t able to find docs on the existing Uri classes (had to look it up in the prev RFC). Is the search broken or are the docs just not there yet?

2. Regarding the interface extraction, is there any difference between the internal states of Uri\WhatWg\UrlQueryParams and Uri\Rfc3986\UriQueryParams objects? I would naively expect not only a common interface, but a common class. A QueryParams that could be supplied to any of the withers/setters and that would be able to ->toRfc3986QueryString() or ->toWhatWgQueryString() on use not on instantiation.

Similarly with the builders I fail to see why do I need to decide on Uri\Rfc3986\UriBuilder and Uri\WhatWg\UrlBuilder at the start instead of having Uri\Builder that could be consumed via ->buildRfc3986Url() or `->buildWhatWgUri(). Is that UserInfo thing the whole dealbreaker? To me all the other parts like specifying host, port and so on seem spec-agnostic until serialization.

3. I agree with Larry that I’d expect [‘key1’ => ‘value1’, ‘key2’ => ‘value2’] to “just work”. I understand the issue about having multiple entries with the same keys, but I don’t think I should have to understand that to successfully build queries.

Maybe another method like bestEffortFromArray()could be added that would supoprt various forms of nested arrays and do whatever is possible to make them into a query string, similar to how http_build_query() does? I know it’s not perfect, but it would be great to have the new tooling as easy to work with as the previous, instead of having to reshape your arrays to fit a particular syntax.

BR,

Juris

Hi Everyone,

I'd like to introduce my latest RFC that I've been working on for a while
now: https://wiki.php.net/rfc/uri_followup.

It proposes 5 followup improvements for ext/uri in the following areas:

• URI Building
• Query Parameter Manipulation
• Accessing Path Segments as an Array
• Host Type Detection
• URI Type Detection
• Percent-Encoding and Decoding Support

I did my best to write an RFC that was at least as extensive as
https://wiki.php.net/rfc/url_parsing_api had become by the end. Despite
my efforts,
there are still a couple things which need a final decision, or which
need to be polished/improved. Some examples:

• How to support array/object values for constructing query strings? (
  https://wiki.php.net/rfc/uri_followup#type_support)
• How to make the UriQueryParams and UrlQueryParams classes more
  interoperable with the query string component (mainly with respect to
  percent-encoding)? (
  https://wiki.php.net/rfc/uri_followup#percent-encoding_and_decoding)
• Exactly how the advanced percent-decoding capabilities should work? Does
  it make sense to support all the possible modes (UriPercentEncodingMode)
  for percent-decoding as well (
  https://wiki.php.net/rfc/uri_followup#percent-encoding_and_decoding_support
  )
• etc.

Regards,
Máté

Hi Màté,

After thinking about it here's my take on the current proposal
regarding the Query Parameter Manipulation RFC. Sorry for the wall of
text, but I tried to summarize my thoughts.

First of all, I tried to put myself in the shoes of a regular PHP
developer who has little to no knowledge about the different URI
specifications but has a general grasp of PHP. From that point of view
the developer knows that:

• PHP already gives access to the URI query parameters via the _GET
  super globals
• to parse the query string in PHP, the developer can rely on parse_str.
• that to build a query string he should use the http_build_query function.

What we do know is that:

the _GET values are also the result of using parse_str and its logic is:

• not documented
• PHP centric
• mangles the data
• truncates query string

Its original goal was to allow direct conversion of query string into
PHP variables usable in scripts. But this behaviour has been removed
for security reasons from PHP.

http_build_query allow creating a query string in a more predictable
way but still exposes PHP centric behaviour:

• It uses get_object_vars on objects. which is counter-intuitive:

  • All iterable structures do not give the same result.
  • Depending on the object implementation the result varies between
    PHP versions (ie DateTimeImmutable used to be rendered before PHP7.4
    since then it fails silently resulting in an empty string being
    generated.)

• It adds "[", "]" and indices around arrays. This is PHP centric
  (other languages would just repeat the array name)

• It always adds the array indices even when the array is a list which
  again can lead to unexpected behaviour, even within the PHP ecosystem.

On the other hand:

• Other modern languages like Java HttpServletRequest or the WHATWG
  URLSearchParams have a complete different takes: They view the query
  string as a collection of tuple (key/value pair) that can be repeated,
  there is no notion of brackets. The data is preserved even though as
  you mention the round-trip between encoding and decoding is never
  guarantee.
• We have the new HTTP QUERY method which may or may not fall into the
  "Should this also be managed by a putative Query class".

Currently, in your proposal you have 2 Query objects. This will give
the developper a lot of work to understand where, when and which
object to choose and why. Is that complexity really needed? IMHO we
may end up with a correct API ... that no-one will use.

With all that in mind I believe a single Uri\Query should be used.
Its goal should be:

• to be immutable
• to store the query in its decoded form.
• to manipulate and change the data in a consistent way.

Decoding/encoding should happen at the object boundaries but
everything inside the object should
be done on decoded data. Since no algorithm guarantee preserving
encoding during a decode/encode round-trip,
there is no need to try hard to do so.

This also means:

• having multiple string representations
• not having a Uri::withQueryParams or a Url::withQueryParams method.

It should be left to the developer to understand which string version he needs.

On a bonus side, it would be nice to have a mechanism in PHP that
allows the application to switch
from the current parse_str usage to the new improved parsing
provided by the new class when
populating the _GET array. (So that deprecating parse_str can be
initiated in some distant future.)
This last observation/remark is not mandatory but nice to have.

So I would propose the following methods:

namespace Uri {
    //takes no arguments returns an empty object
    Query::__construct();

    // named constructor to allow
    // returning a new instance from
    // PHP variables (same syntax as http_build_query)
    Query::fromVariables(array $variable): static

    // named constructor to allow
    // returning a new instance from
    // a list of tuples see the returns
    // value of Query::toTuples()
    Query::fromTuples(array $params): static

    // named constructor to allow
    // returning a new instance from
    // query string this is where
    // decoding takes place

    Query::parseRfc1738String(): ?static
    Query::parseRfc3986String(): ?static
    Query::parseFormDataString(): ?static
    Query::parseWhatWgString(): ?static

    //String representation query
    //this is where encoding should happen
    //internal decoded data
    //should only be encoded here

    Query::toRfc3986String();
    Query::toRfc1738String();
    Query::toFormDataString();
    Query::toWhatWgString();

    // Tuple related methods
    // like the one defined by the WHATWG specifications
    // method names are changed or update to highlight
    // the immutable state for modifying methods

    Query::toTuples(): array<string, null|string|array<null|string>>
    Query::count(): int;
    Query::has(string $name): bool;
    Query::hasValue(string $name, null|string $value): bool;
    Query::getFirst(string $name): null|string;
    Query::getLast(string $name): null|string;
    Query::getAll(string $name): array<null|string>;

    // Tuple modifying methods

    Query::sort(): static;
    Query::withValue(string $name, null|string|array<null,string>
$value): static;
    Query::append(string $name, null|string|array<null,string> $value): static;
    Query::delete(string $name): static;
    Query::deleteValue(string $name, null|string $value): static;

    // PHP variables related methods
    // the parse_str replacement API

    Query::toVariables(): array;  // returns the same array as
parse_str (without mangled data)
    Query::countVariables(): int; // returns the number of variable found
    Query::hasVariable(string $variableName): bool; // tells whether
the variable exists
    Query::getVariable(string $variableName): null|string|array; //
returns the variable value
    Query::mergeVariable(array $variables): static // the same syntax
returned by the `Query::toVariables` method
    Query::replaceVariable(string $variableName,
null|string|int|float|array $value): static
    Query::deleteVariable(string $variableName): static
}

With the following changes:

• in respect to parse_str, no mangled data should occur on parsing:

parse_str("foo.bar=baz", $params);
echo $params['foo_bar'];             // returns "baz"
array_key_exists('foo.bar', $params); // returns false

$query = \Uri\Query::parseRfc1738String("foo.bar=baz");
$query->getVariable("foo.bar"); //returns "baz"
$query->hasVariable("foo_bar"); //returns false

• in respect to http_build_query.

• Only accept scalar values, null, and array. If an object or a
  resource is detected a ValueError error
  should be thrown.

echo http_build_query(['a' => `tmpfile()`]); //return '';
new \Uri\Query::fromVariables(['a' => `tmpfile()`]); // throw new ValueError

• Remove the addition of indices if the array is a list.

echo http_build_query(['a' => [3, 5, 7]]); //return
a%5B0%5D=3&a%5B1%5D=5&a%5B2%5D=7;
new \Uri\Query::fromVariables(['a' => [3, 5, 7]])->toRfc1738String();
// return a%5B%5D=3&a%5B%5D=5&a%5B%5D=7

Best regards,

Ignace

I'd like to introduce my latest RFC that I've been working on for a
while now: https://wiki.php.net/rfc/uri_followup.

It proposes 5 followup improvements for ext/uri in the following areas:

• URI Building

Would it make sense to have an interface for the set*() methods? Besides
build(), they all seem to have the same API.

• Query Parameter Manipulation

I see this adds NoDiscard:
#[\NoDiscard(message: "as Uri\Rfc3986\Uri::withQueryParams() does not modify the object itself")]

But the original methods on the classes don't have these NoDiscards, and
it doesn't seem that this RFC is suggesting to add them. It should at
least be consistent.

• Accessing Path Segments as an Array

Compare:

"especially considering the fact that Uri\Rfc3986\Uri internally stores
the path as a list of segments."

And:

Uri\Rfc3986\Uri::withPathSegments() … internally concatenate the input
segments separated by a / character, and then trigger
Uri\Rfc3986\Uri::withPath() …

Why does it need to do this concattenation, and then call withPath() for
Rfc3986\Uri then?

cheers,
Derick

--
https://derickrethans.nl | https://xdebug.org | https://dram.io

Author of Xdebug. Like it? Consider supporting me: https://xdebug.org/support

mastodon: @derickr@phpc.social @xdebug@phpc.social