Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:126489
Precedence: bulk
MIME-Version: 1.0
References: <CAH5C8xUb1O20ZDrOQNC=ckFxHUUWSK7sw_njQQzFBd0qgQqoww@mail.gmail.com>
 <dd61999c-1ebd-4765-9add-cd8065968965@gmail.com> <CAOV5rgYZ_s1of-igLFEK7oqqh6=3HeYv0=JvvKvvjkcp0F6Q9Q@mail.gmail.com>
 <CAH5C8xV67frqOBCvLt73RM7QO86_pu40sHP5h71_kDRbKPtA8Q@mail.gmail.com>
 <1BCB4144-231D-45EA-A914-98EE8F0F503A@automattic.com> <8E614C9C-BA85-45D8-9A4E-A30D69981C5D@automattic.com>
 <CAH5C8xV69pHnSKWJrVB8EQab7vPhcaXAwYezoG6z3Oj7ZkXBaQ@mail.gmail.com>
 <bc5472e39bc607e2c7accc9d2bd6f239@bastelstu.be> <9bf11a89-39d9-457b-b0ea-789fd07d7370@gmail.com>
 <af6314918964afe648c497fd9d9b7bd8@bastelstu.be>
In-Reply-To: <af6314918964afe648c497fd9d9b7bd8@bastelstu.be>
Date: Mon, 24 Feb 2025 12:08:07 +0100
Message-ID: <CAOWwgpk=V=wg3OZC2nx_MNpU652XH7e30+x8y3fCsvTR6z_Etw@mail.gmail.com>
Subject: Re: [PHP-DEV] [RFC] [Discussion] Add WHATWG compliant URL parsing API
To: internals@lists.php.net
Content-Type: multipart/alternative; boundary="000000000000a64f0c062ee15ba2"
From: nicolas.grekas+php@gmail.com (Nicolas Grekas)

--000000000000a64f0c062ee15ba2
Content-Type: text/plain; charset="UTF-8"

Hi,

Thanks for all the efforts making this RFC happen, it'll be a game changer
in the domain!

I'm seeing a push to make the classes final. Please don't!
This would badly break the open/closed principle to me.

When shipping a new class, one ships two things: a behavior and a type. The
behavior is what some want to close by making the class final. But the
result is that the type will also be final. And this would lead to a
situation where people tighly couple their code to one single
implementation - the internal one.

The situation I'm telling about is when one will accept an argument
described as
function (\Uri\WhatWg\Url $url)

If the Url class is final, this signature means only one possible
implementation can ever be passed: the native one. Composition cannot be
achieve because there's no type to compose.

Fine-tuning the behavior provided by the RFC is what we might be most
interested in, but we should not forget that we also ship a type. By making
the type non-final, we keep things open enough for userland to build on it.
If not, we're going to end up with a fragmented community: some will
tightly couple to the native Url implementation, some others will define a
UriInterface of their own and will compose it with the native
implementation, all these with non-interoperable base types of course,
because interop is hard.

By making the classes non-final, there will be one base type to build upon
for userland.
(the alternative would be to define native UrlInterface, but that'd
increase complexity for little to no gain IMHO - althought that'd solve my
main concern).


> 5 - Can the returned array from __debugInfo be used in a "normal"
> > method like `toComponents` naming can be changed/improve to ease
> > migration from parse_url or is this left for userland library ?
>
> I would prefer not expose this functionality for the same reason that
> there are no raw properties provided: The user must make an explicit
> choice whether they are interested in the raw or in the normalized
> version of the individual components.
>

The RFC is also missing whether __debugInfo returns raw or non-raw
components. Then, I'm wondering if we need this per-component break for
debugging at all? It might be less confusing (on this encoding aspect) to
dump basically what __serialize() returns (under another key than __uri of
course).
This would also close the avenue of calling __debugInfo() directly (at the
cost of making it possibly harder to move away from parse_url(), but I
don't think we need to make this simpler - getting familiar with the new
API before would be required and welcome actually.)



> It can make sense to normalize a hostname, but not the path. My usual
> example against normalizing the path is that SAML signs the *encoded*
> URI instead of the payload and changing the case in percent-encoded
> characters is sufficient to break the signature


I would be careful with this argument: signature validation should be done
on raw bytes. Requiring an object to preserve byte-level accuracy while the
very purpose of OOP is to provide abstractions might be conflicting. The
signing topic can be solved by keeping the raw signed payload around.

--000000000000a64f0c062ee15ba2
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,</div><div dir=3D"ltr=
"><br></div><div>Thanks for all the efforts making this RFC happen, it&#39;=
ll be a game changer in the domain!</div><div class=3D"gmail_quote"><div di=
r=3D"ltr" class=3D"gmail_attr"><br></div><div class=3D"gmail_attr">I&#39;m =
seeing a push to make the classes final. Please don&#39;t!</div><div class=
=3D"gmail_attr">This would badly break the open/closed principle to me.</di=
v><div class=3D"gmail_attr"><br></div><div class=3D"gmail_attr">When shippi=
ng a new class, one ships two things: a behavior and a type. The behavior i=
s what some want to close by making the class final. But the result is that=
 the type will also be final. And this would lead to a situation where peop=
le tighly couple their code to one single implementation - the internal one=
.</div><div class=3D"gmail_attr"><br></div><div class=3D"gmail_attr">The si=
tuation I&#39;m telling about is when one will accept an argument described=
 as</div><div class=3D"gmail_attr">function (\Uri\WhatWg\Url $url)</div><di=
v class=3D"gmail_attr"><br></div><div class=3D"gmail_attr">If the Url class=
 is final, this signature means only one possible implementation can ever b=
e passed: the native one. Composition cannot be achieve because there&#39;s=
 no type to compose.</div><div class=3D"gmail_attr"><br></div><div class=3D=
"gmail_attr">Fine-tuning the behavior provided by the RFC is what we might =
be most interested in, but we should not forget that we also ship a type. B=
y making the type non-final, we keep things open enough for userland to bui=
ld on it. If not, we&#39;re going to end up with a fragmented community: so=
me will tightly couple to the native Url implementation, some others will d=
efine a UriInterface of their own and will compose it with the native imple=
mentation, all these with non-interoperable base types of course, because i=
nterop is hard.</div><div class=3D"gmail_attr"><br></div><div class=3D"gmai=
l_attr">By making the classes non-final, there will be one base type to bui=
ld upon for userland.</div><div class=3D"gmail_attr">(the alternative would=
 be to define native UrlInterface, but that&#39;d increase complexity for l=
ittle to no gain IMHO - althought that&#39;d solve my main concern).</div><=
div class=3D"gmail_attr"><br></div><div dir=3D"ltr" class=3D"gmail_attr"><b=
r></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">&gt; 5 - Can the =
returned array from __debugInfo be used in a &quot;normal&quot; <br>
&gt; method like `toComponents` naming can be changed/improve to ease <br>
&gt; migration from parse_url or is this left for userland library ?<br>
<br>
I would prefer not expose this functionality for the same reason that <br>
there are no raw properties provided: The user must make an explicit <br>
choice whether they are interested in the raw or in the normalized <br>
version of the individual components.<br></blockquote><div><br></div><div>T=
he RFC is also missing whether __debugInfo returns raw or non-raw component=
s. Then, I&#39;m wondering if we need this per-component break for debuggin=
g at all? It might be less confusing (on this encoding aspect) to dump basi=
cally what=C2=A0__serialize() returns (under another key than __uri of cour=
se).</div><div>This would also close the avenue of calling __debugInfo() di=
rectly (at the cost of making it possibly harder to move away from parse_ur=
l(), but I don&#39;t think we need to make this simpler - getting familiar=
=C2=A0with the new API before would be required and welcome actually.)</div=
><div><br></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-le=
ft:1ex">It can make sense to normalize a hostname, but not the path. My usu=
al <br>
example against normalizing the path is that SAML signs the *encoded* <br>
URI instead of the payload and changing the case in percent-encoded <br>
characters is sufficient to break the signature</blockquote><div><br></div>=
<div>I would be careful with this argument: signature validation should be =
done on raw bytes. Requiring an object to preserve byte-level accuracy whil=
e the very purpose of OOP is to provide abstractions might be conflicting. =
The signing topic can be solved by keeping the raw signed payload around.</=
div><div><br></div><div>=C2=A0</div><div><br></div></div></div>
</div>

--000000000000a64f0c062ee15ba2--