Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:130306
Precedence: list
MIME-Version: 1.0
References: <CAH5C8xXUPKgd0wWwLC3G2Ye=kRZN58gK3DY82v5GoduBNNtb5A@mail.gmail.com>
 <83238ad3-c844-4457-dfb3-11321787e022@php.net> <CAH5C8xUOEkfQ6u8AxuV8UGjWU0h7goDsvWa9QQmANhNdU=Dwhw@mail.gmail.com>
 <CAOV5rgZ1LTVf9PBpfQekahmESo=KXE=RVrpeUXrbqxG4OSDMnQ@mail.gmail.com>
 <d6a46bb627a9545e621f37e83c0441e5@glaive.pro> <CAOV5rgZGKJEo+jV7q-qQFB59CXhfNwadYPDbm70e_F+1v=Fgng@mail.gmail.com>
 <CAH5C8xWwcNC3z=FxQZiN2r1aZ1FgriVNAtdw+YwhaN4S2joyzw@mail.gmail.com>
 <CAOV5rgaLX9s-=WZZgxoTE4wmWEzL9mZUF-d8bzGrN3BQDU7i3w@mail.gmail.com>
 <CAOV5rgY3+PrOk2sH0iZ2ne1wg1WVzW4evh6qwu1nyq68tMH-9A@mail.gmail.com>
 <CAH5C8xUEBtywOxgV4zgHVhdzO0KoFejtp04q0LQ+bEAb9nUAvw@mail.gmail.com>
 <CAOV5rgaGjJ+_Qa10pVaCYD_aozV10+UHvLUuVuNOXTz87xHpXA@mail.gmail.com>
 <CAOV5rgZ5y6YF3hLeDAc8TcaOyxkbbF7S0U7R7JMx8Cwff=3QGA@mail.gmail.com>
 <CAH5C8xXAtUN0ja_thZqbN4u4nndr2Lp_SxqZT3E+WN5ASFqWSA@mail.gmail.com>
 <CAH5C8xVjx1v_CX+5O4sj7CVyJOx4rDASvuqOukWfgupPa=YtGA@mail.gmail.com> <CAOV5rgbsDscEffqiv+c7WThN-Aj0xhL0w5eE13ALY_F5Kmcwmg@mail.gmail.com>
In-Reply-To: <CAOV5rgbsDscEffqiv+c7WThN-Aj0xhL0w5eE13ALY_F5Kmcwmg@mail.gmail.com>
Date: Fri, 13 Mar 2026 10:38:45 +0100
Message-ID: <CAH5C8xXXrY5iPuAcL0XXYDoRPh=Tz0m7xhszspNqXnfVmM_aAQ@mail.gmail.com>
Subject: Re: [PHP-DEV] [RFC] [Discussion] Followup Improvements for ext/uri
To: ignace nyamagana butera <nyamsprod@gmail.com>
Cc: PHP Internals List <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000573797064ce4a35f"
From: kocsismate90@gmail.com (=?UTF-8?B?TcOhdMOpIEtvY3Npcw==?=)

--000000000000573797064ce4a35f
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Ignace,

I just re-read the RFC and I like the updates and precision you've brought
> to it here's my review:
> For the builders I have nothing more design wise to add this is already
> solid. I may nitpick on the *Builder::clear() method name I would have go=
ne
> with *Builder::reset() but I presume other developers would go with clear=
.
> Other than that the public API is spot on.
>

I also like the reset() method name much more! So I'll update the RFC
accordingly.

For the Enum, my only concern is that they serve just as flags and their
> usage is tightly coupled to the Uri classes. I would add 2 static named
> constructors fromUrl and tryFromUrl just for completeness. I believe the
> maintenance cost is negligible but the developer DX is improved and allow=
s
> for a broader usage of the Enum.
>

I don't really understand why it would make sense to invert the coupling?
(decouple the UriHostType/UrlHostType and the
UriType enums from the Uri/Url classes, and in the same time, couple the
enums to the Uri/Url classes). IMO it's far more
ergonomic to retrieve the URI type/host type from the URI class directly,
rather than to instantiate an enum each time?


> Last but not least, The Percent encoding feature should be IMHO improved
> by moving the encode/decode methods from being static methods on the URI
> classes to becoming public API on the Enum. This would indeed imply
> renaming the enum from  Uri\Rfc3986\UriPercentEncodingMode to
> Uri\Rfc3986\UriPercentEncoder with two methods encode/decode. Again it
> makes for a more self-contained feature and adds to the DX. Developer wil=
l
> not have to always statically call the URI classes for encoding/decoding
> strings as the Enums and their cases already convey the information
> correctly.
>

Personally - and I may be in the minority - I don't see an issue with
having two static methods on Uri/Url.
Uri::percentEncode() and Uri::percentDecode() as well as
Url::percentEncode() and Url::percentDecode()
could indeed be implemented via a dedicated UriPercentEncoder and
UrlPercentEncoder class, or even a
shared PercentEncoder one, but:

- Its methods would still be static
- I don't think it's worth to add one or two dedicated classes just for
this purpose

I also got feedback that these functions could be free-standing in the URI
namespace. But I really don't like free standing functions,
so I won't go in this direction. IMO even two static methods on the Uri and
Url classes are easier to use and find.

So if we reject these ideas, then the next candidate is your suggestion of
having a UriComponent/UrlComponent enum
with an encode() and decode() method (
https://github.com/thephpleague/uri-src/pull/186#issuecomment-4016602880):

$uri =3D new Uri\Rfc3986\Uri("https://example.com/?q=3D%3A%29");
$query =3D $uri->getQuery(); // returns "q=3D%3A%29"
echo Uri\Rfc3986\UriComponent::Query->decode($query); // returns "q=3D:)"

But as I mentioned in my comment on the PR, percent-encoding/decoding is
not necessarily tied to URI/URL components,
for example because the proposal currently contains the
Uri\Rfc3986\UriPercentEncodingMode::AllReservedCharacters,
Uri\Rfc3986\UriPercentEncodingMode::AllButUnreservedCharacters, or
Uri\Rfc3986\UriPercentEncodingMode::PathSegment.

Some of the enum cases which don't relate to a component could be removed,
but at least the AllReservedCharacters case is
important because it provides a direct alternative for rawurlencode() and
rawurldecode(). My side-quest is to gradually phase out
*urlencode() and *urldecode() functions because their naming is very
confusing, and people usually don't know when to use which.

And I've just noticed that probably yet another enum case would be needed
to provide a direct alternative for urlencode() and urldecode(),
because they differ from rawurlencode() and rawurldecode() with regards to
how the "~" is handled, besides the " " character.
But at this point, I became unsure if it's worth to pursue this goal,
because this is not RFC 3986 compliant behavior anymore (and TBH
not even Uri\Rfc3986\UriPercentEncodingMode::FormQuery is compliant), so it
has nothing to do with the Uri\Rfc3986 namespace.

So all in all... As far as I can see, not even a Uri\Rfc3986\UriComponent
enum could provide a complete solution for the custom
percent-encoding/decoding part of the proposal. If we used a
Uri\Rfc3986\UriEncoding enum name instead, then there would be no issue
with the various kinds of encoding/decoding modes not referring to URI
components, but the naming would probably still not be right,
as I wouldn't expect a class name with "ing" suffix to perform
percent-encoding/decoding itself. But I'm happy to be corrected by
native English speakers :)

As I don't have any other ideas, I think I still prefer the static method
based approach.

Regards,
M=C3=A1t=C3=A9

--000000000000573797064ce4a35f
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr"><div dir=3D"ltr">Hi Ignace,</div><div dir=
=3D"ltr"><br></div><div class=3D"gmail_quote"><blockquote class=3D"gmail_qu=
ote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,20=
4);padding-left:1ex"><div dir=3D"ltr"><div>I just re-read the RFC and I lik=
e the updates and precision you&#39;ve brought to it here&#39;s my review:<=
/div><div>For the builders I have nothing more design wise to add this is a=
lready solid. I may nitpick on the *Builder::clear() method name I would ha=
ve gone with *Builder::reset() but I presume other developers would go with=
 clear. Other than that the public API is spot on.</div></div></blockquote>=
<div><br></div><div>I also like the reset() method name much more! So I&#39=
;ll update the RFC accordingly.</div><div><br></div><blockquote class=3D"gm=
ail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,=
204,204);padding-left:1ex"><div dir=3D"ltr"><div>For the Enum, my only conc=
ern is that they serve just as flags and their usage is tightly=C2=A0couple=
d to the Uri classes. I would add=C2=A02 static=C2=A0named constructors fro=
mUrl and tryFromUrl just for completeness. I believe the maintenance cost i=
s negligible=C2=A0but the developer DX is improved and allows for a broader=
 usage of the Enum.</div></div></blockquote><div><br></div><div>I don&#39;t=
 really understand why it would make sense to invert the coupling? (decoupl=
e the UriHostType/UrlHostType and the</div><div>UriType enums from the Uri/=
Url classes, and in the same time, couple the enums to the Uri/Url classes)=
. IMO it&#39;s far more</div><div>ergonomic to retrieve the URI type/host t=
ype from the URI class directly, rather than to instantiate an enum each ti=
me?</div><div>=C2=A0</div><blockquote class=3D"gmail_quote" style=3D"margin=
:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"=
><div dir=3D"ltr"><div></div><div>Last=C2=A0but not=C2=A0least, The Percent=
 encoding feature should be IMHO improved by moving the encode/decode metho=
ds from being static methods on the URI classes to becoming public API on t=
he Enum. This would indeed imply renaming the enum from=C2=A0 Uri\Rfc3986\U=
riPercentEncodingMode to Uri\Rfc3986\UriPercentEncoder with two methods enc=
ode/decode. Again it makes for a more self-contained feature and adds to th=
e DX. Developer will not have to always statically call the URI classes for=
 encoding/decoding strings as the Enums and their cases already convey the =
information correctly.</div></div></blockquote><div><br></div><div>Personal=
ly - and I may be in the minority - I don&#39;t see an issue with having tw=
o static methods on Uri/Url.</div><div>Uri::percentEncode() and Uri::percen=
tDecode() as well as Url::percentEncode() and Url::percentDecode()</div><di=
v>could indeed be implemented via a dedicated UriPercentEncoder and UrlPerc=
entEncoder class, or even a</div><div>shared PercentEncoder one, but:</div>=
<div><br></div><div>- Its methods would still be static</div><div>- I don&#=
39;t think it&#39;s worth to add one or two dedicated classes just for this=
 purpose</div><div><br></div><div>I also got feedback that these functions =
could be free-standing in the URI namespace. But I really don&#39;t like=C2=
=A0free=C2=A0standing functions,</div><div>so I won&#39;t=C2=A0go in this d=
irection. IMO even two static methods on the Uri and Url classes are easier=
 to use and find.</div><div><br></div><div>So if we reject these ideas, the=
n the next candidate is your suggestion of having a UriComponent/UrlCompone=
nt enum</div><div>with an encode() and decode() method (<a href=3D"https://=
github.com/thephpleague/uri-src/pull/186#issuecomment-4016602880" target=3D=
"_blank">https://github.com/thephpleague/uri-src/pull/186#issuecomment-4016=
602880</a>):</div><div><br></div><div>$uri =3D new Uri\Rfc3986\Uri(&quot;<a=
 href=3D"https://example.com/?q=3D%3A%29" target=3D"_blank">https://example=
.com/?q=3D%3A%29</a>&quot;);<br>$query =3D $uri-&gt;getQuery(); // returns =
&quot;q=3D%3A%29&quot;<br>echo Uri\Rfc3986\UriComponent::Query-&gt;decode($=
query); // returns &quot;q=3D:)&quot;</div><div><br></div><div>But as I men=
tioned in my comment on the PR, percent-encoding/decoding is not necessaril=
y tied to URI/URL components,</div><div>for example because the proposal cu=
rrently contains the Uri\Rfc3986\UriPercentEncodingMode::AllReservedCharact=
ers,</div><div>Uri\Rfc3986\UriPercentEncodingMode::AllButUnreservedCharacte=
rs, or Uri\Rfc3986\UriPercentEncodingMode::PathSegment.</div><div><br></div=
><div>Some of the enum cases which don&#39;t relate to a component could be=
 removed, but at least the AllReservedCharacters case is</div><div>importan=
t because it provides a direct alternative for rawurlencode() and rawurldec=
ode(). My side-quest is to gradually phase out</div><div>*urlencode() and *=
urldecode() functions because their naming is very confusing,=C2=A0and peop=
le usually don&#39;t know when to use which.</div><div><br></div><div>And I=
&#39;ve just noticed that probably yet another enum case would be needed to=
 provide a direct alternative for urlencode() and urldecode(),</div><div>be=
cause they differ from rawurlencode() and rawurldecode() with regards to ho=
w the &quot;~&quot; is handled, besides the &quot; &quot; character.</div><=
div>But at this point, I became unsure if it&#39;s worth to pursue this goa=
l, because this is not RFC 3986 compliant behavior anymore (and TBH</div><d=
iv>not even Uri\Rfc3986\UriPercentEncodingMode::FormQuery is compliant), so=
 it has nothing to do with the Uri\Rfc3986 namespace.</div><div><br></div><=
div>So all in all... As far as I can see, not even a Uri\Rfc3986\UriCompone=
nt enum could provide a complete solution for the custom</div><div>percent-=
encoding/decoding part of the proposal. If we used a Uri\Rfc3986\UriEncodin=
g enum name instead, then there would be no issue</div><div>with the variou=
s kinds of encoding/decoding modes not referring to URI components, but the=
 naming would probably still not be right,</div><div>as I wouldn&#39;t expe=
ct a class name with &quot;ing&quot; suffix to perform percent-encoding/dec=
oding itself. But I&#39;m happy to be corrected by</div><div>native English=
 speakers :)=C2=A0</div><div><br></div><div>As I don&#39;t have any other i=
deas, I think I still prefer the static method based approach.</div><div><b=
r></div><div>Regards,</div><div>M=C3=A1t=C3=A9</div><div><br></div><div><br=
></div><div><br></div><blockquote class=3D"gmail_quote" style=3D"margin:0px=
 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><di=
v class=3D"gmail_quote"><blockquote class=3D"gmail_quote" style=3D"margin:0=
px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><=
div dir=3D"ltr"><div class=3D"gmail_quote"><blockquote class=3D"gmail_quote=
" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);=
padding-left:1ex"><div dir=3D"ltr"><div class=3D"gmail_quote"><blockquote c=
lass=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px soli=
d rgb(204,204,204);padding-left:1ex"><div dir=3D"ltr">
</div>
</blockquote></div></div>
</blockquote></div></div>
</blockquote></div>
</blockquote></div></div>
</div>

--000000000000573797064ce4a35f--