Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:117808
Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3696.100.31\))
In-Reply-To: <8f212d08-81f3-4e7b-ae68-82b04d74929b@Canary>
Date: Sat, 28 May 2022 03:35:45 +0100
Cc: Internals <internals@lists.php.net>
Content-Transfer-Encoding: quoted-printable
Message-ID: <0AE752E6-312B-4639-AC1C-1ED201DAF627@craigfrancis.co.uk>
References: <CAFv4g+Ej4JbM9SzQ0iU-d8C4T+2XNo=8jnJ1uwfQ=skZZXx9Tg@mail.gmail.com>
 <1755E8B5-229B-47B2-BBAF-B5E014F5473D@craigfrancis.co.uk>
 <1180af01-080f-ee0a-3159-74bf7e0a8aea@gmail.com>
 <73F563E2-7C31-4B5E-A6B9-AE1BD05ADD1C@craigfrancis.co.uk>
 <CANvqVdrmVOwKaMxsO6Awbr0KSpc=QjqNFwFivKphdT_KAZRcSg@mail.gmail.com>
 <2DC21BE3-558E-4530-B5D8-C5EE6F6CFEF3@craigfrancis.co.uk>
 <8f212d08-81f3-4e7b-ae68-82b04d74929b@Canary>
To: Michael Babker <michael.babker@gmail.com>
Subject: Re: [PHP-DEV] NULL Coercion Consistency
From: craig@craigfrancis.co.uk (Craig Francis)

On 26 May 2022, at 20:06, Michael Babker <michael.babker@gmail.com> =
wrote:
> On Thursday, May 26, 2022 at 11:41 AM, Craig Francis =
<craig@craigfrancis.co.uk (mailto:craig@craigfrancis.co.uk)> wrote:
>> [...] If there is a good reason for throwing an exception when NULL =
is passed to `htmlspecialchars()`, then that reason would also apply to =
Laravel Blade for it's HTML encoding.
>=20
> Laravel=E2=80=99s `e()` helper function is more than a wrapper around =
`htmlspecialchars()` which supports values beyond a string, and has =
elected to support a null value being passed into that function with =
explicit handling for it. I=E2=80=99ve seen similar patches land in =
other packages as well. Whether it is to prevent that package from =
triggering the deprecation regarding null values itself or the package =
owners choosing to explicitly handle the null case so users don=E2=80=99t =
need to deal with it doesn=E2=80=99t really matter, the point is folks =
have made that decision and it is not problematic in any way IMO. =
You=E2=80=99re basically saying that a framework choosing to apply the =
same `htmlspecialchars($string ?? =E2=80=98=E2=80=99)` patch that =
applications are suggested to use (for those who don=E2=80=99t care to =
differentiate between null and empty string before something reaches an =
escaping function) is in the wrong by arguing for Laravel to revert that =
patch.


Sorry, I've read this a few times, and I just don't understand what you =
mean... but let's leave it, I cannot think how else to re-write my =
question to get an answer.



On 26 May 2022, at 20:06, Michael Babker <michael.babker@gmail.com> =
wrote:
> On Thursday, May 26, 2022 at 11:41 AM, Craig Francis =
<craig@craigfrancis.co.uk (mailto:craig@craigfrancis.co.uk)> wrote:
>>=20
>> `htmlspecialchars()` can and always has safely worked with NULL, the =
same with the other 335 parameters I've listed. But when I suggested =
changing these parameters to be nullable, it was not well received.
>=20
> The arginfo for the `htmlspecialchars()` function, and if I=E2=80=99m =
reading correctly (I=E2=80=99m no C developer so I could very well be =
misinterpreting things) the internal `php_html_entities()` function =
which `htmlspecialchars()` calls, does not declare null as a supported =
value for the `$string` parameter. Meaning that depending on whether =
your code uses strict_types or not, either calling the function with a =
null value triggers a type error or requires implicit type coercision to =
convert the value to a type that the function does support. This means =
to me that the `htmlspecialchars()` function does NOT support null =
values and that passing null through only works because of the reliance =
on implicit type coercision. That, to me, is a valid reason for a type =
error to be raised; the code very explicitly requires a string and is =
not written to process a null value.
>=20
> A blanket suggestion to make every string parameter which implicitly =
supports a null value nullable because they previously supported null =
through type coercion IMO should be shot down. Instead, parameters which =
are emitting a deprecation notice because they are receiving an =
unsupported null value should be reviewed, and if that function can by =
design support null values, those parameters should be updated to =
reflect nullability. IMO, `htmlspecialchars()` is not a function which =
should expliciitly support a null value and the present deprecation is =
correct.


Erm, I really don't wish to be rude, but have you read my RFC, and the =
problem I'm trying to solve... in short, the backwards compatibility =
issue for PHP 9.0.

Developers will need to spend a lot of time updating their code, which =
is important to avoid the fatal type errors, but I simply do not =
understand what the benefit for them will be.



On 26 May 2022, at 20:06, Michael Babker <michael.babker@gmail.com> =
wrote:
> On Thursday, May 26, 2022 at 11:41 AM, Craig Francis =
<craig@craigfrancis.co.uk (mailto:craig@craigfrancis.co.uk)> wrote:
>> On 26 May 2022, at 15:01, Michael Babker <michael.babker@gmail.com> =
wrote:
>>> Yes, that means that there is a lot of work involved for folks =
between now and the release of PHP 9.0 to either refactor code to avoid =
type errors or to get the language to expand the supported types in =
appropriate functions, but IMO that effort from all parties is worth it =
in the long run to ensure language consistency (I really don=E2=80=99t =
think userland PHP and internal/extension PHP should have vastly =
different behaviors, and type coercion support based on where a function =
is designed is one of those holes that needs filled) and to ensure all =
APIs explicitly declare what types of data they support.
>>=20
>>=20
>>=20
>> I don't think userland and internal functions should have different =
behaviours either (my RFC does say "keep user-defined and internal =
functions consistent").
>>=20
>> To achieve that, for those not using `strict_types=3D1`, I'm simply =
suggesting that NULL coercion continues to work for internal functions, =
and we update user-defined functions to allow NULL coercion.
>=20
> On the record, I personally disagree with the strict_types =
declaration=E2=80=99s existence purely because it makes the runtime =
inconsistent based solely on what file something was called from, plus =
there are way to bypass a developer=E2=80=99s explicit opt-in to =
strict_types being enabled which makes it a =E2=80=9Cmostly strict but =
there=E2=80=99s still a gotcha=E2=80=9D declaration at best. I don=E2=80=99=
t think this type of deep rooted behavioral difference improves PHP in =
any way, but that ship has long sailed.


I believe George Peter Banyard is trying to work on this.



> With that said though, I disagree with exposing the internal type =
coercion behavior to userland code. I think the userland behavior as it =
is today is the correct behavior, even if I look at implicit type =
coercision as a bug which will cause hard-to-identify production issues =
for somebody at some point down the road. Let=E2=80=99s also be real =
here, a not-so-insignificant number of PHP builds are powered by =
platforms which have minimal if any automated testing and even less =
static analysis, meaning a static analyzer is not going to be of much =
help to those builds to begin with because they=E2=80=99re using =
non-analyzed code as a foundation. Changing the language in a way that =
makes these already fragile platforms even more suseptable to hidden =
type-related bugs is a bad idea. Personally speaking, I=E2=80=99ve been =
bitten by enough type-coercion related bugs in my career (and continue =
to address new issues arise thanks to a combination of poorly formed =
legacy data in the database plus open source packages becoming stricter =
in the types they support) that I would rather advocate for the language =
moving in a direction that makes type-related issues harder to hit and =
it to be noisy when it does encounter such issues; the changes in PHP =
8.1 IMO are a step in the right direction.


Sorry, but I'm finding this hard to follow, it sounds like you don't =
want any type coercion... e.g. the string "5" to int 5 should just not =
happen without the developer explicitly converting the type (e.g. =
intval)... I disagree, but we can leave it at that.

Craig