Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:117795
Date: Thu, 26 May 2022 14:06:19 -0500
To: Internals <internals@lists.php.net>
Message-ID: <8f212d08-81f3-4e7b-ae68-82b04d74929b@Canary>
In-Reply-To: <2DC21BE3-558E-4530-B5D8-C5EE6F6CFEF3@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>
MIME-Version: 1.0
Content-Type: multipart/alternative; boundary="628fcfab_7c83e458_1804"
Subject: Re: [PHP-DEV] NULL Coercion Consistency
From: michael.babker@gmail.com (Michael Babker)

--628fcfab_7c83e458_1804
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: inline


On Thursday, May 26, 2022 at 11:41 AM, Craig =46rancis <craig=40craigfran=
cis.co.uk (mailto:craig=40craigfrancis.co.uk)> wrote:
> On 26 May 2022, at 15:01, Michael Babker <michael.babker=40gmail.com> w=
rote:
> > On Thu, May 26, 2022 at 7:21 AM Craig =46rancis <craig=40craigfrancis=
.co.uk> wrote:
> > > That said, I would still like to know about the benefits of rejecti=
ng NULL for =60htmlspecialchars()=60, in the context of that Laravel Blad=
e patch; because, if it is beneficial (vs the BC break), they should reve=
rt that patch... shouldn't they=3F
> >
> > No, they should not. Laravel made an explicit choice to handle null v=
alues in its escaping function for its templating framework. That is thei=
r choice to make. They should not be forced to implicitly handle null val=
ues because the language decides to add implicit type coercion to user de=
fined functions (because consistency seems to be so important to some), a=
nd they should not be forced to reject null values because underlying lan=
guage functions reject them as well.
>
>
> Erm, I'm simply asking - If there is a good reason for throwing an exce=
ption when NULL is passed to =60htmlspecialchars()=60, then that reason w=
ould also apply to Laravel Blade for it's HTML encoding.

Laravel=E2=80=99s =60e()=60 helper function is more than a wrapper around=
 =60htmlspecialchars()=60 which supports values beyond a string, and has =
elected to support a null value being passed into that function with expl=
icit handling for it. I=E2=80=99ve seen similar patches land in other pac=
kages as well. Whether it is to prevent that package from triggering the =
deprecation regarding null values itself or the package owners choosing t=
o explicitly handle the null case so users don=E2=80=99t need to deal wit=
h it doesn=E2=80=99t really matter, the point is folks have made that dec=
ision and it is not problematic in any way IMO. You=E2=80=99re basically =
saying that a framework choosing to apply the same =60htmlspecialchars(=24=
string =3F=3F =E2=80=98=E2=80=99)=60 patch that applications are suggeste=
d 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.

On Thursday, May 26, 2022 at 11:41 AM, Craig =46rancis <craig=40craigfran=
cis.co.uk (mailto:craig=40craigfrancis.co.uk)> wrote:
> > If a function, by explicit design, can safely work with null values t=
hen that parameter should be properly declared nullable.
>
>
> =60htmlspecialchars()=60 can and always has safely worked with NULL, th=
e same with the other 335 parameters I've listed. But when I suggested ch=
anging these parameters to be nullable, it was not well received.

The arginfo for the =60htmlspecialchars()=60 function, and if I=E2=80=99m=
 reading correctly (I=E2=80=99m no C developer so I could very well be mi=
sinterpreting things) the internal =60php=5Fhtml=5Fentities()=60 function=
 which =60htmlspecialchars()=60 calls, does not declare null as a support=
ed value for the =60=24string=60 parameter. Meaning that depending on whe=
ther your code uses strict=5Ftypes or not, either calling the function wi=
th a null value triggers a type error or requires implicit type coercisio=
n to convert the value to a type that the function does support. This mea=
ns to me that the =60htmlspecialchars()=60 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 er=
ror to be raised; the code very explicitly requires a string and is not w=
ritten to process a null value.

A blanket suggestion to make every string parameter which implicitly supp=
orts a null value nullable because they previously supported null through=
 type coercion IMO should be shot down. Instead, parameters which are emi=
tting a deprecation notice because they are receiving an unsupported null=
 value should be reviewed, and if that function can by design support nul=
l values, those parameters should be updated to reflect nullability. IMO,=
 =60htmlspecialchars()=60 is not a function which should expliciitly supp=
ort a null value and the present deprecation is correct.

On Thursday, May 26, 2022 at 11:41 AM, Craig =46rancis <craig=40craigfran=
cis.co.uk (mailto:craig=40craigfrancis.co.uk)> wrote:
> On 26 May 2022, at 15:01, Michael Babker <michael.babker=40gmail.com> w=
rote:
> > Yes, that means that there is a lot of work involved for folks betwee=
n now and the release of PHP 9.0 to either refactor code to avoid type er=
rors 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 r=
un to ensure language consistency (I really don=E2=80=99t think userland =
PHP and internal/extension PHP should have vastly different behaviors, an=
d type coercion support based on where a function is designed is one of t=
hose holes that needs filled) and to ensure all APIs explicitly declare w=
hat types of data they support.
>
>
>
> I don't think userland and internal functions should have different beh=
aviours either (my R=46C does say =22keep user-defined and internal funct=
ions consistent=22).
>
> To achieve that, for those not using =60strict=5Ftypes=3D1=60, I'm simp=
ly suggesting that NULL coercion continues to work for internal functions=
, and we update user-defined functions to allow NULL coercion.

On the record, I personally disagree with the strict=5Ftypes 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 bypa=
ss a developer=E2=80=99s explicit opt-in to strict=5Ftypes being enabled =
which makes it a =E2=80=9Cmostly strict but there=E2=80=99s still a gotch=
a=E2=80=9D declaration at best. I don=E2=80=99t think this type of deep r=
ooted behavioral difference improves PHP in any way, but that ship has lo=
ng sailed.

With that said though, I disagree with exposing the internal type coercio=
n 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-insig=
nificant 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 b=
ecause they=E2=80=99re using non-analyzed code as a foundation. Changing =
the language in a way that makes these already fragile platforms even mor=
e suseptable to hidden type-related bugs is a bad idea. Personally speaki=
ng, I=E2=80=99ve been bitten by enough type-coercion related bugs in my c=
areer (and continue to address new issues arise thanks to a combination o=
f poorly formed legacy data in the database plus open source packages bec=
oming stricter in the types they support) that I would rather advocate fo=
r the language moving in a direction that makes type-related issues harde=
r to hit and it to be noisy when it does encounter such issues; the chang=
es in PHP 8.1 IMO are a step in the right direction.
--628fcfab_7c83e458_1804--