[PHP-DEV] Adding the OpenSSF Scorecards GitHub Action
I've made this suggestion as issue #9778 ( https://github.com/php/php-src/issues/9778) and PR # 9789 ( https://github.com/php/php-src/pull/9789), but have been invited by @damianwadley to bring it to the mailing list. The Scorecards GitHub Action basically keeps an eye on a repo's security posture and makes simple, objective suggestions for possible improvements. For PHP's current Scorecard results, see here: https://api.securityscorecards.dev/projects/github.com/php/php-src. At the moment it's a raw json dump, but it contains information on the results of all the individual checks as well as comments on how to improve the scores. When the Action is installed, this is cleanly added to the project's GitHub Security Panel with step-by-step instructions. @iluuu1994 raised the issue that Scorecards suggests maximal branch protection and code review (prefer all contributions come via PRs with some form of code review prior to being added to the repo), which is quite distinct from the current PHP workflow which allows core maintainers to simply push directly. The reasons for this are entirely understandable. The Scorecard simply serves to indicate that other, more secure workflows exist. Whether their costs (in terms of agility and especially maintainer time) are worth it is a determination only the core team can make. I'm happy to answer any questions anyone might have, and am also happy to help PHP in other ways if I can! Thanks, Pedro P.S. First time contribution to the mailing-list, apologies for any missteps!
Re: [PHP-DEV] Compact can't resolve outer scoped variables usingshort closures
On 20.10.2022 at 19:03, Rowan Tommins wrote:
> On 20/10/2022 17:48, David Rodrigues wrote:
>
>> Regarding this, in case you're literally saying that internally "fn()"
>> turns into a common "function", and getting back to the main topic,
>> would it be possible to do the same with compact()?
>>
>> fn() => compact('dummy')
>> turns into:
>> fn() => [ 'dummy' => $dummy ]
>> which turns into:
>> function() use($dummy) { return [ 'dummy' => $dummy ]; }
>
> Yes, that is what I meant by "it would be possible for the compiler to
> special-case this scenario";
Well, in the general case, the compiler cannot special-case this,
because compact() is a normal function, so might be used as "variable
function", or may not refer to the top-level compact() at all (the
called symbol may not be fully qualified).
> I explained why I think that would be a bad
> idea, and suggested an alternative.
ACK
--
Christoph M. Becker
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
[PHP-DEV] Re: NULL Coercion Consistency
On 28 May 2022, at 03:36, Craig Francis wrote: > On 8 Apr 2022, at 18:34, Craig Francis wrote: >> I've written a new draft RFC to address the NULL coercion problems: >> https://wiki.php.net/rfc/null_coercion_consistency > > > I give up. For everyone affected by this... Rector has a "solution" via NullToStrictStringFuncCallArgRector. It has a list of 275 functions (442 parameters), and every time these functions are used, and Rector isn't sure a variable cannot be NULL, it will add a string type cast, e.g. - + So yeah, it's very messy, and while I think it makes the code worse, at least it's one way to avoid the fatal errors (coming in 9.0). Oh, and developers can easily re-introduce more "problems" (because they might not be aware which variables can be NULL), so Rector will need to re-run. I'm also tempted to create a composer library to provide a second solution, by using a namespace to re-define these functions, e.g. Have fun, Craig https://github.com/rectorphp/rector-src/blob/main/rules/Php81/Rector/FuncCall/NullToStrictStringFuncCallArgRector.php#L37 -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Compact can't resolve outer scoped variables using short closures
On 20/10/2022 17:48, David Rodrigues wrote:
Regarding this, in case you're literally saying that internally "fn()"
turns into a common "function", and getting back to the main topic,
would it be possible to do the same with compact()?
fn() => compact('dummy')
turns into:
fn() => [ 'dummy' => $dummy ]
which turns into:
function() use($dummy) { return [ 'dummy' => $dummy ]; }
Yes, that is what I meant by "it would be possible for the compiler to
special-case this scenario"; I explained why I think that would be a bad
idea, and suggested an alternative.
Regards,
--
Rowan Tommins
[IMSoP]
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Compact can't resolve outer scoped variables using short closures
> $foo = fn() => [$a, compact('a', 'b')];
> is essentially compiled to:
> $foo = function (use $a) { return [$a, compact('a', 'b')] };
Regarding this, in case you're literally saying that internally "fn()"
turns into a common "function", and getting back to the main topic, would
it be possible to do the same with compact()?
fn() => compact('dummy')
turns into:
fn() => [ 'dummy' => $dummy ]
which turns into:
function() use($dummy) { return [ 'dummy' => $dummy ]; }
Atenciosamente,
David Rodrigues
[PHP-DEV] Proposal to incrementally improve timeout and signal handling
Hello Internals, PHP suffers from several issues related to timeout and signal handling, especially when built with ZTS enabled. 1. The current implementation of timeouts on UNIX builds seems "fundamentally incompatible with ZTS" ( https://bugs.php.net/bug.php?id=79464#1589205685) and more anecdotally conflicts with some Go features ( https://github.com/golang/go/issues/56260#issuecomment-1281040802) 2. "Zend Signals" causes segmentation faults and other problems in multi-threaded environments ( https://github.com/php/php-src/issues/9649#issuecomment-1264330874, https://github.com/php/php-src/pull/5591#issuecomment-650064098), and seems useless anyway since PHP 7.1 ( https://github.com/php/php-src/pull/5591#issuecomment-645428002) In 2020, Alex Dowad started a major refactoring to improve these parts ( https://github.com/php/php-src/pull/5570, https://github.com/php/php-src/pull/5591, https://github.com/php/php-src/pull/5710), but he stopped working on it. Instead of doing a major one-time refactoring like that, I propose moving forward little by little to limit the risks and the potential backward compatibility breaks. Here is the plan: 1. Switch to timer_create() for timeouts, but only on Linux (it's not supported on other platforms yet), when ZTS is enabled and Zend Signals is disabled. As the feature is currently entirely broken when ZTS is on, this can be considered a bug fix and cannot be worse than it currently is. 1bis. Can be done independently of 1., and even in parallel, optional as long as we keep the --disable-zend-signals flag: Remove Zend Signals entirely, because even if it can be partially fixed (I proposed a patch fixing segfaults: https://github.com/php/php-src/pull/9766), it seems now useless and causes unfixable issues with some signals such as SIGINT ( https://github.com/php/php-src/issues/9649#issuecomment-1265811930) 2. Switch to Grand Central Dispatch on macOS and FreeBSD when ZTS is enabled and Zend Signals is disabled (if not removed at this point), which provides a feature similar to timer_create() for these platforms. 3. Probably in a future major version, optional: switch to timer_create()/GCD even for non-ZTS builds to uniformize and simplify the code. What do you think about this plan? Apart from the technical aspects, what's the best way forward? Submit patches? Propose an RFC? Do both? (pardon my ignorance of internals processes). Thank you, -- Kévin Dunglas
Re: [PHP-DEV] [VOTE] Improve unserialize() error handling
On Wed, 19 Oct 2022 at 19:11, Tim Düsterhus wrote: > > While the behavior would in fact change, it would not introduce a > "security issue" if this what you were hinting at. No, just that it would break in production, and then have to be fixed after a live site was already affecting end-users. > This would just result in an uncaught Exception which should > be very visible in your error tracking service. My impression is that most web-servers running PHP don't have those. For people who run most sites, the first they would know about it is when end-users started complaining. cheers Dan Ack -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Compact can't resolve outer scoped variables using short closures
On 19/10/2022 18:04, David Rodrigues wrote:
It seems to me to be a reasonable problem and one that needs attention, as
the message is not that "compact cannot be used here", but that "the
variable does not exist".
I'd just like to point out that the error message here is 100% correct:
there is nothing wrong with using compact() inside a closure, it just
only has access *at run-time* to the variables which were captured when
the closure was created, which are determined *at compile-time*.
$foo = fn() => [$a, compact('a', 'b')];
is essentially compiled to:
$foo = function (use $a) { return [$a, compact('a', 'b')] };
When you later execute $foo, compact() can see the local $a, but there
is no $b anywhere in the compiled definition.
On 20/10/2022 08:15, Claude Pache wrote:
Although it is difficult to make it work in general (of course), there is the
specific case of names given as literal strings, as in the example provided by
the OP, that does not suffer from the impossibility of static analysis.
While it would be possible for the compiler to special-case this
scenario, I think it could be rather confusing to have a function that
is sometimes evaluated at compile-time and sometimes at run-time (other
than as a transparent performance optimisation). For instance, each of
the following *can* be fully evaluated to a constant at compile-time,
but which ones would be?
compact('a' . 'b');
$name = 'a';
compact($name);
$x = 1;
compact('a' . $x);
$x = 1;
$x++;
compact('a' . $x);
I think it would be better to look at a new syntax for specifying such
arrays that doesn't rely on a run-time function, similar to how in JS {
foo, bar } is short-hand for { foo: $foo, bar: $bar }
This has come up before, for instance in Nikita's discussion of named
parameter syntaxes:
https://wiki.php.net/rfc/named_params#shorthand_syntax_for_matching_parameter_and_variable_name
As well as removing the need to pass a variable name as a string, this
would allow combining explicit and automatic names in one literal, e.g.:
// Current array literal
$x = [ 'foo' => $foo, 'bar' => someFunc() ];
// compact()
$bar = someFunc();
$x = compact('foo', 'bar');
// Shortest variation in Nikita's RFC linked above
$x = [ :$foo, bar: someFunc() ];
I'm personally not a fan of this coding style, because I think variables
should be named to be meaningful in the current scope, not somewhere
they're coming from or going to, but a dedicated syntax would at least
allow that flexibility.
Regards,
--
Rowan Tommins
[IMSoP]
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Compact can't resolve outer scoped variables using short closures
>
> How would you like it to work, if you pass a variable name variable to
> compact then?
> $x = 123;
> $name = 'x';
> (fn () => compact($name))();
>
Although it is difficult to make it work in general (of course), there is the
specific case of names given as literal strings, as in the example provided by
the OP, that does not suffer from the impossibility of static analysis.
> I agree with the idea of deprecating compact() & extract() and in
> long-term variable of variables as well.
However, in `compact('x', 'y')`, the name of the variables are not variable,
they are constant.
—Claude
