Re: [PHP-DEV] Re: Proposal for RFC to remove undefined array indexwarning when used ina ternary

2022-02-26 Thread Mark Randall

On 26/02/2022 22:34, Robert Landers wrote:

This is not semantically the same though. A $_POST of a form, for example,
will usually contain an empty value if the form input was empty, but
missing if the form control wasn't in the form (maybe the form control is
injected via Javascript, or conditionally output by the script).



PHP is a general purpose language, the behaviour you're asking for is 
specific to one use case and may be of detriment to others, without 
offering significant benefit in exchange.


I suggest you write yourself a function that performs the operations for 
you. This may also be a good opportunity to move away from directly 
accessing the superglobals in userland code.


$form->get('foo', 'defaultgoeshere');

Performance concerns would be so so small they would be practically 
undetectable in the context of a real request.


Also, this:

$name = empty($_POST['name'] ?? 'Default Name') ?: 'Default Name';

Should be:

$name = ($_POST['name'] ?? null) ?: 'Default Name')

--
Mark Randall

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] Re: Proposal for RFC to remove undefined array index warning when used ina ternary

2022-02-26 Thread Kamil Tekiela
I just wanted to add that the following

$name = $_POST['name'] ?: 'Default Name';

with existence check would be

$name = $_POST['name'] ?? null ?: 'Default Name';

You don't need empty().

I would be against changing the behaviour of elvis/ternary operator.
However, I remember seeing past suggestions to implement another operator.
One that would fill the gap between null-coalesce and elvis operators. If I
recall correctly, most of the time, these proposals end in consensus that
such operator isn't really needed. See
https://externals.io/message/89292#89292 and
https://externals.io/message/101606#101610
Maybe, it would be worthwhile to refresh a discussion about adding such
operator to the PHP language?


Re: [PHP-DEV] Re: Proposal for RFC to remove undefined array index warning when used ina ternary

2022-02-26 Thread Robert Landers
On Sat, Feb 26, 2022 at 11:34 PM Robert Landers 
wrote:

> On Sat, Feb 26, 2022 at 5:35 PM Mark Randall  wrote:
>
>> On 26/02/2022 09:09, Robert Landers wrote:
>> > I'd like to propose and implement an RFC that would consider dropping
>> the
>> > warning if and only if array access is the only test in ternary and
>> short
>> > ternary (?:) operations but first I'd like to start a discussion to see
>> if
>> > it is worth pursuing. I'd like to target PHP 8.2 or 8.3.
>>
>> The warning comes from the array key being missing, not from being
>> empty. There are many values which are empty which do not emit the
>> warning.
>>
>> If you want to provide a default should the array key be missing you
>> want null coalesce.
>>
>> $x = $array['foo'] ?? 'somethingelse';
>>
>>
> This is not semantically the same though. A $_POST of a form, for example,
> will usually contain an empty value if the form input was empty, but
> missing if the form control wasn't in the form (maybe the form control is
> injected via Javascript, or conditionally output by the script).
>
> Here is how the code might look currently to handle a default value for an
> empty form post that may or may not be sent in the request (ignoring how
> insecure this looks):
>
> $name = $_POST['name'] ?: 'Default Name';
>
> Semantically the same to get rid of the warning:
>
> $name = empty($_POST['name']) ? 'Default Name' : $_POST['name'];
>
> I'm not sure your suggestion to set a default applies since we have to do
> the empty check anyway, which swallows the warning?
>
> $name = empty($_POST['name'] ?? 'Default Name') ?: 'Default Name';
>

The fact that I just wrote ^^ that bug only to realize as soon as I sent it
should underscore how error-prone this migration could be.



>
>
>>
>> --
>> PHP Internals - PHP Runtime Development Mailing List
>> To unsubscribe, visit: https://www.php.net/unsub.php
>>
>>
> Robert Landers
> Software Engineer
> Utrecht NL
>
>
>


Re: [PHP-DEV] Re: Proposal for RFC to remove undefined array index warning when used ina ternary

2022-02-26 Thread Robert Landers
On Sat, Feb 26, 2022 at 5:35 PM Mark Randall  wrote:

> On 26/02/2022 09:09, Robert Landers wrote:
> > I'd like to propose and implement an RFC that would consider dropping the
> > warning if and only if array access is the only test in ternary and short
> > ternary (?:) operations but first I'd like to start a discussion to see
> if
> > it is worth pursuing. I'd like to target PHP 8.2 or 8.3.
>
> The warning comes from the array key being missing, not from being
> empty. There are many values which are empty which do not emit the warning.
>
> If you want to provide a default should the array key be missing you
> want null coalesce.
>
> $x = $array['foo'] ?? 'somethingelse';
>
>
This is not semantically the same though. A $_POST of a form, for example,
will usually contain an empty value if the form input was empty, but
missing if the form control wasn't in the form (maybe the form control is
injected via Javascript, or conditionally output by the script).

Here is how the code might look currently to handle a default value for an
empty form post that may or may not be sent in the request (ignoring how
insecure this looks):

$name = $_POST['name'] ?: 'Default Name';

Semantically the same to get rid of the warning:

$name = empty($_POST['name']) ? 'Default Name' : $_POST['name'];

I'm not sure your suggestion to set a default applies since we have to do
the empty check anyway, which swallows the warning?

$name = empty($_POST['name'] ?? 'Default Name') ?: 'Default Name';


>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
>
>
Robert Landers
Software Engineer
Utrecht NL


[PHP-DEV] Re: Proposal for RFC to remove undefined array index warning when used ina ternary

2022-02-26 Thread Mark Randall

On 26/02/2022 09:09, Robert Landers wrote:

I'd like to propose and implement an RFC that would consider dropping the
warning if and only if array access is the only test in ternary and short
ternary (?:) operations but first I'd like to start a discussion to see if
it is worth pursuing. I'd like to target PHP 8.2 or 8.3.


The warning comes from the array key being missing, not from being 
empty. There are many values which are empty which do not emit the warning.


If you want to provide a default should the array key be missing you 
want null coalesce.


$x = $array['foo'] ?? 'somethingelse';


--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] SensitiveParameterValue serialization behavior

2022-02-26 Thread Claude Pache


> 
> 1. Disallow both serialization and unserialization.
> 
> This will make the serialization issue very obvious, but will require 
> adjustments to exception handlers that serialize the stack traces.

Hi,

Note that exception handlers that serialise stack traces without taking into 
account that the operation may fail, are already broken as of today, because 
common unserialisable objects, such as Closure instances (anonymous functions), 
may appear in stack traces.

https://3v4l.org/tv1s1 

https://3v4l.org/PGKnl 

Making SensitiveParameterValue fail on serialisation won’t break those 
handlers, but make their existing brokenness apparent in more cases (which is a 
good thing).

—Claude



Re: [PHP-DEV] SensitiveParameterValue serialization behavior

2022-02-26 Thread Dan Ackroyd
On Thu, 24 Feb 2022 at 14:11, Tim Düsterhus, WoltLab GmbH
 wrote:
>
> I see two possible options to remediate this issue:
>
> ---
>
> 1. Disallow both serialization and unserialization.
>
> This will make the serialization issue very obvious, but will require
> adjustments to exception handlers that serialize the stack traces.

That sounds the best option. Yes, people who serialize/unserialize
stack traces will need to take into account the new behaviour, but
they will need to do that anyway, and it's better for something to
fail early, when the data is serialized rather than it happen later
when someone goes to use that data. Programs continuing when they have
already failed is Bad™.

> Allow unserialization, but poison the unserialized object and
> disallow calling ->getValue() on it.

That sounds like something to be instantly added to the list of PHP Sadness.

> I theoretically could add an additional public function isPoisoned(): bool as 
> well.

To me, that sounds like a workaround needed by a bad design aka a hack.

Guilliam Xavier wrote:
> to avoid potentially breaking existing code.

Technically, all existing code will continue to run, as no-one is
currently using SensitiveParameter and so that code will continue to
run. When people start using that feature, then yes they will need to
make sure that any stack serializer is aware of the new feature. But
that doesn't sound like a huge problem to me.

In general, I think we should only add surprising and awkward apis
when there is a really strong reason for doing so, not because there
might be a problem. If it's left as unserializable for now, people
would have the opportunity for saying why it needs to be relaxed
later, aka "no is temporary, yes is forever" for features.

cheers
Dan
Ack

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



[PHP-DEV] Proposal for RFC to remove undefined array index warning when used in a ternary

2022-02-26 Thread Robert Landers
Hello internals!

There is a lot of code in existence that does something like this:

$item['id'] ? [$item['id']] : $something_else

which to dissolve the current warning must be changed to something like
this:

!empty($item['id']) ? [$item['id']] : $something_else

However, if performance is an issue, we can do this in one less opcode (in
fact, the same number of opcodes as the original):

empty($item['id']) ? $something_ else : [$item['id']]

As far as I can tell, when single array access is the only test in a
ternary operation, it is exactly the same semantics as wrapping the array
access in a "!empty()". Fixing this warning seems like a lot of "busy work"
with little to no tangible benefit.

I'd like to propose and implement an RFC that would consider dropping the
warning if and only if array access is the only test in ternary and short
ternary (?:) operations but first I'd like to start a discussion to see if
it is worth pursuing. I'd like to target PHP 8.2 or 8.3.

It may also be worth discussing extending it to multiple array accesses
because to invert the not empties is counter-intuitive and error-prone:

!empty($a['test']) && !empty($a['ok']) ? 'a' : 'b';

becomes:

empty($a['test']) || empty($a['ok']) ? 'b' : 'a';

which was before (and IMHO, more readable):

$a['test'] && $a['ok'] ? 'a' : 'b';

This is very tricky to get right and to be fair, there will probably be
automated tools (if you trust such things on large codebases). There are
also performance issues to consider, as some basic benchmarks show
ISSET_ISEMPTY_DIM_OBJ is 6x slower than FETCH_DIM_R.

Robert Landers
Software Engineer
Utrecht NL