Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:126965
Content-Type: multipart/alternative;
 boundary="------------0tE44KDqB04ouNlCDUqpm5ED"
Message-ID: <f5952335-898a-4707-aa6e-7668b678ff13@gmail.com>
Date: Thu, 27 Mar 2025 23:49:39 +0100
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PHP-DEV] [RFC] [Discussion] Add WHATWG compliant URL parsing API
To: =?UTF-8?B?TcOhdMOpIEtvY3Npcw==?= <kocsismate90@gmail.com>
Cc: PHP Internals List <internals@lists.php.net>
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>
 <CAH5C8xWnoytE=jm1sxr-HcSZFzmG8SrzghinmBZa-QyGqN4uUg@mail.gmail.com>
 <6430b9ed-638d-4247-9fa9-d1a9148c382b@gmail.com>
 <CAH5C8xX3tCS3B3z2YFbRudCXi=ZNHE0scrmoFO7cXxU773EfpA@mail.gmail.com>
 <2e95e8fe-7cf0-493f-bd0a-9fff0956baaa@gmail.com>
 <CAH5C8xXuSEBx6y4ds8bhReFFZe0i05DFonvEP+sz+mdkembZtQ@mail.gmail.com>
Content-Language: fr
In-Reply-To: <CAH5C8xXuSEBx6y4ds8bhReFFZe0i05DFonvEP+sz+mdkembZtQ@mail.gmail.com>
From: nyamsprod@gmail.com (Ignace Nyamagana Butera)

This is a multi-part message in MIME format.
--------------0tE44KDqB04ouNlCDUqpm5ED
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit


On 27/03/2025 22:04, Máté Kocsis wrote:
> Hi Ignace,
>
>     While implementing the polyfill I am finding easier DX wise to
>     make the constructor private and use instead named constructors
>     for instantiation. I would be in favor of
>
>     `Uri::parse` and `Uri::tryParse` like it is done currently with
>     Enum and the `from` and `tryfrom` named constructors.
>
>     My reasoning is as follow:
>
>      there's no right way or wrong way to instantiate an URI there are
>     only contexts. While the parse method is all about parsing a
>     string, one could legitimately use other named constructors like
>     `Uri::fromComponents` which would take for instance the result of
>     parse_url to build a new URI. This can become handy in the case of
>     RFC3986 URI if you need to create an new URI not related to the
>     http scheme and that do not use all the components like the email,
>     data or FTP schemes.
>
>      By allowing creating URI based on their respective components
>     value you make it easier for dev to use the class. Also this means
>     that if we want to have a balance API then a `toComponents` method
>     should come hand in hand with the named constructor.
>
>     I would understand if that idea to add both components related
>     methods is rejected, they could be implemented on userland, but
>     the main point was to prove that from the VO or the developer POV
>     in absence of a clearly defined instantiation process, having a
>     traditional constructor fails to convey all the different way to
>     create an URI.
>
> There are a few things which came to my mind:
> - Currently, the underlying C libraries don't support a 
> `fromComponents` feature. How I could naively imagine this to work is 
> that the components are recomposed to a URI string based on the 
> relevant algorithm (for RFC 3986: 
> https://datatracker.ietf.org/doc/html/rfc3986#section-5.3), and then 
> this string is parsed and validated. Unfortunately, I recently 
> realized that this approach may leave room for some kind of parsing 
> confusion attack, namely when the scheme is for example "https", the 
> authority is empty, and the path is "example.com 
> <http://example.com>". This will result in a https://example.com URI. 
> I believe a similar bug is not possible with the rest of the 
> components because they have their delimiters. So possibly some other 
> solution will be needed, or maybe adding some additional validation (?).
>
> - Nicolas raised my awareness that if URIs didn't have a proper 
> constructor, then one wouldn't be able to use URI objects as parameter 
> default values, like below:
> function (Uri $foo = new Uri('blah'))
> I think this omission would cause some usability regression. For this 
> reason, it may make sense to have a distinguished way of instantiating 
> an Uri.
>
> - I have a similar feeling for a toComponents() method as for another 
> named constructor instead of __construct(): I am not completely 
> against it, but I'm not totally convinced about it.
>
> Máté
>
Hi Máté,

    for RFC 3986:
    https://datatracker.ietf.org/doc/html/rfc3986#section-5.3), and then
    this string is parsed and validated. Unfortunately, I recently
    realized that this approach may leave room for some kind of parsing
    confusion attack, namely when the scheme is for example "https", the
    authority is empty, and the path is "example.com
    <http://example.com>". This will result in a https://example.com
    URI. I believe a similar bug is not possible with the rest of the
    components because they have their delimiters. So possibly some
    other solution will be needed, or maybe adding some additional
    validation (?).

This is not correct according to RFC3986 
https://datatracker.ietf.org/doc/html/rfc3986#section-3


*When authority is present, the path must either be empty or begin with 
a slash ("/") character. When authority is not present, the path cannot 
begin with two slash characters ("//"). *

So in your example it should throw an Uri\InvalidUriException 🙂 for RFC3986 and in case of the WhatwgUrl algorithm it should trigger a soft error and correct the behaviour for the http(s) schemes.
This is also one of the many reasons why at least for RFC3986 the path component can never be `null` but that's another discussion. Like I said having a `fromComponenta` named constructor would allow the "removal" of the need for a UriBuilder (in your future section) and would IMHO be useful outside of the context of the http(s) scheme but I can understand it being left out of the current implementation it might be brought back for future improvements.

I have one last question regarding the URI implementations which are raised by my polyfill:

Did you also took into account the delimiters when submitting data via the withers ? In other words is

```php
$uri->withQuery('?foo=bar');
//the same as
$uri->withQuery('foo=bar');
```

I know it is the case in of the WHATWG specification but I do not know if you kept this behaviour in your implementation for the WhatWgUrl for the Rfc3986 or for both. I would lean toward not accepting this "normalization" but since this is not documented in the RFC I wanted to know what is the expected behaviour.

Thanks for the hard work



--------------0tE44KDqB04ouNlCDUqpm5ED
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: 8bit

<!DOCTYPE html>
<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body>
    <p><br>
    </p>
    <div class="moz-cite-prefix">On 27/03/2025 22:04, Máté Kocsis wrote:<br>
    </div>
    <blockquote type="cite"
cite="mid:CAH5C8xXuSEBx6y4ds8bhReFFZe0i05DFonvEP+sz+mdkembZtQ@mail.gmail.com">
      <meta http-equiv="content-type" content="text/html; charset=UTF-8">
      <div dir="ltr">
        <div dir="ltr">Hi Ignace,</div>
        <div dir="ltr"><br>
        </div>
        <div class="gmail_quote gmail_quote_container">
          <blockquote class="gmail_quote"
style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
            <div>
              <p>While implementing the polyfill I am finding easier DX
                wise to make the constructor private and use instead
                named constructors for instantiation. I would be in
                favor of </p>
              <p>`Uri::parse` and `Uri::tryParse` like it is done
                currently with Enum and the `from` and `tryfrom` named
                constructors.<br>
              </p>
              <p>My reasoning is as follow:</p>
              <p> there's no right way or wrong way to instantiate an
                URI there are only contexts. While the parse method is
                all about parsing a string, one could legitimately use
                other named constructors like `Uri::fromComponents`
                which would take for instance the result of parse_url to
                build a new URI. This can become handy in the case of
                RFC3986 URI if you need to create an new URI not related
                to the http scheme and that do not use all the
                components like the email, data or FTP schemes.</p>
              <p> By allowing creating URI based on their respective
                components value you make it easier for dev to use the
                class. Also this means that if we want to have a balance
                API then a `toComponents` method should come hand in
                hand with the named constructor.</p>
              <p>I would understand if that idea to add both components
                related methods is rejected, they could be implemented
                on userland, but the main point was to prove that from
                the VO or the developer POV in absence of a clearly
                defined instantiation process, having a traditional
                constructor fails to convey all the different way to
                create an URI.</p>
            </div>
          </blockquote>
          <div> </div>
          <div>There are a few things which came to my mind:</div>
          <div>- Currently, the underlying C libraries don't support a
            `fromComponents` feature. How I could naively imagine this
            to work is that the components are recomposed to a URI
            string based on the relevant algorithm (for RFC 3986: <a
href="https://datatracker.ietf.org/doc/html/rfc3986#section-5.3"
              moz-do-not-send="true" class="moz-txt-link-freetext">https://datatracker.ietf.org/doc/html/rfc3986#section-5.3</a>),
            and then this string is parsed and validated. Unfortunately,
            I recently realized that this approach may leave room for
            some kind of parsing confusion attack, namely when the
            scheme is for example "https", the authority is empty, and
            the path is "<a href="http://example.com"
              moz-do-not-send="true">example.com</a>". This will result
            in a <a href="https://example.com" moz-do-not-send="true"
              class="moz-txt-link-freetext">https://example.com</a> URI.
            I believe a similar bug is not possible with the rest of the
            components because they have their delimiters. So possibly
            some other solution will be needed, or maybe adding some
            additional validation (?).</div>
          <div><br>
          </div>
          <div>- Nicolas raised my awareness that if URIs didn't have a
            proper constructor, then one wouldn't be able to use URI
            objects as parameter default values, like below:</div>
          <div>function (Uri $foo = new Uri('blah'))</div>
          <div>I think this omission would cause some usability
            regression. For this reason, it may make sense to have a
            distinguished way of instantiating an Uri.</div>
          <div><br>
          </div>
          <div>- I have a similar feeling for a toComponents() method as
            for another named constructor instead of __construct(): I am
            not completely against it, but I'm not totally convinced
            about it.</div>
          <div><br>
          </div>
          <div>Máté</div>
          <div><br>
          </div>
        </div>
      </div>
    </blockquote>
    <p>Hi Máté,</p>
    <blockquote>
      <p>for RFC 3986: <a
href="https://datatracker.ietf.org/doc/html/rfc3986#section-5.3"
          class="moz-txt-link-freetext">https://datatracker.ietf.org/doc/html/rfc3986#section-5.3</a>),
        and then this string is parsed and validated. Unfortunately, I
        recently realized that this approach may leave room for some
        kind of parsing confusion attack, namely when the scheme is for
        example "https", the authority is empty, and the path is "<a
          href="http://example.com">example.com</a>". This will result
        in a <a href="https://example.com"
          class="moz-txt-link-freetext">https://example.com</a> URI. I
        believe a similar bug is not possible with the rest of the
        components because they have their delimiters. So possibly some
        other solution will be needed, or maybe adding some additional
        validation (?).</p>
    </blockquote>
    <p>This is not correct according to RFC3986
      <a class="moz-txt-link-freetext" href="https://datatracker.ietf.org/doc/html/rfc3986#section-3">https://datatracker.ietf.org/doc/html/rfc3986#section-3</a></p>
    <p><br>
    </p>
    <pre class="newpage"
style="box-sizing: border-box; font-family: var(--bs-font-monospace); font-size: 16px; margin: -1.25em 0px 0px; display: block; overflow: visible; padding: 0px; width: 80ch; white-space: pre-wrap; color: rgb(33, 37, 41); font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(255, 255, 255); text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial;"><b>When authority is present, the path must either be empty or begin with a slash ("/") character.  When authority is not present, the path cannot begin with two slash characters ("//"). </b>

So in your example it should throw an Uri\InvalidUriException 🙂 for RFC3986 and in case of the WhatwgUrl algorithm it should trigger a soft error and correct the behaviour for the http(s) schemes. 
This is also one of the many reasons why at least for RFC3986 the path component can never be `null` but that's another discussion. Like I said having a `fromComponenta` named constructor would allow the "removal" of the need for a UriBuilder (in your future section) and would IMHO be useful outside of the context of the http(s) scheme but I can understand it being left out of the current implementation it might be brought back for future improvements.

I have one last question regarding the URI implementations which are raised by my polyfill:

Did you also took into account the delimiters when submitting data via the withers ? In other words is

```php
$uri-&gt;withQuery('?foo=bar');
//the same as 
$uri-&gt;withQuery('foo=bar');
```

I know it is the case in of the WHATWG specification but I do not know if you kept this behaviour in your implementation for the WhatWgUrl for the Rfc3986 or for both. I would lean toward not accepting this "normalization" but since this is not documented in the RFC I wanted to know what is the expected behaviour.

Thanks for the hard work 


</pre>
    <p></p>
    <p><br>
    </p>
  </body>
</html>

--------------0tE44KDqB04ouNlCDUqpm5ED--