Allowing NULL for some internal functions

3 years ago by Craig Francis — view source — reply

unread

On Thu, Dec 2, 2021 at 8:48 AM Craig Francis craig@craigfrancis.co.uk
wrote:

Is there any value in me proposing an RFC to update some internal
functions so they can accept NULL?

I'm not hard against this idea. The interpretation of null in these
contexts as being equivalent to empty string isn't unreasonable. I guess
the only objection I could have would be an academic one and I can't really
defend that. So yeah, sure... why not?

Thanks Sara,

I've spent a few days coming up with a short(ish) list of parameters that
are likely to receive NULL.

The focus is on making it easier for developers to upgrade to newer
versions of PHP. I'm thinking of the typical developer, probably using
WordPress, isn't using Psalm (at levels 1 to 3, with no baseline), and
isn't using strict_types; where they don't really have the time to add
strval() to everything that could be NULL.

Draft RFC:
https://wiki.php.net/rfc/allow_null

And the list, where I'm proposing we only relax this requirement for the
bold parameters:
https://github.com/craigfrancis/php-allow-null-rfc/blob/main/functions-change.md

I'll give it a few weeks to see if there are any parameter suggestions
(welcome via email, pull request, carrier pigeon, etc), then I'll start
looking for the best way to implement this.

Craig

PS; starting new thread, as I have gone a bit off topic, original one at:
https://externals.io/message/116519#116556

3 years ago by AllenJB — view source — reply

unread

I've spent a few days coming up with a short(ish) list of parameters that
are likely to receive NULL.

The focus is on making it easier for developers to upgrade to newer
versions of PHP. I'm thinking of the typical developer, probably using
WordPress, isn't using Psalm (at levels 1 to 3, with no baseline), and
isn't using strict_types; where they don't really have the time to add
strval() to everything that could be NULL.

Draft RFC:
https://wiki.php.net/rfc/allow_null

And the list, where I'm proposing we only relax this requirement for the
bold parameters:
https://github.com/craigfrancis/php-allow-null-rfc/blob/main/functions-change.md

I'll give it a few weeks to see if there are any parameter suggestions
(welcome via email, pull request, carrier pigeon, etc), then I'll start
looking for the best way to implement this.

Craig

PS; starting new thread, as I have gone a bit off topic, original one at:
https://externals.io/message/116519#116556

This RFC has come out of a similar discussion around deprecating dynamic
properties (and this isn't the first time "upgrading is hard" / "the
language is moving too fast" has come up), so it's not just this one
change that's affected.

I think this RFC is trying to solve the wrong problem / only fixing one
small portion of the "immediate" problem without paying attention to the
bigger picture.

Alternative solutions:

I feel that effort would be better spent on improving and encouraging
use of tooling such as the PHPCompatibility CodeSniffer ruleset (which
has unfortunately fallen behind for changes in PHP 8+) and Rector to
improve the upgrading process for everyone. I recognize that there's
arguments against the project being seen to endorse / contribute to
specific tools / libraries / frameworks, but I feel that in this area
the benefits outweigh the costs.

The project could provide guides (or link to selected existing articles)
for upgrading older code. Whilst significant documentation does already
exist on this it's not always easy to find or contains errors (a lot of

probably copy-paste - blog articles that I've seen focus on
PHPCompatibility for upgrading to 8.0 / 8.1 even tho its coverage of the
changes in these versions is poor). This could be done via something
like a user-contributed wiki similar to what Python and other projects
do: https://wiki.python.org/moin/FrontPage - I recognize that there's
significant work in this suggestion, but I feel it also offers good
opportunities for further improvements to the PHP ecosystem / community.

The migration guides could include documentation on updating existing
code for each change (could / should more be done to encourage this
being done at the time the change is made rather than trying to compile
migration docs just before release?) - they do this sometimes but I feel
this is an area with room for improvement.

The above mentioned alternatives solve the entire class of problem
whilst also maintaining PHP's work towards being a stricter language
that's that much harder to create bugs with and needs less "boilerplate"
for more defensive programming. Which leads to the argument that in
trying to solve a one-time problem for some developers, this RFC creates
work for developers both in debugging code, and for developers with
defensive programming styles to (re-)add checks to detect the exact
class of problem these deprecations / warnings highlight.

Developers also already have tools to selectively silence deprecation
notices using custom error handlers.

On the proposed changes / RFC content:

The RFC uses retrieving request parameters (get/post/cookie) values
using various frameworks as a (primary) example. With the exception of
CodeIgniter (it may be possible, but wasn't immediately obvious from the
quickest of glances at their docs) all the frameworks used in the
example have an additional default value parameter. This makes all of
these (and the null coalesce alternative) relatively trivial to update
using a regular expression find and replace, that most text editors can do.

The RFC focuses on changing string parameters, but uses
mysqli_fetch_row(), json_decode() and error_get_last() in its arguments

none of which are usually used to return a string (or anything you'd
want to treat as a string). Accessing properties of non-objects (since
PHP 5.0) and accessing non-existant array indexes (since PHP 7.4) have
been warnings / notices for a signifcant time already.

Some of the suggested functions (parameters) to change are strange
selections in my opinion - gz functions, bcmath parameters,
password_hash() and mail() (subject / message) are not usually values
you want to pass either null or an empty value to (and are almost
certainly bugs if the code does that). (And similarly from the maybe
list, I would apply this to most of the date_create functions, dns
functions hostname and logging functions message parameter)

If this change were to be made, I think it would be better to severely
restrict the list to only the most commonly used functions. How many
codebases affected by the null parameter deprecation are using sodium or
MessageFormatter? While this might leave some work for developers
upgrading codebases, I feel the long list of functions affected by this
RFC (even without considering the maybe list) goes too far.

How would code that declares strict_types be affected by this RFC? This
isn't clearly defined in the RFC. I would prefer it if these function
continue to not accept null (and throw a TypeError) if strict_types is
declared.

3 years ago by Craig Francis — view source — reply

unread

Thanks Allen,

Before I start, the RFC is still a Draft, and I'm trying to find a
solution... I'll also use a bit of sarcasm, just to be entertaining/funny,
so it's not a page of boring technical stuff, I'm not arguing or saying
you're wrong, and I genuinely appreciate your views on this.

I think this RFC is trying to solve the wrong problem / only fixing one
small portion of the "immediate" problem without paying attention to the
bigger picture.

Dynamic properties do cause problems, and I'm convinced they should
deprecated, whereas NULL is a useful value.

e.g. when a value has not been set; and then providing NULL to a function
like htmlspecialchars(), I don't think should be a problem.

I feel that effort would be better spent on improving and encouraging

use of tooling such as the PHPCompatibility CodeSniffer ruleset.

This one requires variable tracing from source to sink, not an easy task
(it's why I noted "Psalm at level 3, with no baseline").

Neither PHPCompatibility or Rector have rules for this at the moment, and
I'm pretty sure they never will; other than the really obvious ones, e.g.
str_replace('a', NULL, $name), which is clearly a terrible thing to do :-)

As an aside, PHPCompatibility did the bad thing of using trim() on
getConfigData(), which can return NULL:

https://github.com/PHPCompatibility/PHPCompatibility/commit/ab7b0b82d95c82e62f9cffb0f1960f3ccbf0805e

https://github.com/squizlabs/PHP_CodeSniffer/blob/d6a58bdd8707bab1032344f33d01c0627e0f3f97/src/Config.php#L1479

As noted in the RFC, while these don't take long to fix, finding them is
tricky, it nearly always results in more code (yay?), and it is being seen
as wasting time on a fairly questionable improvement.

The migration guides could include documentation on updating existing

code for each change

While migration guides are useful (I appreciate them), I'm focusing on this
change to avoid unnecessary work. I don't think nullable variables should
need to be explicitly cast to a string before calling these functions -
doing so doesen't seem to make the code better (if anything, it's making it
more complex).

... this RFC creates work for developers both in debugging code, and for

developers with
defensive programming styles to (re-)add checks to detect the exact
class of problem these deprecations / warnings highlight.

For example?

e.g. $name, if it's set to NULL, why should ucwords() have a problem
with that? Won't it cause extra work having to explicitly convert it to a
string?

Consider those who aren't using Static Analysis (or they are using the
entry levels, with a baseline), it requires that everyone remembers that
while their HTML form would normally provide this value, the user can edit
the <form> in their browser DOM, or a browser extension could do something
weird (e.g. a password manager), or a network issue results in a partial
page load... and kaboom, future Fatal TypeError in the middle of
processing? yay? party time?

...all the frameworks used in the example have an additional

default value parameter. This makes all of these (and the null
coalesce alternative) relatively trivial to update using a regular
expression find and replace, that most text editors can do.

They use a default value of NULL so you can determine when the value has
not been provided... should we tell them to stop doing this? default to
returning an empty string instead? I'm not sure they will accept that one.

Some of the suggested functions (parameters) to change are strange

selections in my opinion

It's a draft, I just started with as many candidates as I could find, on
the basis that it's easier to cut back if NULL can cause a problem.

But as to your examples (gz compressing, bcmath, password_hash, and mail);
they can all be provided with user input, and other sources of NULL.

e.g. password_hash(), while I wouldn't advise anyone to use a blank
password, or anything less than 8 characters, this function does accept it.
And because passwords often come from a POST request, if the value is not
present (framework function returns NULL)... well, assuming this
deprecation indicates a future where this is not acceptable, what happens?
future Fatal TypeError?

This is why I have the "Future Scope" section - noting that some parameters
could be updated (in a different RFC) to complain when an Empty String or
NULL is provided. e.g. setcookie() already complains when $name is
empty, and strpos() probably should complain when $needle is empty.

If this change were to be made, I think it would be better to severely
restrict the list to only the most commonly used functions.

I'm fine with that, but what should be removed? Feel free to do this on or
off-list, pull request, etc. If you can, I'd really appreciate examples
where the NULL would cause a problem but an empty string is fine - i.e.
assume developers will be lazy, and just use strval() to fix the hideous
crime they have been committing.

How would code that declares strict_types be affected by this RFC?

Good question... personally I think a trim() on NULL should return an empty
string, not a fatal error, so maybe we are looking at this problem in a
slightly different way?

Craig

3 years ago by Christoph M. Becker — view source — reply

unread

On Thu, Dec 2, 2021 at 8:48 AM Craig Francis craig@craigfrancis.co.uk
wrote:

Is there any value in me proposing an RFC to update some internal
functions so they can accept NULL?

I'm not hard against this idea. The interpretation of null in these
contexts as being equivalent to empty string isn't unreasonable. I guess
the only objection I could have would be an academic one and I can't really
defend that. So yeah, sure... why not?

I've spent a few days coming up with a short(ish) list of parameters that
are likely to receive NULL.

The focus is on making it easier for developers to upgrade to newer
versions of PHP. I'm thinking of the typical developer, probably using
WordPress, isn't using Psalm (at levels 1 to 3, with no baseline), and
isn't using strict_types; where they don't really have the time to add
strval() to everything that could be NULL.

Draft RFC:
https://wiki.php.net/rfc/allow_null

And the list, where I'm proposing we only relax this requirement for the
bold parameters:
https://github.com/craigfrancis/php-allow-null-rfc/blob/main/functions-change.md

I'll give it a few weeks to see if there are any parameter suggestions
(welcome via email, pull request, carrier pigeon, etc), then I'll start
looking for the best way to implement this.

In my opinion, it makes no sense to special case some functions in this
regard; the selection of which parameters of which function to special
case will always be too long for some, and to short for others. And
even if we find an acceptable compromise, what about functions of
external extensions (e.g. PECL packages, but also others, maybe private
packages). We have not much, if any, say on these, but they will be
affected by the deprecation anyway.

And then it's totally unclear to me how this is supposed to affect
strict_types=1. Either the respective string parameters are made
nullable unconditionally, so null would be accepted in strict mode as
well, what doesn't appear to be favorable. Or do we want to special
case this kind of nullable parameters, so they are only nullable for
strict_types=0? In my opinion, that would be the worst possible
outcome. How should this even be documented?

If the BC break is deemed to serious (maybe for string parameters only),
we better consider to undeprecate this, although that would make
internal functions behave differently to userland functions for
strict_types=0. But frankly, in that case the original RFC[1] should
not have passed, but it did, with 46 votes in favor, and none against.

[1] https://wiki.php.net/rfc/deprecate_null_to_scalar_internal_arg

Christoph

3 years ago by Craig Francis — view source — reply

unread

[...]
And then it's totally unclear to me how this is supposed to affect
strict_types=1.
[...]
If the BC break is deemed to serious (maybe for string parameters only),
we better consider to undeprecate this, although that would make
internal functions behave differently to userland functions for
strict_types=0. But frankly, in that case the original RFC[1] should
not have passed, but it did, with 46 votes in favor, and none against.

I'm open to suggestions on how this should be handled, my RFC is just a
draft.

As to the original RFC, there wasn't really a discussion on it:

https://externals.io/message/112327

The only person who raised a concern was Craig Duncan.

And tbh, just scanning the RFC text, it's very reasonable, as consistency
is ideal... but I didn't realise how much code would need to be changed
until a few projects I work on started trying to use 8.1 (one team has gone
back to 8.0, although I'm hoping they will accept a custom error handler to
ignore these deprecation notices).

Craig

3 years ago by Andreas Leathley — view source — reply

unread

I've spent a few days coming up with a short(ish) list of parameters that
are likely to receive NULL.

The focus is on making it easier for developers to upgrade to newer
versions of PHP. I'm thinking of the typical developer, probably using
WordPress, isn't using Psalm (at levels 1 to 3, with no baseline), and
isn't using strict_types; where they don't really have the time to add
strval() to everything that could be NULL.

I don't think the current deprecation notices are a real hindrance in
upgrading to a newer PHP version - there have been many such changes
which require possibly many small adaptions in code in the past, like
making undefined array keys a warning in PHP 8.0 (which seems a lot more
radical than adding the current deprecation notices). Singling out this
specific deprecation/change as impactful seems a bit overblown.

I also feel like such an un-deprecation would be premature. PHP 8.1 has
only just come out, the road until PHP 9.0 is still quite long, and
deprecation notices can easily be ignored for now, for those who do not
want to change their code right away. If in a year or two this is a big
problem in real code, then there would at least be a basis for changing
it, but I suspect most codebases will see more benefit in handling their
types more properly than if the language "sometimes" automatically
converts null to an empty string without complaining about it.

I did have some occurances of this deprecation notice in my applications
when upgrading to 8.1, and each time it was clearly a bug or oversight.
Removing the deprecation notices would also remove those insights into
possibly unreliable code.

3 years ago by Craig Francis — view source — reply

unread

I don't think the current deprecation notices are a real hindrance in
upgrading to a newer PHP version.

Nice, but I suspect your code doesen't exactly represents most code :-)

And while these may be "small adaptions", I work with some people where the
quantity is, impressive... and tbh I can't actually explain what the point
for this change is.

As an aside, it is making it even harder for me to get them to use static
analysis, as they keep saying this stuff is elitist bull... I'll leave it
there :-)

I did have some occurances of this deprecation notice in my applications

when upgrading to 8.1, and each time it was clearly a bug or oversight.
Removing the deprecation notices would also remove those insights into
possibly unreliable code.

Would you mind sharing some of them, as I've not seen anything like that
myself (that's genuine interest).

Craig

3 years ago by Craig Francis — view source — reply

unread

Draft RFC:
https://wiki.php.net/rfc/allow_null

I know, I'm a glutton for punishment, but I've started writing a patch for
this:

https://github.com/craigfrancis/php-src/compare/master...allow-null

It does not affect any Fatal Error exceptions for scripts using
'strict_types'.

For now it simply defines Z_PARAM_STR_ALLOW_NULL; it's not used by any
functions yet (that's the next step).

It works a bit like Z_PARAM_STR_OR_NULL (but it will return an empty
string instead of NULL).

It should be a fairly easy drop in replacement for Z_PARAM_STR.

e.g. To update htmlspecialchars(), and related functions, I'd simply
update this line:

https://github.com/php/php-src/blob/7b90ebeb3f954123915f6d62fb7b2cd3fdf3c6ec/ext/standard/html.c#L1324

Please note I'm not a C programmer, and I barely understand the code base,
so constructive/friendly feedback is welcome.

Thanks,
Craig

Extra notes...

No-one has given a single reason why any one of the listed functions should
not allow NULL; that said, I appreciate that developers using
strict_types do prefer this (and they will not be affected by this).

I've not seen any suggestion on how Rector, PHPCompatibility, or anything
else could provide a way to fix this issue automatically.

Only very strict static analysis (e.g. Psalm at levels 1 to 3, with no
baseline) currently picks up this issue.

I am simply trying to "relax certain type requirements on a case by case
basis" (which will help upgrading to PHP 9); ref the original discussion:

https://externals.io/message/112327

I know one person simply said this was a "terrible idea", but I'm still
waiting to hear any details on why.

In response to the "it's a bit late" position; we only have a deprecation
at the moment (can be ignored), it will be too late when PHP 9 starts
raising Fatal Error exceptions for anyone committing the heinous crime of
passing NULL to these functions (for reference, WordPress aren't exactly
rushing to fix this one).

The change in behaviour is done in zend_parse_arg_str_weak(), so I don't
think it will make much of a performance difference; but considering how
much effort went in to testing is_literal(), and how people still managed
to mis-read that, and how other RFCs don't mention performance details at
all, I'm skipping that bit of pain.

And yes, I have seen the -4 on externals.io... thank you for that
</sarcasm>.

3 years ago by Mark Randall — view source — reply

unread

I know one person simply said this was a "terribl > idea", but I'm still
waiting to hear any details on why.

The changes you propose are not something that I am comfortable with
either.

I understand your motivations in proposing them, but to my mind it goes
against the direction that PHP is developing, which I think is the right
one, where errors and likely errors result in stopping execution rather
than allowing it continue, potentially silently (dependent on error
handling settings).

If a parameter expects a string, that is what it should be given, and
its the callers' responsibility to ensure that is the case. If they fail
to do so then it's an error just like any other.

IMHO reverting to "If it's a null we'll just pretend its a string" is
contrary to how the language should be progressing.

It sucks that it was ever allowed in the first place.

PHP has a long history of making descisions to try to make things 'just
work', and if history teaches us anything, its that we inevitably come
to regret these descisions down the line.

Mark Randall

3 years ago by Björn Larsson via internals — view source — reply

unread

Den 2022-01-02 kl. 00:17, skrev Craig Francis:

On Thu, Dec 2, 2021 at 8:48 AM Craig Francis craig@craigfrancis.co.uk
wrote:

Is there any value in me proposing an RFC to update some internal
functions so they can accept NULL?

I'm not hard against this idea. The interpretation of null in these
contexts as being equivalent to empty string isn't unreasonable. I guess
the only objection I could have would be an academic one and I can't really
defend that. So yeah, sure... why not?

Thanks Sara,

I've spent a few days coming up with a short(ish) list of parameters that
are likely to receive NULL.

The focus is on making it easier for developers to upgrade to newer
versions of PHP. I'm thinking of the typical developer, probably using
WordPress, isn't using Psalm (at levels 1 to 3, with no baseline), and
isn't using strict_types; where they don't really have the time to add
strval() to everything that could be NULL.

Draft RFC:
https://wiki.php.net/rfc/allow_null

And the list, where I'm proposing we only relax this requirement for the
bold parameters:
https://github.com/craigfrancis/php-allow-null-rfc/blob/main/functions-change.md

I'll give it a few weeks to see if there are any parameter suggestions
(welcome via email, pull request, carrier pigeon, etc), then I'll start
looking for the best way to implement this.

Craig

PS; starting new thread, as I have gone a bit off topic, original one at:
https://externals.io/message/116519#116556

Hi,

Thanks for this RFC! I think it could interesting to see how libraries
that needs to adapt to this are doing it IRL.

One example I have seen how to fix this for e.g. substr is to replace:
$result = substr($string, 0, $length)
with:
$result = substr($string ?? '', 0, $lenght)

So the null coalescing operator comes in quite handy ;-) Now if this is
a good way to solve it is another matter. But if you are a maintainer of
a library that works fine and wants to support PHP 8.1 this will solve
the upgrade in a smooth way.

Would be interesting to see how common this approach is for solving
PHP 8.1 upgrades for non-nullable arguments with internal functions?

I mean, it shows the value with the RFC from a slightly another angle.

Regards //Björn

3 years ago by Craig Francis — view source — reply

unread

Draft RFC:
https://wiki.php.net/rfc/allow_null

I've been over-complicating this... why should NULL be treated so
differently?

<?php
$values = ['a', 1, 2.3, false, NULL];
foreach ($values as $value) {
echo urlencode($value);
}
?>

It just struck me, this whole thing is about type coercion when not using
strict_types=1; and the 8.1 change has introduced an inconstancy by
deprecating the coercion of NULL, a frequently used value.

Craig

3 years ago by Christoph M. Becker — view source — reply

unread

Draft RFC:
https://wiki.php.net/rfc/allow_null

I've been over-complicating this... why should NULL be treated so
differently?

<?php
$values = ['a', 1, 2.3, false, NULL];
foreach ($values as $value) {
echo urlencode($value);
}
?>

It just struck me, this whole thing is about type coercion when not using
strict_types=1; and the 8.1 change has introduced an inconstancy by
deprecating the coercion of NULL, a frequently used value.

That "inconsistency" had been introduced with PHP 7.0.0, i.e. right when
scalar type declarations have been introduced. Passing a null to a
non-nullable parameter of a userland function throws a TypeError:
https://3v4l.org/lbF4h. As of PHP 8.1.0, internal functions behave
the same as userland function in this regard.

--
Christoph M. Becker

3 years ago by Craig Francis — view source — reply

unread

That "inconsistency" had been introduced with PHP 7.0.0, i.e. right when
scalar type declarations have been introduced. Passing a null to a
non-nullable parameter of a userland function throws a TypeError

Thanks Christoph, that's a good way of looking at it.

Considering NULL has been passed to these internal function parameters for
years, for a variety of reasons, and PHP has coerced it into an empty
string, integer 0, float 0, or boolean false... I'm wondering if my RFC
should focus on matching that expectation, so anyone not using
strict_types=1 does not need to make loads of hard-to-find/unnecessary
edits; that way NULL would be treated the same as the other variable types
(I appreciate arrays and objects don't get coerced, but I don't think
anyone expects them to).

Craig

3 years ago by drealecs@gmail.com — view source — reply

unread

On Mon, Feb 21, 2022 at 5:32 PM Craig Francis craig@craigfrancis.co.uk
wrote:

On Mon, 21 Feb 2022 at 09:04, Christoph M. Becker cmbecker69@gmx.de
wrote:

That "inconsistency" had been introduced with PHP 7.0.0, i.e. right when
scalar type declarations have been introduced. Passing a null to a
non-nullable parameter of a userland function throws a TypeError

Thanks Christoph, that's a good way of looking at it.

Considering NULL has been passed to these internal function parameters for
years, for a variety of reasons, and PHP has coerced it into an empty
string, integer 0, float 0, or boolean false... I'm wondering if my RFC
should focus on matching that expectation, so anyone not using
strict_types=1 does not need to make loads of hard-to-find/unnecessary
edits; that way NULL would be treated the same as the other variable types
(I appreciate arrays and objects don't get coerced, but I don't think
anyone expects them to).

Yes, exactly that.
A few weeks ago when the issue was mentioned again I had to remember by
trying on 3v4l myself that using null for a string parameter wasn't an
automatically coerced case when strict_types = 0.

In my view, false would have the same problem for those internal functions.
Why should it be coerced to an empty string?
https://wiki.php.net/rfc/deprecate_null_to_scalar_internal_arg#proposal is
all about removing the automatic coercion that was happening for null in
the internal functions
Planning that in PHP 9 the type interpretation for internal function will
be done the same as for user defined functions. Consistency.

Right now, if a parameter type is defined as string in an user function, no
matter what strict_type is configured, it would still result in a TypeError
when passing null.
For internal functions, even if the parameter type is defined as string,
null would be passed and a coercion would happen.

As I mentioned, coercing false to int would be for me almost as bad as
coercing null to int.
But when types are not considered important I think it's worth pursuing
extending the coercion from null to the 4 other types where it's happening
right now:

int as 0,
float as 0.0,
string as an empty string
bool as false.

I don't like it and I have no good idea how that would work as it would be
a pretty big BC break.

Maybe it would be easier to change strict_types to default to 1 in PHP 9
along with adding the null coercion for null when strict_types is 0 rather
than inventing a new strict_types value like -1 that would allow null
coercion as well.
Starting with a notice in PHP 8.2 when the directive is not present might
be an interesting starting point. And no more warning for implicit coercion
when strict_types=0 directive is explicitly defined as it would not change
anymore in PHP 9.

Regards,
Alex

3 years ago by Guilliam Xavier — view source — reply

unread

On Tue, Feb 22, 2022 at 3:59 AM Alexandru Pătrănescu drealecs@gmail.com
wrote:

On Mon, Feb 21, 2022 at 5:32 PM Craig Francis craig@craigfrancis.co.uk
wrote:

On Mon, 21 Feb 2022 at 09:04, Christoph M. Becker cmbecker69@gmx.de
wrote:

That "inconsistency" had been introduced with PHP 7.0.0, i.e. right
when
scalar type declarations have been introduced. Passing a null to a
non-nullable parameter of a userland function throws a TypeError

Thanks Christoph, that's a good way of looking at it.

Considering NULL has been passed to these internal function parameters
for
years, for a variety of reasons, and PHP has coerced it into an empty
string, integer 0, float 0, or boolean false... I'm wondering if my RFC
should focus on matching that expectation, so anyone not using
strict_types=1 does not need to make loads of hard-to-find/unnecessary
edits; that way NULL would be treated the same as the other variable
types
(I appreciate arrays and objects don't get coerced, but I don't think
anyone expects them to).

Yes, exactly that.
A few weeks ago when the issue was mentioned again I had to remember by
trying on 3v4l myself that using null for a string parameter wasn't an
automatically coerced case when strict_types = 0.

For history:
https://wiki.php.net/rfc/scalar_type_hints_v5#behaviour_of_weak_type_checks
(emphasis mine):

A weakly type-checked call to an extension or built-in PHP function has
exactly the same behaviour as it did in previous PHP versions.

The weak type checking rules for the new scalar type declarations are
mostly the same as those of extension and built-in PHP functions. The
only exception to this is the handling of NULL: in order to be
consistent with our existing type declarations for classes, callables and
arrays, NULL is not accepted by default (...)

In hindsight, maybe that wasn't the most "fortunate" decision... (although,
fair enough, NULL was never considered to be "scalar")

In my view, false would have the same problem for those internal functions.
Why should it be coerced to an empty string?
https://wiki.php.net/rfc/deprecate_null_to_scalar_internal_arg#proposal is
all about removing the automatic coercion that was happening for null in
the internal functions
Planning that in PHP 9 the type interpretation for internal function will
be done the same as for user defined functions. Consistency.

(...)

As I mentioned, coercing false to int would be for me almost as bad as
coercing null to int.
But when types are not considered important I think it's worth pursuing
extending the coercion from null to the 4 other types where it's happening
right now:

int as 0,

float as 0.0,

string as an empty string

bool as false.

Indeed, that could also be a way to solve the original (PHP 7.0)
inconsistency between internal and user-defined functions
(although PHP 8.1 started to take the opposite route...)

I don't like it and I have no good idea how that would work as it would be
a pretty big BC break.

Maybe it would be easier to change strict_types to default to 1 in PHP 9
along with adding the null coercion for null when strict_types is 0 rather
than inventing a new strict_types value like -1 that would allow null
coercion as well.
Starting with a notice in PHP 8.2 when the directive is not present might
be an interesting starting point. And no more warning for implicit coercion
when strict_types=0 directive is explicitly defined as it would not change
anymore in PHP 9.

Or maybe (if going directly from error to implicit coercion is deemed too
"risky") the current TypeError in non-strict_types mode (when passing NULL
to a user-defined function expecting a scalar) could first be "demoted" to
some kind of Notice [who said E_STRICT] in 8.2 (along with reverting the
Deprecation added in 8.1 for internal functions) and removed in 9.0?

--
Guilliam Xavier

3 years ago by Craig Francis — view source — reply

unread

On Wed, 23 Feb 2022 at 14:00, Guilliam Xavier guilliam.xavier@gmail.com
wrote:

On Tue, Feb 22, 2022 at 3:59 AM Alexandru Pătrănescu drealecs@gmail.com
wrote:

But when types are not considered important I think it's worth pursuing
extending the coercion from null to the 4 other types where it's
happening
right now:

int as 0,

float as 0.0,

string as an empty string

bool as false.

Indeed, that could also be a way to solve the original (PHP 7.0)
inconsistency between internal and user-defined functions
(although PHP 8.1 started to take the opposite route...)

Yep, I'm hoping to find a solution that keeps as many people happy as
possible, and improves the language.

Personally I think strict_types=1 is fine for my code, but I would never
want to force that style on everyone, because doing so would be fairly
hostile for a language that's popular and well known for being easy to
use/learn.

And for those who like strict environments, I'm always interested in what
Content-Security-Policy they use, because that's a really useful form of
strict coding, especially when using default-src 'none'; script-src https://static.example.com; require-trusted-types-for 'script'; trusted-types 'none'; and Content-Type: application/xhtml+xml; charset=UTF-8 to ensure quoted attributes :-)

Craig

3 years ago by Mark Randall — view source — reply

unread

Personally I think strict_types=1 is fine for my code, but I would never
want to force that style on everyone, because doing so would be fairly
hostile for a language that's popular and well known for being easy to
use/learn.

Magically coercing things and hiding things under the hood is useful for
the first 10 minutes of learning.

After that it just becomes a hindrance difficult because you have to
know all of the secret rules about type juggling.

3 years ago by Dik Takken — view source — reply

unread

That "inconsistency" had been introduced with PHP 7.0.0, i.e. right when
scalar type declarations have been introduced. Passing a null to a
non-nullable parameter of a userland function throws a TypeError:
https://3v4l.org/lbF4h. As of PHP 8.1.0, internal functions behave
the same as userland function in this regard.

In my view, consistency between internal and userland functions brings a
lot of value, and not only for the language itself. As soon as internal
and userland become fully consistent it will become a lot easier to
write "internal" functions in PHP rather than C. Not only will that make
developing the standard library easier, it may also make the optimizer
and JIT compiler more effective. The more consistency the better.

Approaches 2 and 3 from the RFC are a step in the right direction.

Regards,
Dik Takken

3 years ago by Guilliam Xavier — view source — reply

unread

In my view, consistency between internal and userland functions brings a
lot of value, and not only for the language itself. As soon as internal
and userland become fully consistent it will become a lot easier to
write "internal" functions in PHP rather than C. Not only will that make
developing the standard library easier, it may also make the optimizer
and JIT compiler more effective. The more consistency the better.

Yes, and we see two possible ways to make them consistent w.r.t. handling
of null argument passed into scalar parameter:

implicit coercion by default, error with strict_types=1
error (independent of strict_types)

AFAIK, internal functions have been doing 1 since like forever, but PHP 7.0
chose 2 for userland functions when introducing scalar parameter type
declarations (see my previous message for history) and PHP 8.1 continued in
that direction by deprecating 1 for internal functions (and planning to
change them to 2 in PHP 9.0).
Call me devil's advocate, but is it too late to discuss revisiting past
decisions and consider changing direction towards 1 for userland functions
(esp. in implications of BC impact)?

Regards,

--
Guilliam Xavier

3 years ago by Larry Garfield — view source — reply

unread

In my view, consistency between internal and userland functions brings a
lot of value, and not only for the language itself. As soon as internal
and userland become fully consistent it will become a lot easier to
write "internal" functions in PHP rather than C. Not only will that make
developing the standard library easier, it may also make the optimizer
and JIT compiler more effective. The more consistency the better.

Yes, and we see two possible ways to make them consistent w.r.t. handling
of null argument passed into scalar parameter:

implicit coercion by default, error with strict_types=1

error (independent of strict_types)

AFAIK, internal functions have been doing 1 since like forever, but PHP 7.0
chose 2 for userland functions when introducing scalar parameter type
declarations (see my previous message for history) and PHP 8.1 continued in
that direction by deprecating 1 for internal functions (and planning to
change them to 2 in PHP 9.0).
Call me devil's advocate, but is it too late to discuss revisiting past
decisions and consider changing direction towards 1 for userland functions
(esp. in implications of BC impact)?

Under absolutely no circumstances should parameters become implicitly nullable. That way lies madness. And yes, coercing null to a magic zero-value is a form of implicit nullability. Absolutely not, under any circumstances.

Bringing internal functions into line with user-space was the correct move. There may be internals functions that make sense to be nullable on their own right, on a case by case basis. We can evaluate that case by case.

--Larry Garfield

3 years ago by Craig Francis — view source — reply

unread

Bringing internal functions into line with user-space was the correct
move. There may be internals functions that make sense to be nullable on
their own right, on a case by case basis. We can evaluate that case by
case.

Thanks Larry,

I agree about bringing them into line, but I think our understanding of
NULL may be different... these are PHP scripts which have used NULL as a
distinct value for, well, forever? and many developers expect it to be
coerced like the other values.

And while some developers use strict_types=1 (like myself) because we like
the type to match up without values being coerced (I should be the one that
manually chooses to convert)... I don't think I should force that strict
coding style onto everyone, because there is nothing technically wrong with
passing NULL into functions like urlencode(), it just implies, iff you use
strict_types=1, that something may have gone wrong earlier.

As an aside, I often note that many developers who talk about how strict
their code is... obv not you, but many still use 'unsafe-inline' JavaScript
on their websites (mixing content), and don't use Trusted Types to disable
unsafe JS APIs, which I consider a much bigger security concern :-)

And after all of this, no-one has come up with a way to find or address
this problem, e.g.

<?php
$nullable = ($_GET['a'] ?? NULL);
echo htmlentities($nullable);
?>

./vendor/bin/psalm --init ./public/ 4
./vendor/bin/psalm
No errors found!

./vendor/bin/phpstan analyse -l 9 ./public/
[OK] No errors

./vendor/bin/phpcs -p public/ --standard=PHPCompatibility
. 1 / 1 (100%)

./vendor/bin/rector process ./public/
[OK] Rector is done!

Craig

3 years ago by Christoph M. Becker — view source — reply

unread

And after all of this, no-one has come up with a way to find or address
this problem, e.g.

<?php
$nullable = ($_GET['a'] ?? NULL);
echo htmlentities($nullable);
?>

<?php
function my_htmlentities(string|null $value) {
return htmlentities($value ?? "");
}

$nullable = ($_GET['a'] ?? NULL);
echo my_htmlentities($nullable);
?>

The BC break doesn't appear to be that serious after all.

--
Christoph M. Becker

3 years ago by Christian Schneider — view source — reply

unread

Am 28.02.2022 um 22:05 schrieb Christoph M. Becker cmbecker69@gmx.de:

And after all of this, no-one has come up with a way to find or address
this problem, e.g.

<?php
$nullable = ($_GET['a'] ?? NULL);
echo htmlentities($nullable);
?>

<?php
function my_htmlentities(string|null $value) {
return htmlentities($value ?? "");
}

$nullable = ($_GET['a'] ?? NULL);
echo my_htmlentities($nullable);
?>

The BC break doesn't appear to be that serious after all.

I'm not sure I get your point here: If you provide a user-land implementation of the previous behavior under a different name then the BC break cannot be serious?

Chris

3 years ago by Craig Francis — view source — reply

unread

On Mon, 28 Feb 2022 at 22:11, Christian Schneider cschneid@cschneid.com
wrote:

Am 28.02.2022 um 22:05 schrieb Christoph M. Becker cmbecker69@gmx.de:

The BC break doesn't appear to be that serious after all.

I'm not sure I get your point here: If you provide a user-land
implementation of the previous behavior under a different name then the BC
break cannot be serious?

Chris

Yeah, I'm not sure what the point is either... sorry, I thought the
suggestion was a joke, and replied off-list as such, but creating
replacement userland functions for all of these functions, and to update
all code to use those functions... maybe I'm missing something.

And because I'm back to tired pissed-off sarcasm masking depression (why
bother spending days on a solution that works for everyone when we can make
a hostile/elitist environment/language)... maybe we could simply suggest
that everyone affected by this should use strval() for everything?
Admittedly the following only does 1 parameter, with a single variable, but
at least this is one way to dismiss/ignore the problem:

sed -i -E
's/(htmlspecialchars)(($[a-zA-Z_\x80-\xff][a-zA-Z0-9_\x80-\xff]*))/\1(strval(\2))/i'
index.php

As to voting at:

https://quiz.craigfrancis.co.uk/

So far it's 10 votes to continue supporting null coercing for those not
using strict_types=1 (as PHP always has always done with internal
functions, and non-strict comparison operators)... 10 votes to ignore the
problem and make a hard backwards compatibility break for PHP 9.0... and 4
votes to update some functions to explicitly allow null (going to round
two, you get 1 more vote each, and 2 which drop out because they either
didn't provide a second choice, or selected "Don't Mind")... also, while I
have counted them, 7 people didn't enter a name, or entered "N/A"... and
obviously this vote only helps me setup the RFC, it will be the RFC that
will be the official record of who voted for what.

Craig

3 years ago by Mark Randall — view source — reply

unread

And because I'm back to tired pissed-off sarcasm masking depression (why
bother spending days on a solution that works for everyone when we can make
a hostile/elitist environment/language)... maybe we could simply suggest
that everyone affected by this should use strval() for everything?

People aren't giving you the responses you want, because you haven't yet
made an argument to convince everyone that your proposed solution is
better than the existing ones, which are to use the language's existing
tools to provide a sensible string default where one is required.

You see a problem, but rather than trying to fix the underlying cause,
you're proposing making changes at other layers to accommodate the side
effects of the original problem.

That is practically the definition of a hack.

Except rather than a short term fix, you're proposing to make it a
permanent part of PHP.

That goes against what many of us would consider in the best interests
of PHP and its users in the long term, and yes, we acknowledge that
getting to that point will first require some short term pain for those
who have historically been overly lax with their coding standards.

Mark Randall

3 years ago by Christoph M. Becker — view source — reply

unread

Am 28.02.2022 um 22:05 schrieb Christoph M. Becker cmbecker69@gmx.de:

And after all of this, no-one has come up with a way to find or address
this problem, e.g.

<?php
$nullable = ($_GET['a'] ?? NULL);
echo htmlentities($nullable);
?>

<?php
function my_htmlentities(string|null $value) {
return htmlentities($value ?? "");
}

$nullable = ($_GET['a'] ?? NULL);
echo my_htmlentities($nullable);
?>

The BC break doesn't appear to be that serious after all.

I'm not sure I get your point here: If you provide a user-land implementation of the previous behavior under a different name then the BC break cannot be serious?

I said, the BC break doesn't appear to be that serious.

To elaborate: in my opinion, it is a good thing if internal functions
and userland functions behave the same regarding parameter types. As
such I'm fine with bringing internal functions inline with userland
functions. A somewhat reasonable alternative would be to bring userland
functions inline with internal functions (prior to PHP 8.1.0). However,
picking some functions and make some of their parameters nullable
does not look reasonable to me; that is just a hack to not break some
code, but still would not work for a lot of other code (e.g. code using
PECL extensions, which may not be changed in this regard), and would
make it harder to remember which parameters of which functions are
nullable now (and which have special meaning for null, and which do
simply treat null as an empty string).

Since it has been brought up multiple times that it is hard to find all
occurrences which would have to be adapted, I suggested a somewhat
viable solution for userland code (namely to define userland functions
which establish the old behavior; and of course, you'd need some tooling
to replace the original function calls). So everybody could use this
technique for those functions where it's relevant to the code base at hand.

--
Christoph M. Becker

3 years ago by Christian Schneider — view source — reply

unread

Am 01.03.2022 um 11:43 schrieb Christoph M. Becker cmbecker69@gmx.de:

Am 28.02.2022 um 22:05 schrieb Christoph M. Becker cmbecker69@gmx.de:

And after all of this, no-one has come up with a way to find or address
this problem, e.g.

<?php
$nullable = ($_GET['a'] ?? NULL);
echo htmlentities($nullable);
?>

<?php
function my_htmlentities(string|null $value) {
return htmlentities($value ?? "");
}

$nullable = ($_GET['a'] ?? NULL);
echo my_htmlentities($nullable);
?>

The BC break doesn't appear to be that serious after all.

I'm not sure I get your point here: If you provide a user-land implementation of the previous behavior under a different name then the BC break cannot be serious?

I said, the BC break doesn't appear to be that serious.

To elaborate: in my opinion, it is a good thing if internal functions
and userland functions behave the same regarding parameter types.

... just so it is mentioned as well (again, sorry): The other way of making internal functions behave like user land functions would be to change the definition of the internal function to
function htmlentities(?string $value) { ... }
and casting it to string internally which would make the definition/documentation/behavior consistent while staying backward compatible.

And while a purist might think this is the end of the world because null is evil I'm still looking for evidence where this pragmatic approach would lead to a real harm or security issues going unnoticed.

Speaking of user land wrappers around internal functions: If you really are that worried about invalid input to something like htmlentities then I'd suggest that you add even more checks to your htmlentities-like function, e.g. checkout for double-encodings. If you're dealing with various data sources this will help you catch visible problems. And a check for null can be very easily added there if you want to ;-)

I guess my conclusion is that the BC break ("damage") for these basic functions is not worth it since you need more checks in user land than relying on the relatively basic PHP type system ("benefit") in any real world application.

That's why I'm advocating for consistency by changing these basic function definitions instead of their behaviors.

Chris

3 years ago by Kamil Tekiela — view source — reply

unread

Hi Craig,

No, there is an inconsistency which leads to a backwards compatibility
issue, and I'm trying to fix it.

Which inconsistency exactly do you have in mind?

The discussion is about passing arguments to functions. We are not
discussing type juggling or type casting. To clarify the whole thing let's
list out the key points:

PHP has a concept of scalar types consisting of these four types: string,
int, float, bool
PHP has type juggling that converts the value to the type required by the
context in which it is used. The behaviour is defined for scalar types, but
often undefined for other types (see juggling to array)
PHP lets users cast values to a specified type. There are 7 (+1)
available casts in PHP (disregarding aliases). Casting to scalar types from
scalar types is defined. Casting to scalar from other types is partially
defined and causes undefined behaviour in some cases. Warnings are
generated in certain situations (e.g. array to string) and fatal errors
when the cast cannot be performed at all.
PHP has a special type called NULL with a single value NULL. It's not a
scalar and it's not a stand-alone type. Casting to NULL is not possible.
For all intents and purposes, NULL value represents a variable with no
value. For this reason, converting to other types generally does not cause
any notices, warnings or errors. However, using NULL might generate
warnings or errors depending on an operation. e.g. null[0],
null->scalar or [...null]
PHP has a concept of strict typing. It applies only to scalar parameters.
By default, PHP will coerce values of the wrong type into the expected
scalar type declaration if possible. NULL being a non-scalar value doesn't
coerce in non-strict mode. Neither does array, object or resource. See
https://3v4l.org/WZdQC

The only inconsistency is that PHP's internal functions silently accepted
NULL values for non-nullable parameters. This will be addressed in PHP 9.0.

Changing the signatures of some functions might be reasonable but, as few
others have said, it's not the solution we want to see because it feels
like a hack. As I have also pointed out off-list, in many cases making the
parameter type nullable would not make any sense as passing null to certain
functions is a clear bug. It would also have an impact on strict typing!

We could also reclassify NULL as a scalar type but that would be a
fundamental language change. It would likely be a bigger BC change than
fixing the current inconsistency.

By the way, your statement that "Coercion works for string/int/float/bool,
and NULL." is invalid. Coercion is defined only for scalar types as I
have explained above. Type juggling for most types from NULL is defined,
but that is not what we are discussing.

Regards,
Kamil

3 years ago by Craig Francis — view source — reply

unread

Hi Craig,

No, there is an inconsistency which leads to a backwards compatibility
issue, and I'm trying to fix it.

Which inconsistency exactly do you have in mind?

Ok, let's try with code (I'll skip using variables):

https://3v4l.org/gvXmr

<?php

var_dump(strtoupper(1));
var_dump(strtoupper(1.2));
var_dump(strtoupper(false));
var_dump(strtoupper('a'));
var_dump(strtoupper(NULL));

var_dump(strtoupper('' == ''));
var_dump(strtoupper(NULL == ''));

?>

Before PHP 8.1 all 5 forms of coercion worked for those not using
strict_types... now a form of coercion (from NULL) has been deprecated, and
for some reason it's likely to trigger a Fatal Error for everyone in 9.0
(while this is appropriate for those using strict_types, who are typically
using static analysis; it's not appropriate for everyone else).

The discussion is about passing arguments to functions. We are not

discussing type juggling or type casting. To clarify the whole thing let's
list out the key points:

PHP has a concept of scalar types consisting of these four types:
string, int, float, bool

But I am talking about Type Juggling:

https://www.php.net/manual/en/language.types.type-juggling.php

Or as noted under the Strict Typing documentation, where I've been using
the name "coercion":

https://www.php.net/manual/en/language.types.declarations.php#language.types.declarations.strict

Anyway, as noted earlier, developers who are not using strict_types, they
do not care about PHP's definition of what a scaler is... they don't
generally think about variable types (ish), and they don't really see type
coercion happening, they just expect it to work... yet, in 8.1, coercing
from NULL with internal functions is now deprecated (still works with
non-strict comparison operators though).

The only inconsistency is that PHP's internal functions silently accepted

NULL values for non-nullable parameters. This will be addressed in PHP 9.0.

Adding a type check for scripts not using strict_types it will make it
much harder for developers to upgrade to PHP 9 (not good).

While sticking strval() everywhere in itself is an easy change, due to the
numbers, and difficulty in finding them (note the lack of tooling), it's
very time consuming to update (your code may be fine, but that's not
common).

Changing the signatures of some functions might be reasonable but, as few

others have said, it's not the solution we want to see because it feels
like a hack. As I have also pointed out off-list, in many cases making the
parameter type nullable would not make any sense as passing null to certain
functions is a clear bug. It would also have an impact on strict typing!

Yep, I agree, see previous emails.

We could also reclassify NULL as a scalar type but that would be a

fundamental language change. It would likely be a bigger BC change than
fixing the current inconsistency.

I doubt we need to change the definition - I doubt many people would care
what the documentation says, and there is no need to modify the output of
is_scalar(), even if I personally find it a bit weird (but this isn't
about me).

We need to accept that PHP has coerced NULL to an empty string, integer 0,
float 0, and boolean false since, well, forever... and developers not using
strict_types have used this feature all the time (nearly always without
thinking about it).

By the way, your statement that "Coercion works for string/int/float/bool,

and NULL." is invalid. Coercion is defined only for scalar types as I
have explained above. Type juggling for most types from NULL is defined,
but that is not what we are discussing.

Erm, we are discussing type juggling "from NULL"... as in, NULL is being
provided to these functions, and the value is coerced from NULL to a
string/int/float/bool, as documented:

https://www.php.net/manual/en/language.types.string.php
"null is always converted to an empty string."

https://www.php.net/manual/en/language.types.integer.php
"null is always converted to zero (0)."

https://www.php.net/manual/en/language.types.float.php
"For values of other types, the conversion is performed by converting the
value to int first and then to float"

https://www.php.net/manual/en/language.types.boolean.php
"When converting to bool, the following values are considered false [...]
the special type NULL"

Craig

3 years ago by Craig Francis — view source — reply

unread

On Tue, 1 Mar 2022 at 13:06, Christian Schneider cschneid@cschneid.com
wrote:

The other way of making internal functions behave like user land functions
would be to change the definition of the internal function to
function htmlentities(?string $value) { ... }
and casting it to string internally which would make the
definition/documentation/behavior consistent while staying backward
compatible.

True, that is an option, and that's where I started... but I changed my
mind, because developers who use strict_types=1 can use this extra meta
data (the type check) to verify their code is working as expected.

In other words, someone checking types can see NULL being passed to
functions like escapeshellarg() as an error, and I do not want to break
that for them.

Craig

3 years ago by Craig Francis — view source — reply

unread

I said, the BC break doesn't appear to be that serious.

It is when it's a Fatal Error, and there are lots of them :-)

To elaborate: in my opinion, it is a good thing if internal functions
and userland functions behave the same regarding parameter types.

Yes, agreed.

However, picking some functions and make some of their parameters

nullable does not look reasonable to me.

Also agreed... while I did start with that suggestion, I'm uncomfortable
with making some parameters nullable due to how it would affect those using
strict_types (where it can be a good check for some oddities).

Since it has been brought up multiple times that it is hard to find all

occurrences which would have to be adapted, I suggested a somewhat
viable solution for userland code (namely to define userland functions
which establish the old behavior; and of course, you'd need some tooling
to replace the original function calls).

But that's a lot of work, and creates even messier code...

I'm pretty confident the best solution is to keep all parameter types the
same (I could argue that some parameters could do with a "cannot be empty"
exception, to reject NULL and an Empty String, but that would be a
different RFC)... and anyone using strict_types will still have a Type
Error when they pass a value of the wrong type to these parameters... the
only change is for those not using strict_types, for NULL to still be
coerced to a string/int/float/bool, as it always has been for internal
functions... and I think this can be done to user functions as well, as
developers not using strict_types won't have cared about this before.

Craig

3 years ago by Dik Takken — view source — reply

unread

I'm pretty confident the best solution is to keep all parameter types the
same (I could argue that some parameters could do with a "cannot be empty"
exception, to reject NULL and an Empty String, but that would be a
different RFC)... and anyone using strict_types will still have a Type
Error when they pass a value of the wrong type to these parameters... the
only change is for those not using strict_types, for NULL to still be
coerced to a string/int/float/bool, as it always has been for internal
functions... and I think this can be done to user functions as well, as
developers not using strict_types won't have cared about this before.

Craig

Hi Craig,

So, to get this crystal clear, this is my understanding of what you are
proposing for passing null to a non-nullable function parameter
(hopefully my ASCII art will come through ok):

which | strict_types | PHP 8.0 | PHP 8.1 | PHP 9
----------|---------------|------------|------------|----------
user | 1 | TypeError | TypeError | TypeError
internal | 1 | TypeError | TypeError | TypeError
user | 0 | TypeError | TypeError | coerced
internal | 0 | coerced | Deprecated | coerced

Is this correct?

Regards,
Dik Takken

3 years ago by Craig Francis — view source — reply

unread

So, to get this crystal clear, this is my understanding of what you are
proposing for passing null to a non-nullable function parameter
(hopefully my ASCII art will come through ok):

which | strict_types | PHP 8.0 | PHP 8.1 | PHP 9
----------|---------------|------------|------------|----------
user | 1 | TypeError | TypeError | TypeError
internal | 1 | TypeError | TypeError | TypeError
user | 0 | TypeError | TypeError | coerced
internal | 0 | coerced | Deprecated | coerced

Is this correct?

Yes, that's correct... although I'd be doing this for 8.2, to avoid
TypeError exceptions being introduced for non strict_types scripts in 9.0.

This is based on the feedback from the quiz and here; so type coercion from
NULL would continue to work, like coercion from the other variable types
(string/int/float/bool).

Craig

3 years ago by Larry Garfield — view source — reply

unread

So, to get this crystal clear, this is my understanding of what you are
proposing for passing null to a non-nullable function parameter
(hopefully my ASCII art will come through ok):

which | strict_types | PHP 8.0 | PHP 8.1 | PHP 9
----------|---------------|------------|------------|----------
user | 1 | TypeError | TypeError | TypeError
internal | 1 | TypeError | TypeError | TypeError
user | 0 | TypeError | TypeError | coerced
internal | 0 | coerced | Deprecated | coerced

Is this correct?

Yes, that's correct... although I'd be doing this for 8.2, to avoid
TypeError exceptions being introduced for non strict_types scripts in 9.0.

This is based on the feedback from the quiz and here; so type coercion from
NULL would continue to work, like coercion from the other variable types
(string/int/float/bool).

Craig

Null is not an empty string. Null is not a string. Null is not 0. Null is not an integer.

The clear trend of the language is toward more type strictness. Going back on that is a bad idea. Furthermore, increasing the delta between weak and strict mode only serves to bifurcate the language and add one more thing that people have to think about when they move from one file to another; the language behaves differently depending on what file you're in. That's already a cognitive overhead. Don't add to it.

I am firmly -1 on weakening the type system, even if "weak mode".

--Larry Garfield

3 years ago by Craig Francis — view source — reply

unread

Null is not an empty string. Null is not a string. Null is not 0. Null
is not an integer.

So what should this do?

$name = ($_POST['name'] ?? NULL);

var_dump($name == '');

Is that now going to be false?

3 years ago by Andreas Leathley — view source — reply

unread

Null is not an empty string. Null is not a string. Null is not 0. Null
is not an integer.

So what should this do?

$name = ($_POST['name'] ?? NULL);

var_dump($name == '');

Is that now going to be false?

Comparisons with == are quite worthless, except when comparing objects,
as == has so many caveats for when it returns true because of weird
rules about what should be considered equal or not for legacy reasons. I
don't think I have used == in the last 5 years, and life has gotten so
much better because of it.

Why not write:

$name = ($_POST['name'] ?? '');

Then you know 100% that $name does not contain null. It might not be a
string, but it cannot be null. You can still not safely pass it to
something like htmlentities or str_starts_with, because $name could be
an array. And I think that is the reason why the current deprecation and
eventual removal makes sense - it highlights missing logic that can
easily lead to errors. If the problem is not null, then it might be that
you have another unexpected type like an array, because the value was
not properly checked. Putting strval around such code (as one
possibility to resolve it) at least improves the code, in that we then
have a clear type for the variable, removing some room for errors and bugs.

3 years ago by Craig Francis — view source — reply

unread

Comparisons with == are quite worthless

Yep, but I'm focusing on how PHP works today, and while I welcome and
encourage improvements to the language, it has to be done gracefully...
forcing strict type checking on everyone (even if it's only for null), is
only going to cause upgrade problems.

I don't think I have used == in the last 5 years, and life has gotten so

much better because of it.

Yep, and I urge (not require) the developers I work with to do this... and
I don't for the kids at code club; they are just getting started, and they
are learning what a if statement is; if I add type checking as well, things
"break", they lose interest, and go off to do something else, like the
PiCar :-)

Why not write:

$name = ($_POST['name'] ?? '');

You can, but take the frameworks I noted earlier (and in the RFC). they
don't do this because sometimes it's useful to tell the difference between
a user provided Empty String, vs not set (NULL).

There is a lot of code that uses null coercion, even little things like
setcookie('name', $name, NULL, NULL, NULL, true) with 4 NULL's that work
today, but will fatal error in the future.

Craig

3 years ago by drealecs@gmail.com — view source — reply

unread

On Wed, Mar 2, 2022 at 4:17 PM Larry Garfield larry@garfieldtech.com
wrote:

So, to get this crystal clear, this is my understanding of what you are
proposing for passing null to a non-nullable function parameter
(hopefully my ASCII art will come through ok):

which | strict_types | PHP 8.0 | PHP 8.1 | PHP 9
----------|---------------|------------|------------|----------
user | 1 | TypeError | TypeError | TypeError
internal | 1 | TypeError | TypeError | TypeError
user | 0 | TypeError | TypeError | coerced
internal | 0 | coerced | Deprecated | coerced

Is this correct?

Yes, that's correct... although I'd be doing this for 8.2, to avoid
TypeError exceptions being introduced for non strict_types scripts in
9.0.

This is based on the feedback from the quiz and here; so type coercion
from
NULL would continue to work, like coercion from the other variable types
(string/int/float/bool).

Craig

Null is not an empty string. Null is not a string. Null is not 0. Null
is not an integer.

The clear trend of the language is toward more type strictness. Going
back on that is a bad idea. Furthermore, increasing the delta between weak
and strict mode only serves to bifurcate the language and add one more
thing that people have to think about when they move from one file to
another; the language behaves differently depending on what file you're
in. That's already a cognitive overhead. Don't add to it.

I am firmly -1 on weakening the type system, even if "weak mode".

This sounds like a good approach as well, not weakening the type system
but, even more, it might be good to strengthen it, even if we are talking
about strict_types=0.

What bothers me a bit is why should strlen(false) be a valid method call
but strlen(null) not so much. I think this is the inconsistency that should
be fixed so the language would be easier to work with.
And maybe the way to fix it is removing some coercions that happen right
now and make little or no sense, strengthening the type system.

The only coercion that is valid even with strict types:

int to float

Coercions that should probably be kept as it makes sense in some cases:

int numeric string to int
float numeric string to float
int numeric string to float
int to string
float to string
bool to int
int to bool

Coercions that should not be allowed, even if explicit type conversion
should be still kept:

float or float numeric string to int (already deprecated in 8.1, not
allowed starting with 9.0 by
https://wiki.php.net/rfc/implicit-float-int-deprecate)
bool to string
string to bool
bool to float
float to bool

Coercions that are already not working, even if explicit type conversion
works and should continue to work:

non numeric string to int or float, starting with PHP 7.0
not well formatted numeric string to int or float, starting with PHP 8.0
null to int/float/bool/string, starting with PHP 7.0, if that can be
considered a possible coercion

I think that some RFC for deprecating some more cases would make the
language a bit more consistent and reduce the coercion variation once we
get to PHP 9.0

Alex

3 years ago by Craig Francis — view source — reply

unread

On Wed, 2 Mar 2022 at 21:42, Alexandru Pătrănescu drealecs@gmail.com
wrote:

What bothers me a bit is why should strlen(false) be a valid method call
but strlen(null) not so much. I think this is the inconsistency that should
be fixed so the language would be easier to work with.

Hi Alexandru,

I typically spend an hour carefully thinking about each email I send, both
on and off list. I read and re-read what each person said, I try to
understand why they are saying it, my replies / suggestions are me trying
to find the best solution that will work for as many people as possible,
and the consequences they would have. I do not consider my coding style to
be only one, and I would never simply force someone to work in a different
way, in the same way that I won't force a CSP scrpt-src that blocked
'unsafe-inline' on everyone no matter how many XSS vulnerabilities it would
solve (I highlight provable problems, I make suggestions on how to fix, and
I'm open to alternative ideas).

With this RFC, I made my original suggestion, which was to alter some
functions so they could continue to accept NULL. I was wrong with that
approach (first I suggested doing this silently, which Larry correctly
pointed out was too messy; then I considered Larry's idea of updating
function signatures, but there are too many, and would reduce some useful
checks for those using strict_types).

The solution I have been proposing for the last week or so is based on
making PHP work with the well documented and understood coercion behaviour
for null, in a way that keeps with the spirit of the original RFC, and is
how many PHP scripts work today (noting that your typical PHP developer
does not dare enter the Internals mailing list).

I respect your idea, but what you are suggesting is very messy, it seems to
randomly deprecate and remove some type coercions, but that is just going
to confuse developers, and make upgrading harder to 9.0 even harder than
what is currently planned (see Python 2 to 3 as a classic example). I know
some people here fantasise of forcing everyone to use strict_types, and
making everyone use static analysis tools, but we are a long way off from
that happening today... what we can do - make it easy to move to
strict_types (ideally with good tooling), to sell that approach with the
positives it brings, while also keeping non-strict_types working as
expected so people continue to upgrade.

Craig

3 years ago by Andreas Leathley — view source — reply

unread

So, to get this crystal clear, this is my understanding of what you are
proposing for passing null to a non-nullable function parameter
(hopefully my ASCII art will come through ok):

which | strict_types | PHP 8.0 | PHP 8.1 | PHP 9
----------|---------------|------------|------------|----------
user | 1 | TypeError | TypeError | TypeError
internal | 1 | TypeError | TypeError | TypeError
user | 0 | TypeError | TypeError | coerced
internal | 0 | coerced | Deprecated | coerced

Is this correct?
Yes, that's correct... although I'd be doing this for 8.2, to avoid
TypeError exceptions being introduced for non strict_types scripts in 9.0.

This is based on the feedback from the quiz and here; so type coercion from
NULL would continue to work, like coercion from the other variable types
(string/int/float/bool).

Type coercion already often does not work - giving the string "s" to an
integer-typed argument will lead to a TypeError, it will not be coerced.
I would prefer less coercions rather than more. That you can give a
bool-typed argument "s" and it will be coerced to true seems quite bad
to me, for example.

I am someone who never uses strict_types although I use types everywhere
in my code - accepting the string "1" for an integer type seems fine to
me, especially because with HTTP requests you always get strings, and
often you also get strings from other external sources, like CSV,
databases, etc. So type coercion from strings to a certain degree seems
reasonable to me (although even there I am using more and more explicit
casts), but having to coerce null seems always avoidable to me, and null
is a special value explicitely. In my data null and an empty string
sometimes are two possible values, and automatic conversion seems like a
value judgement by the language that they are kind of similar, which
they should not be.

3 years ago by Craig Francis — view source — reply

unread

Type coercion already often does not work - giving the string "s" to an
integer-typed argument will lead to a TypeError, it will not be coerced.
I would prefer less coercions rather than more.

Hi Andreas,

I'll note that converting from NULL is used all the time, and is well defined:

https://www.php.net/manual/en/language.types.string.php https://www.php.net/manual/en/language.types.string.php
"null is always converted to an empty string."

https://www.php.net/manual/en/language.types.integer.php https://www.php.net/manual/en/language.types.integer.php
"null is always converted to zero (0)."

https://www.php.net/manual/en/language.types.float.php https://www.php.net/manual/en/language.types.float.php
"For values of other types, the conversion is performed by converting the value to int first and then to float"

https://www.php.net/manual/en/language.types.boolean.php https://www.php.net/manual/en/language.types.boolean.php
"When converting to bool, the following values are considered false [...] the special type NULL"

[...] but having to coerce null seems always avoidable to me, and null
is a special value explicitely. In my data null and an empty string
sometimes are two possible values, and automatic conversion seems like a
value judgement by the language that they are kind of similar, which
they should not be.

It really depends on your code base... Laravel, Symfony, CakePHP, CodeIgniter (the frameworks I'm familiar with) all return NULL for missing GET/POST/COOKIE values, and that is a useful feature... but when the developer naively passes that to any one of those listed parameters, you will get a deprecation message in 8.1, and a Type Error in the future... and finding all of these is incredibly hard.

For consistency, the next step should be to deprecate or Type Error when doing this setcookie('id', 123)

Craig

3 years ago by Andreas Leathley — view source — reply

unread

I'll note that converting from NULL is used all the time, and is well
defined:

https://www.php.net/manual/en/language.types.string.php
"null is always converted to an empty string."

https://www.php.net/manual/en/language.types.integer.php
"null is always converted to zero (0)."

https://www.php.net/manual/en/language.types.float.php
"For values of other types, the conversion is performed by converting
the value to int first and then to float"

https://www.php.net/manual/en/language.types.boolean.php
"When converting to bool, the following values are considered false
[...] the special type NULL"

This is the behavior for explicit type casting (strval) and the implicit
casting when using a variable in a string context, like echo or print.
This is basically just a base necessity - for example an array is
converted to "Array". All information about the array is lost, except
that the string is now called "Array". So would you say something like
htmlentities should also accept array values and automatically convert
it to the string "Array"? It is used all the time with echo and print,
and it is well defined, yet it is still not a good idea. For strval and
for echo it makes sense to convert an array to "Array" (barely, to end
up with a somewhat useable string), but for parameter types it does not
make sense. The same goes, in my opinion, for null.

It really depends on your code base... Laravel, Symfony, CakePHP,
CodeIgniter (the frameworks I'm familiar with) all return NULL for
missing GET/POST/COOKIE values, and that is a useful feature... but
when the developer naively passes that to any one of those listed
parameters, you will get a deprecation message in 8.1, and a Type
Error in the future... and finding all of these is incredibly hard.

These frameworks change their types and signatures quite often - much
more than PHP ever does. One Symfony project of mine that is now 7 years
old has had crazy changes, both in types, signatures, directory
structure, etc., yet people adapted and the changes were for the better.
Compared to the changes produced by frameworks this deprecation about
string values and null seems much smaller to me. And I do think it is
important that coercions in the language become less frequent rather
than more frequent, also because the language does have a certain "role
model" effect. From 7.0 until 8.1 it was a legacy effect that converted
null to an empty string, and only for internal functions, but with your
proposal it would be an intentional decision to start treating null and
scalar types more interchangeably.

3 years ago by Craig Francis — view source — reply

unread

This is the behavior for explicit type casting (strval) and the implicit
casting when using a variable in a string context, like echo or print.

But that's what the RFC is about.

Although you do raise a good point, why does print(NULL) not complain?

Think about htmlspecialchars(1), that integer is going to be cast to the
string '1', in exactly the same way that NULL is cast to the string ''.

This is basically just a base necessity - for example an array is

converted to "Array". All information about the array is lost, except
that the string is now called "Array". So would you say something like
htmlentities should also accept array values and automatically convert
it to the string "Array"?

Oddly, I'm fine with htmlentities(array()) being rejected, because it has
always been rejected... and, arrays/objects cannot be easily coerced to a
simple value (null can be, and is).

My focus is about not needlessly breaking code that works, I want everyone
to upgrade to PHP 9... you will notice I'm not complaining about the fairly
easy to implement #[ReturnTypeWillChange], the change for
mysqli_report(), or the change I made for htmlspecialchars() to ensure
single quotes are encoded by default... but tracing very variable from
source to sink, that is hard.

It really depends on your code base... Laravel, Symfony, CakePHP,

CodeIgniter (the frameworks I'm familiar with) all return NULL for
missing GET/POST/COOKIE values, and that is a useful feature... but
when the developer naively passes that to any one of those listed
parameters, you will get a deprecation message in 8.1, and a Type
Error in the future... and finding all of these is incredibly hard.

These frameworks change their types and signatures quite often

Yes, but, as noted, returning NULL for a missing value is useful... I
highly doubt they will change these function signatures so the default is
an empty string, and make the return value always a string.

Compared to the changes produced by frameworks this deprecation about

string values and null seems much smaller to me.

It's a small problem today, because it's a new deprecation (note how
WordPress is just telling people to ignore it)... the problem comes when it
becomes a Fatal Error.

And I do think it is important that coercions in the language become less

frequent rather than more frequent, also because the language does have a
certain "role model" effect. From 7.0 until 8.1 it was a legacy effect that
converted null to an empty string, and only for internal functions, but
with your proposal it would be an intentional decision to start treating
null and scalar types more interchangeably.

I am not making it more frequent, it's how it has ways worked (with the
oddity of user defined functions that specified parameter types); if you
don't like coercion, that's fine and good, but you should be using
strict_types... if PHP isn't going to support any coercion, then also
fine, but deprecate it all (and accept the consequences).

Craig

3 years ago by Craig Francis — view source — reply

unread

On Mon, 28 Feb 2022 at 17:35, Guilliam Xavier guilliam.xavier@gmail.com
wrote:

Call me devil's advocate, but is it too late to discuss revisiting past
decisions and consider changing direction towards 1 for userland functions

Hi Guilliam,

tbh, for those who use strict_types=1 nothing changes, so we can ignore
them... for everyone else, nothing would break if user functions did start
coercing NULL (if anything it just makes PHP more consistent)... whereas
the current direction would add a Type Error exception for developers who
aren't familiar with them, and those developers will just get annoyed at
having to convert those NULL values manually (see the complaints from
developers upgrading to PHP 8.1).

Craig

3 years ago by Craig Francis — view source — reply

unread

In my view, consistency between internal and userland functions brings a
lot of value, and not only for the language itself.

Thanks Dik,

I agree that consistency is very important, and I do not want to stop
that... I just recognise that many scripts use NULL coercion a lot, as they
do with other variable types (when not using strict_types=1), and that's
being broken just for NULL (which creates a form of inconsistency vs
int/float/string/bool types).

I started with preferring 2, but I think 1 is the best route to keep the
spirit of the original RFC, while not breaking backwards compatibility.

Craig

3 years ago by Craig Francis — view source — reply

unread

Draft RFC:
https://wiki.php.net/rfc/allow_null

To get a better idea on how I should progress this RFC, I've created a
simple quiz (well, modified an old script).

This is to decide if my RFC should either - continue to allow NULL to be
coerced when not using strict_types=1; or update some ~335 parameters to
explicitly allow NULL; or do we just ignore backwards compatibility and let
the current deprecation lead to a Fatal Error for everyone?

https://quiz.craigfrancis.co.uk/

As an aside, Juliette (@jrfnl) has confirmed that getting PHPCompatibility
to solve this problem will be "pretty darn hard to do" because it's "not
reliably sniffable" [1].

Craig

[1] https://twitter.com/jrf_nl/status/1497937320766496772

3 years ago by Lynn — view source — reply

unread

I know one person simply said this was a "terribl > idea", but I'm still
waiting to hear any details on why.

The changes you propose are not something that I am comfortable with
either.

I understand your motivations in proposing them, but to my mind it goes
against the direction that PHP is developing, which I think is the right
one, where errors and likely errors result in stopping execution rather
than allowing it continue, potentially silently (dependent on error
handling settings).

If a parameter expects a string, that is what it should be given, and
its the callers' responsibility to ensure that is the case. If they fail
to do so then it's an error just like any other.

IMHO reverting to "If it's a null we'll just pretend its a string" is
contrary to how the language should be progressing.

It sucks that it was ever allowed in the first place.

PHP has a long history of making descisions to try to make things 'just
work', and if history teaches us anything, its that we inevitably come
to regret these descisions down the line.

I agree with this sentiment. I rather see solutions that solve null values
being passed to these functions instead. The in-house legacy framework I
use has a small array wrapper that through ArrayAccess ensures null is
returned instead of a notice/warning being thrown when accessing a key that
does not exist. During offsetGet it will return itself wrapped around the
value if it's an array. I'm not saying that this is a good alternative for
the problem, it's just one of the solutions that can be taken to tackle
issues with $_POST and $_GET access for example.

3 years ago by Christian Schneider — view source — reply

unread

Am 07.02.2022 um 09:35 schrieb Mark Randall marandall@php.net:

I know one person simply said this was a "terribl > idea", but I'm still
waiting to hear any details on why.

I understand your motivations in proposing them, but to my mind it goes against the direction that PHP is developing, which I think is the right one, where errors and likely errors result in stopping execution rather than allowing it continue, potentially silently (dependent on error handling settings).

I think some proposals for PHP are going too far with strictness so I very much like Crag's RFC.

If a parameter expects a string, that is what it should be given, and its the callers' responsibility to ensure that is the case. If they fail to do so then it's an error just like any other.

The decision to define e.g. strlen() as strlen(string $string) instead of strlen(?string $string) was arbitrary and I for one would prefer the loose definition.

I'm confident that any real world example would abort at an early enough stage to catch errors in your program, without something as basic as strlen() having to do the job while adding complexity for something as simple as "if (strlen($x)) ..".

IMHO reverting to "If it's a null we'll just pretend its a string" is contrary to how the language should be progressing.
It sucks that it was ever allowed in the first place.

I think we agree to disagree here ;-)

PHP has a long history of making descisions to try to make things 'just work', and if history teaches us anything, its that we inevitably come to regret these descisions down the line.

PHP has a long history of being a concise, pragmatic language and there are many who do not regret this at all.
Finding the right balance is key here.

Chris

3 years ago by G. P. B. — view source — reply

unread

On Mon, 7 Feb 2022 at 09:34, Christian Schneider cschneid@cschneid.com
wrote:

If a parameter expects a string, that is what it should be given, and
its the callers' responsibility to ensure that is the case. If they fail to
do so then it's an error just like any other.

The decision to define e.g. strlen() as strlen(string $string) instead of
strlen(?string $string) was arbitrary and I for one would prefer the loose
definition.

Chris

I'm not sure what you are getting on here? strlen() was always defined to
only take a string and never was nullable in the first place.

The RFC which is at the heart of this is the one to make internal functions
and userland functions consistent.
Therefore introducing a mechanism which makes only some functions special
and weird is a complete no go for me.
Either there is a (strong) case for the parameter to be nullable, and then
it should be marked as such and not do weird shenanigans with regards to
strict_types (which I dislike more and more by the second), or there is not.

The list of proposed functions is also excessive and just feels that any
function which raised a diagnostic during a test run on 8.1 was chugged
into it.
Let's look at an example taken from the document (
https://github.com/craigfrancis/php-allow-null-rfc/blob/main/functions-change.md#output
)

setcookie(name:string, value:string, expires_or_options:array|int,
path:string, domain:string, secure:bool, httponly:bool)
setrawcookie(name:string, value:string, expires_or_options:array|int,
path:string, domain:string, secure:bool, httponly:bool)

I really do not see why for the path and the domain should ever accept
NULL, as if somehow that happens I would have some serious questions about
how you get that information.
However, there might be an argument to be made to make the value
parameter nullable. Although I would find this instance very questionable.

I'm not against in principle of marking certain parameters as nullable, but
it for sure should not be on a basis of using strict_types or not.

Best regards,

George P. Banyard

3 years ago by Craig Francis — view source — reply

unread

The list of proposed functions is also excessive and just feels that any
function which raised a diagnostic during a test run on 8.1 was chugged
into it.

Not really, while I did create a throw-away script that used
get_defined_functions() and ReflectionFunction to find all function
arguments that accepted strings, and not ->allowsNull()... I manually
checked every single one, and I considered if passing NULL to that argument
could represent a problem that warrants a Fatal Error for everyone.

https://github.com/craigfrancis/php-allow-null-rfc/blob/main/functions.php

This is why I have 2 other lists which I don't think justify any changes;
e.g. it's almost certainly wrong to pass NULL to the $characters
argument in trim():

https://github.com/craigfrancis/php-allow-null-rfc/blob/main/functions-maybe.md
https://github.com/craigfrancis/php-allow-null-rfc/blob/main/functions-other.md

I'd argue that the length of the list represents the size of the problem
(i.e. the amount of changes developers will need to make for PHP 9)... for
example...

setcookie

setrawcookie

I really do not see why for the path and the domain should ever accept
NULL.

Developers will often use NULL for argument defaults (remember, we didn't
show that the default was an empty string in the documentation, just that
it was an optional argument)... and considering the output for these two
functions, I see it as pretty reasonable that NULL was used as a way to say
"do not set the path attribute".

setcookie('name', 'value', 0, '/', '', true); // set-cookie: a=a;

path=/; secure
setcookie('name', 'value', 0, '', '', true); // set-cookie: a=a; secure
setcookie('name', 'value', 0, NULL, '', true); // set-cookie: a=a;
secure

So back to the same question as before, why should this NULL in the last
line result in a Fatal Error for everyone who does this in PHP 9?

Craig

3 years ago by Christian Schneider — view source — reply

unread

Am 07.02.2022 um 13:07 schrieb G. P. B. george.banyard@gmail.com:

If a parameter expects a string, that is what it should be given, and its the callers' responsibility to ensure that is the case. If they fail to do so then it's an error just like any other.

The decision to define e.g. strlen() as strlen(string $string) instead of strlen(?string $string) was arbitrary and I for one would prefer the loose definition.

Chris

I'm not sure what you are getting on here? strlen() was always defined to only take a string and never was nullable in the first place.

Now this is a little bit misleading: Before 7.0 we did not have scalar type hints so the documentation of strlen(string $string) was somewhat lying, especially because back in those days the automatic type jugging from null to "" was used a lot.

The RFC which is at the heart of this is the one to make internal functions and userland functions consistent.
Therefore introducing a mechanism which makes only some functions special and weird is a complete no go for me.
Either there is a (strong) case for the parameter to be nullable, and then it should be marked as such and not do weird shenanigans with regards to strict_types (which I dislike more and more by the second), or there is not.

So you are basically saying that we should discuss making it
strlen(?string $string): int
which would be cleaner, I agree.

About the case having to be strong: I could also argue that you have to make a strong case on why null should not be allowed any more (any more as per implementation, not documentation) as it is a seizable BC.

There are other things which can go wrong with strlen() on a higher level (e.g. misuse with utf-8) and I would like to see a real world code where strlen(null) actually broke something while not generating any other error in the code path.

Chris

3 years ago by Larry Garfield — view source — reply

unread

Am 07.02.2022 um 13:07 schrieb G. P. B. george.banyard@gmail.com:

So you are basically saying that we should discuss making it
strlen(?string $string): int
which would be cleaner, I agree.

I would agree here as well. The goal is consistency, so keep it consistent. If there's an argument to be made to rewiden a type to include null, make that case and have that debate on a function by function basis. No mode-based switcheroos.

--Larry Garfield

3 years ago by Craig Francis — view source — reply

unread

The goal is consistency, so keep it consistent.

I'd be happy with that.

The only reason I've taken this approach is for developers using type
checking to identify certain mistakes in their code (which can be useful);
so I wanted to keep them happy by not changing any of the stricter "string
only" arguments... that's why my patch silently makes PHP a bit more
tolerant of NULL for the projects that aren't using strict_types... and,
as noted before, I'm not suggesting all arguments - e.g. NULL is quite
likely a bug for the $characters argument in trim(), and PHP probably
should complain, even though an empty string does not.

If there's an argument to be made to rewiden a type to include null, make

that case and have that debate on a function by function basis.

I'm fine with that, but I worry people are underestimating the size of the
problem (having a debate on each one would take a long time).

I've put together my proposed list, it took a few days, and a lot of
thought:

https://github.com/craigfrancis/php-allow-null-rfc/blob/main/functions-change.md

I've asked for suggestions and pull requests a few times, and the best I've
got is $path/$domain for setcookie() (I disagree because NULL is
often used for those arguments, so that $secure and $httponly can be
set to true).

At this point I think people should be suggesting why any of these function
arguments should trigger a Fatal Error in PHP 9 if they receive NULL..
again, happy to discuss here; or off-list via email, video call, etc
(thankfully, for my own mental health, the off-list messages have been
positive)... or you can make suggestions on the repo:

https://github.com/craigfrancis/php-allow-null-rfc/issues

https://github.com/craigfrancis/php-allow-null-rfc/pulls

3 years ago by Larry Garfield — view source — reply

unread

If there's an argument to be made to rewiden a type to include null, make

that case and have that debate on a function by function basis.

I'm fine with that, but I worry people are underestimating the size of the
problem (having a debate on each one would take a long time).

I've put together my proposed list, it took a few days, and a lot of
thought:

https://github.com/craigfrancis/php-allow-null-rfc/blob/main/functions-change.md

I'm not suggesting a separate RFC for each. One RFC with several votes in it is fine, IMO.

What I disagree with is changing the definition of strict_types=0 from "use the well-documented coercion logic at parameter and return points" to "use the well-documented parameter and return points, and also add implicit nullability but only for internal functions, sometimes".

If it makes sense for strlen() to be nullable, then it makes sense for strlen() to be nullable. One or the other, not both based on a flag.

I've asked for suggestions and pull requests a few times, and the best I've
got is $path/$domain for setcookie() (I disagree because NULL is
often used for those arguments, so that $secure and $httponly can be
set to true).

Such is the RFC process...

--Larry Garfield

3 years ago by Craig Francis — view source — reply

unread

I'm not suggesting a separate RFC for each. One RFC with several votes in
it is fine, IMO.

What I disagree with is changing the definition of strict_types=0 from
"use the well-documented coercion logic at parameter and return points" to
"use the well-documented parameter and return points, and also add implicit
nullability but only for internal functions, sometimes".

Thanks Larry, that's fair... I can do that, and update the documentation as
well (will wait a bit to see if anyone else has any feedback).

Now the next problem, if we are going to have "One RFC with several
votes"... I've got 276 functions (336 arguments) which I think should
preserve their old behaviour (as in, there is no reason why a Fatal Error
should happen with a NULL value in PHP 9).

It might be possible to group them, like the 60 functions just related to
encoding values... because voting for them all individually in an RFC would
be a pain (I should know, I was checking each one).

Or should I setup a survey to make it a bit easier to collect yes/no
feedback before the actual RFC vote? I could add a feature to tick or
untick all by default.

Craig

As a side note, there are 99 functions on the "probably not" list (aka
maybe)... and 442 on the "other" list (the edge cases, like $method in
method_exists(), that would be weird if it was given a NULL).

3 years ago by Craig Francis — view source — reply

unread

I understand your motivations in proposing them, but to my mind it goes
against the direction that PHP is developing, which I think is the right
one, where errors and likely errors result in stopping execution rather
than allowing it continue

Hi Mark,

Thanks for responding.

I'm not completely against this approach in general, but I'm trying to find
the right balance.

Back to my example; most frameworks return NULL when a user value is not
provided via GET/POST/COOKIE (this is a useful feature), and many
developers simply pass that nullable string through to functions like
htmlspecialchars(), urlencode(), etc... I do not understand why this is an
error so bad that it must result in a Fatal Error for everyone using PHP 9.

As noted before, I can see how NULL might represent a problem in projects
using strict_types, but I'm not proposing we change anything for them
(also much more likely to be using static analysis).

And specifically with NULL, strings in C are a "one-dimensional array of
characters terminated by a null character", so you could say that NULL is
very similar to an empty string - maybe that helps address the "pretend its
a string" objection?

Craig

3 years ago by Mark Randall — view source — reply

unread

And specifically with NULL, strings in C are a "one-dimensional array of
characters terminated by a null character", so you could say that NULL is
very similar to an empty string - maybe that helps address the "pretend its
a string" objection?

In this case we're talking about the difference between a char* that
points to a 1 byte buffer containing '\0' and a char* that itself points
to NULL. Trying to perform string operations on a null pointer in C
would equally blow up.

With regards to your framework comment, any framework worth its salt
will also have the option to provide a default value other than null
when the value is not provided.

It is the responsibility of the user to make use of this functionality
to provide the correct type for what the soon-to-be-called function is
expecting.

Mark Randall

3 years ago by Craig Francis — view source — reply

unread

With regards to your framework comment, any framework worth its salt
will also have the option to provide a default value other than null
when the value is not provided.

Frameworks do allow developers to provide their own default value when a
GET/POST/COOKIE value has not been provided by the HTTP request.

But the 4 frameworks I've noted have already chosen to return NULL by
default... and while the distinction between an empty string and NULL is
rarely used, it can be helpful.

The only oddity I'm aware of is WordPress (who don't really care about
types), and while get_query_var() doesn't exactly map to $_REQUEST,
they did decide to use an empty string for their $default:

https://developer.wordpress.org/reference/functions/get_query_var/

So are you suggesting that all the frameworks should change their default
to an empty string? or every single project should update every use of
these input functions to always override this default, or cast the received
value to always be a string? all to avoid a Fatal Error if a NULL value
ever dared to be passed to htmlspecialchars() and similar functions in
PHP 9?

Or would it be simpler to allow NULL for some function arguments?

3 years ago by Mark Randall — view source — reply

unread

So are you suggesting that all the frameworks should change their default
to an empty string? or every single project should update every use of
these input functions to always override this default, or cast the received
value to always be a string? all to avoid a Fatal Error if a NULL value
ever dared to be passed to htmlspecialchars() and similar functions in
PHP 9?

I'm suggesting that every project should be specifying the default they
want when they call the function. If by omission you call a function in
a way that effectively says 'give me a null', then don't be surprised if
it breaks when it gives you what you asked for and you pass it to
something that wasn't designed for it.

Special casing things is almost always the wrong approach and will lead
to pain in the long run.

Like "0" being the only non-empty string that's falsey.

3 years ago by Larry Garfield — view source — reply

unread

Personally I think strict_types=1 is fine for my code, but I would never
want to force that style on everyone, because doing so would be fairly
hostile for a language that's popular and well known for being easy to
use/learn.

Magically coercing things and hiding things under the hood is useful for
the first 10 minutes of learning.

After that it just becomes a hindrance difficult because you have to
know all of the secret rules about type juggling.

This needs to be called out more. "Easy to learn" doesn't come from being forgiving of sloppiness. It comes from making the non-sloppy approach straightforward, obvious, and if someone does do something sloppy the error message(s) point people toward doing it the right way.

Forgiving of sloppiness is how you gt security errors, not ease of use.

--Larry Garfield

3 years ago by Craig Francis — view source — reply

unread

You see a problem, but rather than trying to fix the underlying cause,
you're proposing making changes at other layers to accommodate the side
effects of the original problem.

That is practically the definition of a hack.

No, there is an inconsistency which leads to a backwards compatibility
issue, and I'm trying to fix it.

I'm sorry I'm clearly no good at explaining this... maybe I should try
bullet points...

I am not trying undo the intent of the original RFC (constancy is good).

PHP has type coercion when not using strict_types=1; this is a feature,
one you might not like, but many do.

Coercion works for string/int/float/bool, and NULL.

It even has it's own clear paragraphs explaining this in the manual:

"null is always converted to an empty string."
"null is always converted to zero (0)."
"the following values are considered false ... NULL"

Not only does this happen with function parameters, but also non-strict
comparison operators, e.g. ($nullable == '') is true for NULL.

It was only user-defined function parameters with a specified type
(introduced in PHP 7.0) that broke that consistency, and only for NULL,
because the other types continued to be coerced, it was just NULL that was
the oddity.

This is important because NULL is used a lot, as both a distinct value, and
as a convention as the default for parameters (ref setcookie).

And internal functions have always accepted NULL.

As of 8.1, the inconstancy between user-defined vs internal functions was
addressed (good, I dislike consistencies), but the details were barely
discussed (one message, from Craig Duncan):

https://externals.io/message/112327

I'm trying to avoid an upgrade problem for PHP 9.0.

I want to keeping the strict checks in place for those who
use strict_types=1 (so Mark, you would not be affected by this, as I
assume you use strict_types=1 for everything).

And note the complete lack of tools to fix this issue, and how many
instances would need to be changed.

Unless you're planning to force strict_types=1 on everyone (Type Errors
for all, remove all type coercion), this inconstancy does not make any
sense.

Craig