Re: [PHP-DEV] RFC1867 (multipart/form-data) PUT requests

2023-06-27 Thread Andreas Heigl

Hey All

On 28.06.23 02:45, Larry Garfield wrote:

On Tue, Jun 27, 2023, at 3:26 PM, Stephen Reay wrote:

On 28 Jun 2023, at 02:53, Ben Ramsey  wrote:


On Jun 27, 2023, at 04:01, Ilija Tovilo  wrote:

Hi Ben, Hi Rowan

On Mon, Jun 26, 2023 at 8:55 PM Ben Ramsey  wrote:



On Jun 20, 2023, at 06:06, Rowan Tommins  wrote:

On Tue, 20 Jun 2023 at 10:25, Ilija Tovilo  wrote:


Introduce a new function (currently named populate_post_data()) to
read the input stream and populate the $_POST and $_FILES
superglobals.


How about "request_form_populate_globals"?


The word "form" seems a bit out of place (even though it appears in
both multipart/form-data and application/x-www-form-urlencoded),
because this function is mainly targeted at PUT/PATCH requests for
REST APIs. Maybe request_body_populate_globals?


Another option for the name: `populate_multipart_form_data()`.


I avoided the term "multipart" because the function technically also
works for application/x-www-form-urlencoded requests. It's less
necessary for the reasons outlined in my previous email, but it would
allow for consistent handling of such requests for all HTTP methods.

Some people on GitHub voiced that they would prefer an INI setting.
Therefore I will create an RFC accordingly.



I know this issue comes up enough because it’s confusing to developers that 
there’s not a `$_PUT`, etc., but I’m not a fan of introducing something new 
that populates globals.

While I’ve never used `application/x-www-form-urlencoded` data for `PUT` 
requests (because I don’t think it’s a proper content-type for the semantics of 
`PUT`), I do see how this content-type could be used with `PATCH`, and I also 
don’t want to rule-out use cases of it for `PUT` or any other HTTP method.

In the past, I’ve used something like the following to solve this:

parse_str(file_get_contents('php://input'), $data);

I haven’t looked up how any of the frameworks solve this, but I would be 
willing to bet they also do something similar.

Rather than implementing functionality to populate globals, would you be 
interested in introducing some new HTTP request functions. Something like:

http_request_body(): string
http_parse_query(string $queryString): array

`http_request_body()` would return the raw body and would be the equivalent of 
calling `file_get_contents('php://input')`. Of special note is that it should 
_always_ return the raw body, even if `$_POST` is populated, for the sake of 
consistency and reducing confusion.

`http_parse_query()` would be the opposite of `http_build_query()` and would 
return a value instead of requiring a reference parameter, like `parse_str()`.

While these don’t address the confusion users face by not having a `$_PUT` 
superglobal, I’d prefer not to overload the superglobals. Maybe we can update 
the documentation to encourage use of these new functions instead of the 
superglobals?

We also might want to introduce something like `http_query_string(): string` 
that always returns the raw query string, instead of requiring use of the 
superglobal `$_SERVER['QUERY_STRING']`.

Cheers,
Ben



As a userland/library developer I *much* prefer this approach, but
*please* also include `http_uploads()` (name/signature TBD), returning
an array of file uploads using a structure akin to what $_POST (or
http_parse_query) gives, based on the field name, rather than the
current scenario where the presence of square brackets radically
changes the structure of the $_FILES array.

My initial thought was that perhaps the leaf entities in said array
could be a subclass of SplFileInfo referencing the uploaded temporary
file, with additional read-only properties for the extra upload-related
attributes - but really the important thing is just having a sane,
consistent structure. Having an object rather than an array with
constant keys would be a nice addition, but is much less important.



Cheers

Stephen


I also like Ben's suggestion.  It avoids adding more magic globals, but also is 
still simple and straightforward to use in an easily composable fashion.  Based 
on the Content-Type header of the request, it would allow for:

$formData = http_parse_query(http_request_body());
$jsonData = json_decode(http_request_body());
... Whatever other body parsers.

While you should probably not call it exactly like that, it means "take body string, 
pass to mime-appropriate body parser" is an easy pattern, and one that can also be 
easily mocked for testing.


While I like not adding more Superglobals, it seems like we are adding 
more and more functions to retrieve the different parts of a 
Request-Object...


So when we are at it: Why don't we introduce exactly that? A 
Request-Object that has all the methods. And which is immutable.


And one method `request(): \Request`

I deliberately didn't call it `getRequest` (or `get_request`) to not 
confuse people why there isn't also a `post_request` or `put_request` or 
... you get the picture)


One additional 

Re: [PHP-DEV] RFC1867 (multipart/form-data) PUT requests

2023-06-27 Thread Larry Garfield
On Tue, Jun 27, 2023, at 3:26 PM, Stephen Reay wrote:
>> On 28 Jun 2023, at 02:53, Ben Ramsey  wrote:
>> 
>>> On Jun 27, 2023, at 04:01, Ilija Tovilo  wrote:
>>> 
>>> Hi Ben, Hi Rowan
>>> 
>>> On Mon, Jun 26, 2023 at 8:55 PM Ben Ramsey  wrote:
 
> On Jun 20, 2023, at 06:06, Rowan Tommins  wrote:
> 
> On Tue, 20 Jun 2023 at 10:25, Ilija Tovilo  wrote:
> 
>> Introduce a new function (currently named populate_post_data()) to
>> read the input stream and populate the $_POST and $_FILES
>> superglobals.
> 
> How about "request_form_populate_globals"?
>>> 
>>> The word "form" seems a bit out of place (even though it appears in
>>> both multipart/form-data and application/x-www-form-urlencoded),
>>> because this function is mainly targeted at PUT/PATCH requests for
>>> REST APIs. Maybe request_body_populate_globals?
>>> 
 Another option for the name: `populate_multipart_form_data()`.
>>> 
>>> I avoided the term "multipart" because the function technically also
>>> works for application/x-www-form-urlencoded requests. It's less
>>> necessary for the reasons outlined in my previous email, but it would
>>> allow for consistent handling of such requests for all HTTP methods.
>>> 
>>> Some people on GitHub voiced that they would prefer an INI setting.
>>> Therefore I will create an RFC accordingly.
>> 
>> 
>> I know this issue comes up enough because it’s confusing to developers that 
>> there’s not a `$_PUT`, etc., but I’m not a fan of introducing something new 
>> that populates globals.
>> 
>> While I’ve never used `application/x-www-form-urlencoded` data for `PUT` 
>> requests (because I don’t think it’s a proper content-type for the semantics 
>> of `PUT`), I do see how this content-type could be used with `PATCH`, and I 
>> also don’t want to rule-out use cases of it for `PUT` or any other HTTP 
>> method.
>> 
>> In the past, I’ve used something like the following to solve this:
>> 
>>parse_str(file_get_contents('php://input'), $data);
>> 
>> I haven’t looked up how any of the frameworks solve this, but I would be 
>> willing to bet they also do something similar.
>> 
>> Rather than implementing functionality to populate globals, would you be 
>> interested in introducing some new HTTP request functions. Something like:
>> 
>>http_request_body(): string
>>http_parse_query(string $queryString): array
>> 
>> `http_request_body()` would return the raw body and would be the equivalent 
>> of calling `file_get_contents('php://input')`. Of special note is that it 
>> should _always_ return the raw body, even if `$_POST` is populated, for the 
>> sake of consistency and reducing confusion.
>> 
>> `http_parse_query()` would be the opposite of `http_build_query()` and would 
>> return a value instead of requiring a reference parameter, like 
>> `parse_str()`.
>> 
>> While these don’t address the confusion users face by not having a `$_PUT` 
>> superglobal, I’d prefer not to overload the superglobals. Maybe we can 
>> update the documentation to encourage use of these new functions instead of 
>> the superglobals?
>> 
>> We also might want to introduce something like `http_query_string(): string` 
>> that always returns the raw query string, instead of requiring use of the 
>> superglobal `$_SERVER['QUERY_STRING']`.
>> 
>> Cheers,
>> Ben
>
>
> As a userland/library developer I *much* prefer this approach, but 
> *please* also include `http_uploads()` (name/signature TBD), returning 
> an array of file uploads using a structure akin to what $_POST (or 
> http_parse_query) gives, based on the field name, rather than the 
> current scenario where the presence of square brackets radically 
> changes the structure of the $_FILES array.
>
> My initial thought was that perhaps the leaf entities in said array 
> could be a subclass of SplFileInfo referencing the uploaded temporary 
> file, with additional read-only properties for the extra upload-related 
> attributes - but really the important thing is just having a sane, 
> consistent structure. Having an object rather than an array with 
> constant keys would be a nice addition, but is much less important.
>
>
>
> Cheers
>
> Stephen

I also like Ben's suggestion.  It avoids adding more magic globals, but also is 
still simple and straightforward to use in an easily composable fashion.  Based 
on the Content-Type header of the request, it would allow for:

$formData = http_parse_query(http_request_body());
$jsonData = json_decode(http_request_body());
... Whatever other body parsers.

While you should probably not call it exactly like that, it means "take body 
string, pass to mime-appropriate body parser" is an easy pattern, and one that 
can also be easily mocked for testing.

--Larry Garfield

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



Re: [PHP-DEV] RFC1867 (multipart/form-data) PUT requests

2023-06-27 Thread Stephen Reay


> On 28 Jun 2023, at 02:53, Ben Ramsey  wrote:
> 
>> On Jun 27, 2023, at 04:01, Ilija Tovilo  wrote:
>> 
>> Hi Ben, Hi Rowan
>> 
>> On Mon, Jun 26, 2023 at 8:55 PM Ben Ramsey  wrote:
>>> 
 On Jun 20, 2023, at 06:06, Rowan Tommins  wrote:
 
 On Tue, 20 Jun 2023 at 10:25, Ilija Tovilo  wrote:
 
> Introduce a new function (currently named populate_post_data()) to
> read the input stream and populate the $_POST and $_FILES
> superglobals.
 
 How about "request_form_populate_globals"?
>> 
>> The word "form" seems a bit out of place (even though it appears in
>> both multipart/form-data and application/x-www-form-urlencoded),
>> because this function is mainly targeted at PUT/PATCH requests for
>> REST APIs. Maybe request_body_populate_globals?
>> 
>>> Another option for the name: `populate_multipart_form_data()`.
>> 
>> I avoided the term "multipart" because the function technically also
>> works for application/x-www-form-urlencoded requests. It's less
>> necessary for the reasons outlined in my previous email, but it would
>> allow for consistent handling of such requests for all HTTP methods.
>> 
>> Some people on GitHub voiced that they would prefer an INI setting.
>> Therefore I will create an RFC accordingly.
> 
> 
> I know this issue comes up enough because it’s confusing to developers that 
> there’s not a `$_PUT`, etc., but I’m not a fan of introducing something new 
> that populates globals.
> 
> While I’ve never used `application/x-www-form-urlencoded` data for `PUT` 
> requests (because I don’t think it’s a proper content-type for the semantics 
> of `PUT`), I do see how this content-type could be used with `PATCH`, and I 
> also don’t want to rule-out use cases of it for `PUT` or any other HTTP 
> method.
> 
> In the past, I’ve used something like the following to solve this:
> 
>parse_str(file_get_contents('php://input'), $data);
> 
> I haven’t looked up how any of the frameworks solve this, but I would be 
> willing to bet they also do something similar.
> 
> Rather than implementing functionality to populate globals, would you be 
> interested in introducing some new HTTP request functions. Something like:
> 
>http_request_body(): string
>http_parse_query(string $queryString): array
> 
> `http_request_body()` would return the raw body and would be the equivalent 
> of calling `file_get_contents('php://input')`. Of special note is that it 
> should _always_ return the raw body, even if `$_POST` is populated, for the 
> sake of consistency and reducing confusion.
> 
> `http_parse_query()` would be the opposite of `http_build_query()` and would 
> return a value instead of requiring a reference parameter, like `parse_str()`.
> 
> While these don’t address the confusion users face by not having a `$_PUT` 
> superglobal, I’d prefer not to overload the superglobals. Maybe we can update 
> the documentation to encourage use of these new functions instead of the 
> superglobals?
> 
> We also might want to introduce something like `http_query_string(): string` 
> that always returns the raw query string, instead of requiring use of the 
> superglobal `$_SERVER['QUERY_STRING']`.
> 
> Cheers,
> Ben


As a userland/library developer I *much* prefer this approach, but *please* 
also include `http_uploads()` (name/signature TBD), returning an array of file 
uploads using a structure akin to what $_POST (or http_parse_query) gives, 
based on the field name, rather than the current scenario where the presence of 
square brackets radically changes the structure of the $_FILES array.

My initial thought was that perhaps the leaf entities in said array could be a 
subclass of SplFileInfo referencing the uploaded temporary file, with 
additional read-only properties for the extra upload-related attributes - but 
really the important thing is just having a sane, consistent structure. Having 
an object rather than an array with constant keys would be a nice addition, but 
is much less important.



Cheers

Stephen 

Re: [PHP-DEV] RFC1867 (multipart/form-data) PUT requests

2023-06-27 Thread Ben Ramsey
> On Jun 27, 2023, at 04:01, Ilija Tovilo  wrote:
> 
> Hi Ben, Hi Rowan
> 
> On Mon, Jun 26, 2023 at 8:55 PM Ben Ramsey  wrote:
>> 
>>> On Jun 20, 2023, at 06:06, Rowan Tommins  wrote:
>>> 
>>> On Tue, 20 Jun 2023 at 10:25, Ilija Tovilo  wrote:
>>> 
 Introduce a new function (currently named populate_post_data()) to
 read the input stream and populate the $_POST and $_FILES
 superglobals.
>>> 
>>> How about "request_form_populate_globals"?
> 
> The word "form" seems a bit out of place (even though it appears in
> both multipart/form-data and application/x-www-form-urlencoded),
> because this function is mainly targeted at PUT/PATCH requests for
> REST APIs. Maybe request_body_populate_globals?
> 
>> Another option for the name: `populate_multipart_form_data()`.
> 
> I avoided the term "multipart" because the function technically also
> works for application/x-www-form-urlencoded requests. It's less
> necessary for the reasons outlined in my previous email, but it would
> allow for consistent handling of such requests for all HTTP methods.
> 
> Some people on GitHub voiced that they would prefer an INI setting.
> Therefore I will create an RFC accordingly.


I know this issue comes up enough because it’s confusing to developers that 
there’s not a `$_PUT`, etc., but I’m not a fan of introducing something new 
that populates globals.

While I’ve never used `application/x-www-form-urlencoded` data for `PUT` 
requests (because I don’t think it’s a proper content-type for the semantics of 
`PUT`), I do see how this content-type could be used with `PATCH`, and I also 
don’t want to rule-out use cases of it for `PUT` or any other HTTP method.

In the past, I’ve used something like the following to solve this:

parse_str(file_get_contents('php://input'), $data);

I haven’t looked up how any of the frameworks solve this, but I would be 
willing to bet they also do something similar.

Rather than implementing functionality to populate globals, would you be 
interested in introducing some new HTTP request functions. Something like:

http_request_body(): string
http_parse_query(string $queryString): array

`http_request_body()` would return the raw body and would be the equivalent of 
calling `file_get_contents('php://input')`. Of special note is that it should 
_always_ return the raw body, even if `$_POST` is populated, for the sake of 
consistency and reducing confusion.

`http_parse_query()` would be the opposite of `http_build_query()` and would 
return a value instead of requiring a reference parameter, like `parse_str()`.

While these don’t address the confusion users face by not having a `$_PUT` 
superglobal, I’d prefer not to overload the superglobals. Maybe we can update 
the documentation to encourage use of these new functions instead of the 
superglobals?

We also might want to introduce something like `http_query_string(): string` 
that always returns the raw query string, instead of requiring use of the 
superglobal `$_SERVER['QUERY_STRING']`.

Cheers,
Ben



signature.asc
Description: Message signed with OpenPGP


Re: [PHP-DEV] PDO Subclasses coming to vote soon.

2023-06-27 Thread Larry Garfield
On Tue, Jun 27, 2023, at 5:20 PM, Dan Ackroyd wrote:
> On Tue, 27 Jun 2023 at 17:25, Larry Garfield  wrote:
>>
>> The RFC doesn't specify if `new PDO(...)` changes behavior at all.
>
> That behaviour is not changed at all.
>
>> will PDO still have the postgres methods
>
> Yes, until someone does another RFC to deprecate and remove them.
>
>> if someone does [`new PDO(...)`], will they now get back `PdoPgsql`,
>
> No.
>
> New`ing one object, and getting a different object back would be far
> too surprising to even be contemplated.
>
> cheers
> Dan
> Ack

OK, thanks.  I'm fine with that approach, but it would be good to make it 
explicit, and probably add a Future Scope for "we should probably remove the 
base versions eventually in a few versions" or something.

--Larry Garfield

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



Re: [PHP-DEV] PDO Subclasses coming to vote soon.

2023-06-27 Thread Dan Ackroyd
On Tue, 27 Jun 2023 at 17:25, Larry Garfield  wrote:
>
> The RFC doesn't specify if `new PDO(...)` changes behavior at all.

That behaviour is not changed at all.

> will PDO still have the postgres methods

Yes, until someone does another RFC to deprecate and remove them.

> if someone does [`new PDO(...)`], will they now get back `PdoPgsql`,

No.

New`ing one object, and getting a different object back would be far
too surprising to even be contemplated.

cheers
Dan
Ack

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



Re: [PHP-DEV] PDO Subclasses coming to vote soon.

2023-06-27 Thread Larry Garfield
On Tue, Jun 27, 2023, at 1:49 PM, Dan Ackroyd wrote:
> Hi everyone,
>
> Just giving an update on the
> https://wiki.php.net/rfc/pdo_driver_specific_subclasses RFC as time is
> running out. The RFC text has been updated with the implemented
> subclasses stubs.

The RFC doesn't specify if `new PDO(...)` changes behavior at all.  Currently, 
as I understand it, `$db = new PDO($postgresConnectString)` returns a PDO 
instance that has the Postgres-specific methods enabled, which they can use.  
With this RFC, if someone does the same, will they now get back `PdoPgsql`, or 
will PDO still have the postgres methods, or will their code break?

--Larry Garfield

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



[PHP-DEV] PDO Subclasses coming to vote soon.

2023-06-27 Thread Dan Ackroyd
Hi everyone,

Just giving an update on the
https://wiki.php.net/rfc/pdo_driver_specific_subclasses RFC as time is
running out. The RFC text has been updated with the implemented
subclasses stubs.

There are a few small things to note, and one larger thing:

Marc Bennewitz wrote:
> It would be great if driver specific constants would be added to the
driver specific sub-classes without the driver name repeated in the
const name.

Okay, that will be done before it goes to vote. It probably has a
downside of making there need to be a large duplication of tests,
rather than being able to re-use the existing tests, but I guess it's
probably worth doing.

>> Create all DB sub-classes?
>
> yes please

k. That has been done.

Rowan Tommins wrote:
> but as a minimum there should be an internal API for
> registering a sub-class, without any modification to ext/pdo.

There is now.

The larger issue is whether to add an ini setting for SQLite extension
loading or not.

The sqlite3 PHP extension has an ini setting which limits extensions
to be loaded from a particular directory, presumably as a safety
precaution.

The Sqlite3 extension uses the code:

  sqlite3_enable_load_extension(sqlite_handle, 1);

which affects both the C api and loading extensions through SQL code.

However, in the proposed PdoSqlite class this code:

  sqlite3_db_config(db, SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION, 1);

is used to temporarily enable extension loading, which only enables it
through the C api.

As that means that SQLite extensions can only be loaded through C code
(not through SQL), and if someone can upload and execute code to your
server, your server is compromised anyway, having to edit ini files to
enable extension loading, seems like a bad tradeoff.

Thoughts?

cheers
Dan
Ack

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



Re: [PHP-DEV] [RFC] Deprecate remains of string evaluated code assertions

2023-06-27 Thread G. P. B.
I am going to group the response from the 3 emails.

On Tue, 27 Jun 2023 at 09:17, Claude Pache  wrote:

> Although the specific comment in the php.ini file talks about runtime
> switching, it is in fact irrelevant whether it is set at runtime or not.
> More concretely; I use currently the settings:
>
> * `zend.assertion=1` as set in the global php.ini;
> * `assert.active = on` or `off` in my per-directory .user.ini or .htaccess
> file, depending on whether I want to enable assertions by default;
> * optionally  `ini_set('assert.active', '1')` at runtime when I want to
> enable debug mode temporarily.
>
> I have established the above settings several years ago (at the time of
> PHP 7), after having carefully read the documentation (and having been
> slightly confused by the redundancy between `assert.active` and
> `zend.assertion`). I might have end up to switch between `zend.assertion =
> 0/1` instead of switching between `assert.active = on/off` instead; I can’t
> say exactly why I chose the former option instead of the latter, but I
> guess that both the comment in the `php.ini` file, and the existence of
> `assert_options(ASSERT_ACTIVE, ...)` as an alternative to
> `ini_set('assert.active', ...)`, have influenced my choice.
>
> Note also that the above settings minus `zend.assertion` was the way to do
> it in PHP 5 (again it is irrelevant whether it is at runtime or not), and
> many people that used assertions in PHP 5 may have continued to use that
> way without modification, as there is currently no strong reason to change
> (only `zend.assertions=-1` is relevant if you are especially concerned
> about micro-optimisation).
>
> Now, of course, people will adapt to the new settings. But given the
> confusing state of the options (both `zend.assertion` and `assert.active`
> must be enabled, and I am not even speaking about `assert.exception`), you
> ought to be super-clear (in the deprecation notice, in the php.ini file, in
> the docs), what is the the expected state: paradoxically leave
> `assert.active` and `assert.exceptions` enabled (the default value) even
> when you want to disable assertions, and playing exclusively with
> `zend.assertion`.
>

I would argue that zend.assertions=-1 is more than a micro-optimisation,
but that not the point.
Yes, you would need to switch to toggle zend.assertions between 1 and 0 if
you would want to enable/disable assertions at run-time.
However, considering that code should behave the same even when assertions
are not compiled (with the -1 value) I don't really see the point of
toggling between both modes.

The expected state is to leave all the assert.* INI settings to their
default value and only use the zend.assertions INI setting.

The state of the assert() docs were in very bad shape, and I hope that my
recent changes to it has improved it and made them clearer.
However, I will make sure that the path forward is very clear be that in
the INI setting docs, the assert() docs, the assert_options() docs, the
migratrion guide, and in the deprecation message.


On Tue, 27 Jun 2023 at 09:37, Claude Pache  wrote:

> I don’t see the RFC listed under https://wiki.php.net/rfc#under_discussion
> .
>

Fixed.


> The RFC is imprecise in what is meant by “deprecating”. I guess that a
> deprecation notice (E_DEPRECATED) will be triggered at least under the
> following conditions:
>
> * at startup when one of the assert.* setting has not the default value;
> * at runtime when `assert_options(...)` is used;
> * at runtime when `ini_set(...)` is used to set an `assert.*` option to a
> non-default value?
>
> It is unclear to me what will happen when:
>
> * `ini_set(...)` is used to set an `assert.*` option to its default value,
> either as a no-op or not?
>

Clarified.


> Moreover, by removing `assert.callback` (and other options), are you
> effectively removing a feature? (I don’t really know, I have never
> considered it even in my worst dreams.) If so, it should be noted in a
> “Backward Incompatible Changes”.
>

Yes, I don't really see the point of such a section as a deprecation, and
thus removal is clearly a BC Break.
I have tried to improve the wording to convey this more clearly.

On Tue, 27 Jun 2023 at 09:47, Claude Pache  wrote:

> changing the return value of `assert(...)` from `bool` (true) to `void` is
> also a BC break, (and it is unclear to me what is the effective advantage
> of the break).
>

Considering that any failed assertion is going to abort execution, having
it return a value is pointless.
Moreover, one must not rely on the expression being asserted to be executed
(thus it should have no side effects) and changing it to void very clearly
conveys this meaning that one must not store the result of the assert() as
it is not a totally normal function call.

Best regards,

George P. Banyard


Re: [PHP-DEV] [RFC] [Vote] Deprecate functions with overloaded signatures

2023-06-27 Thread Rowan Tommins

On 27/06/2023 02:25, Theodore Brown wrote:

Currently the following code returns only the array keys that loosely equal 
null [1]:

 array_keys($array, null)

If the function is changed to a single signature with $filter_value defaulting 
to null (with an argument count check to preserve the above functionality), its 
behavior becomes a lot more surprising/unexpected. If a parameter defaults to 
null, as a user I don't expect the function to work differently when I 
explicitly pass the default value.



An alternative solution to this situation is to introduce a new dummy 
value for such parameters, similar to how JavaScript uses the special 
"Symbol" type to have keys that can't collide with any userland value. 
In this context, this could be achieved with a single-value enum, i.e.:


enum ArrayKeys { case IncludeAll; }

function array_keys(array $array, mixed $filter_value = 
ArrayKeys::IncludeAll, bool $strict = false): array {}


That way, the optional parameter has a real default value, distinct from 
any value the user might wish to filter by, including null, and full 
compatibility is maintained with existing code.



I don't know whether this is a direction we want to go, but thought I'd 
throw it out there.


Regards,

--
Rowan Tommins
[IMSoP]

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



Re: [PHP-DEV] RFC Karma

2023-06-27 Thread Tim Düsterhus

Hi Nick

On 6/5/23 21:25, Nick Humphries wrote:

The folks handing out the RFC karma would need to know your Wiki
username to do so.


Thanks Tim, apologies for not including it in the original request.
Username is "humni"


Please apologize this large delay in handling your request. The number
of folks with the necessary permissions is small (but increasing them is
work-in-progress) and thus it was not handled in time. I've messaged the
relevant folks out-of-band and was told that you should now be able to
create an RFC. Please let me know if it doesn't work.

Best regards
Tim Düsterhus

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



Re: [PHP-DEV] [RFC] path_join function

2023-06-27 Thread Tim Düsterhus

Hi Soner

On 5/17/23 23:49, p...@shyim.de wrote:

Yes I would like to work on an proper RFC and make thoughts on the already 
given feedback here :)

My wiki name is: shyim


Please apologize this large delay in handling your request. The number 
of folks with the necessary permissions is small (but increasing them is 
work-in-progress) and thus it was not handled in time. I've messaged the 
relevant folks out-of-band and was told that you should now be able to 
create an RFC. Please let me know if it doesn't work.


Best regards
Tim Düsterhus

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



Re: [PHP-DEV] Re: RFC Karma Request: deprecation of ISO_8601 constants

2023-06-27 Thread Tim Düsterhus

Hi Jorg

On 6/22/23 18:41, Jorg Sowa wrote:

I didn't get any response and I'm not sure then whether I followed
everything correctly.


You did everything correctly. However the number of folks who are able 
to hand out Wiki karma is pretty small [1] and your request got lost. 
I'm not able to hand out the necessary karma myself, but messaged the 
relevant folks out of band.


I was told that you now have the necessary permissions. Please let me 
know if you have any other requests with regard to procedure.


Best regards
Tim Düsterhus

[1] Increasing the number is work-in-progress.

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



Re: [PHP-DEV] RFC1867 (multipart/form-data) PUT requests

2023-06-27 Thread Ilija Tovilo
Hi Ben, Hi Rowan

On Mon, Jun 26, 2023 at 8:55 PM Ben Ramsey  wrote:
>
> > On Jun 20, 2023, at 06:06, Rowan Tommins  wrote:
> >
> > On Tue, 20 Jun 2023 at 10:25, Ilija Tovilo  wrote:
> >
> >> Introduce a new function (currently named populate_post_data()) to
> >> read the input stream and populate the $_POST and $_FILES
> >> superglobals.
> >
> > How about "request_form_populate_globals"?

The word "form" seems a bit out of place (even though it appears in
both multipart/form-data and application/x-www-form-urlencoded),
because this function is mainly targeted at PUT/PATCH requests for
REST APIs. Maybe request_body_populate_globals?

> Another option for the name: `populate_multipart_form_data()`.

I avoided the term "multipart" because the function technically also
works for application/x-www-form-urlencoded requests. It's less
necessary for the reasons outlined in my previous email, but it would
allow for consistent handling of such requests for all HTTP methods.

Some people on GitHub voiced that they would prefer an INI setting.
Therefore I will create an RFC accordingly.

Ilija

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



Re: [PHP-DEV] [RFC] Deprecate remains of string evaluated code assertions

2023-06-27 Thread Claude Pache


> Le 27 juin 2023 à 10:36, Claude Pache  a écrit :
> 
> 
> 
>> Le 26 juin 2023 à 17:06, G. P. B.  a écrit :
>> 
>> On Wed, 31 May 2023 at 13:08, G. P. B.  wrote:
>> 
>>> Hello internals,
>>> 
>>> I would like to start the discussion about deprecating various remains
>>> from the now removed string code evaluated assertions functionality of
>>> assert().
>>> 
>>> The RFC is located on the wiki at the following address:
>>> https://wiki.php.net/rfc/assert-string-eval-cleanup
>>> 
>>> Initially, this was part of the mass PHP 8.3 deprecation RFC, but only the
>>> assert_options() function was part of it.
>>> 
>> 
>> Head's up, I'm planning on opening the vote on this on Wednesday the 28th
>> of June.
>> 
>> Best regards,
>> 
>> George P. Banyard
> 
> Hi,
> 
> Still some points:
> 
> I don’t see the RFC listed under https://wiki.php.net/rfc#under_discussion.
> 
> The RFC is imprecise in what is meant by “deprecating”. I guess that a 
> deprecation notice (E_DEPRECATED) will be triggered at least under the 
> following conditions:
> 
> * at startup when one of the assert.* setting has not the default value;
> * at runtime when `assert_options(...)` is used;
> * at runtime when `ini_set(...)` is used to set an `assert.*` option to a 
> non-default value?
> 
> It is unclear to me what will happen when:
> 
> * `ini_set(...)` is used to set an `assert.*` option to its default value, 
> either as a no-op or not?
> 
> Moreover, by removing `assert.callback` (and other options), are you 
> effectively removing a feature? (I don’t really know, I have never considered 
> it even in my worst dreams.) If so, it should be noted in a “Backward 
> Incompatible Changes”.
> 
> —Claude
> 

... and, of course, changing the return value of `assert(...)` from `bool` 
(true) to `void` is also a BC break, (and it is unclear to me what is the 
effective advantage of the break).

—Claude





Re: [PHP-DEV] [RFC] Deprecate remains of string evaluated code assertions

2023-06-27 Thread Claude Pache


> Le 26 juin 2023 à 17:06, G. P. B.  a écrit :
> 
> On Wed, 31 May 2023 at 13:08, G. P. B.  wrote:
> 
>> Hello internals,
>> 
>> I would like to start the discussion about deprecating various remains
>> from the now removed string code evaluated assertions functionality of
>> assert().
>> 
>> The RFC is located on the wiki at the following address:
>> https://wiki.php.net/rfc/assert-string-eval-cleanup
>> 
>> Initially, this was part of the mass PHP 8.3 deprecation RFC, but only the
>> assert_options() function was part of it.
>> 
> 
> Head's up, I'm planning on opening the vote on this on Wednesday the 28th
> of June.
> 
> Best regards,
> 
> George P. Banyard

Hi,

Still some points:

I don’t see the RFC listed under https://wiki.php.net/rfc#under_discussion.

The RFC is imprecise in what is meant by “deprecating”. I guess that a 
deprecation notice (E_DEPRECATED) will be triggered at least under the 
following conditions:

* at startup when one of the assert.* setting has not the default value;
* at runtime when `assert_options(...)` is used;
* at runtime when `ini_set(...)` is used to set an `assert.*` option to a 
non-default value?

It is unclear to me what will happen when:

* `ini_set(...)` is used to set an `assert.*` option to its default value, 
either as a no-op or not?

Moreover, by removing `assert.callback` (and other options), are you 
effectively removing a feature? (I don’t really know, I have never considered 
it even in my worst dreams.) If so, it should be noted in a “Backward 
Incompatible Changes”.

—Claude






Re: [PHP-DEV] [RFC] Deprecate remains of string evaluated code assertions

2023-06-27 Thread Claude Pache


> Le 26 juin 2023 à 17:05, G. P. B.  a écrit :
> 
> On Wed, 31 May 2023 at 23:20, Claude Pache  > wrote:
>> Although your RFC says that the `zend.assertions` ini setting has superseded 
>> `assert.active` for a while, the “official” php.ini file still advises to 
>> modify the value of `assert.active` rather than the one of `zend.assertion` 
>> in order to switch behaviour at runtime:
>> 
>> https://github.com/php/php-src/blob/91fd5641cde138b8894a48c921929b6e4abd5c97/php.ini-development#L1604
>> 
>> I bet that many people (myself included) follows the advice given in 
>> `php.ini`.
> 
> This talks about run-time modification, which is something that I don't think 
> should be done in practice, nor is often done.

Although the specific comment in the php.ini file talks about runtime 
switching, it is in fact irrelevant whether it is set at runtime or not. More 
concretely; I use currently the settings:

* `zend.assertion=1` as set in the global php.ini;
* `assert.active = on` or `off` in my per-directory .user.ini or .htaccess 
file, depending on whether I want to enable assertions by default;
* optionally  `ini_set('assert.active', '1')` at runtime when I want to enable 
debug mode temporarily.

I have established the above settings several years ago (at the time of PHP 7), 
after having carefully read the documentation (and having been slightly 
confused by the redundancy between `assert.active` and `zend.assertion`). I 
might have end up to switch between `zend.assertion = 0/1` instead of switching 
between `assert.active = on/off` instead; I can’t say exactly why I chose the 
former option instead of the latter, but I guess that both the comment in the 
`php.ini` file, and the existence of `assert_options(ASSERT_ACTIVE, ...)` as an 
alternative to `ini_set('assert.active', ...)`, have influenced my choice.

Note also that the above settings minus `zend.assertion` was the way to do it 
in PHP 5 (again it is irrelevant whether it is at runtime or not), and many 
people that used assertions in PHP 5 may have continued to use that way without 
modification, as there is currently no strong reason to change (only 
`zend.assertions=-1` is relevant if you are especially concerned about 
micro-optimisation).

Now, of course, people will adapt to the new settings. But given the confusing 
state of the options (both `zend.assertion` and `assert.active` must be 
enabled, and I am not even speaking about `assert.exception`), you ought to be 
super-clear (in the deprecation notice, in the php.ini file, in the docs), what 
is the the expected state: paradoxically leave `assert.active` and 
`assert.exceptions` enabled (the default value) even when you want to disable 
assertions, and playing exclusively with `zend.assertion`.

—Claude