Re: [PHP-DEV] Re: PHP-FPM process management woes

2021-12-22 Thread Pierre Joye
On Thu, Dec 23, 2021 at 5:32 AM Christoph M. Becker  wrote:
>
> On 22.12.2021 at 22:44, Jakub Zelenka wrote:
>
> > After thinking about this a bit more I think we would still need some way
> > to provide the current functionality of the MINIT if that's moved on child
> > level. The problem with not having such step is that opcache shm would be
> > then segmented between children - each child process would have its own shm
> > so it means that it would likely break things like opcache_reset that would
> > work only for a single child but wouldn't have any impact on other
> > children.
>
> I think the children could still re-attach to OPcache (like on Windows),
> and that might not even be that bad as on Windows (where ASLR can break
> that).  However, forking late might break preloading (not sure), and
> generally re-attaching isn't the nicest solution.

I remember some similar issues a while back. I wonder if it could make
sense to separate these two flows.

The MINIT is critical to many extensions, not doing fancy things like
OpCache or similar. Would it be possible to split them? One actual
MINIT (as per doc, once per process) and one for more fancy stuff like
what is done in OpCache (or apcu afair)? I can't think of another way
to make the MINIT/SHUTDOWN API behave as it should while being
designed before fpm came to life. Or we let the ext developers handle
it themselves by providing some helpers function about the current
stage the ext is in (root process or childs) but that will be painful
to do and port.

Best,
-- 
Pierre

@pierrejoye | http://www.libgd.org

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



Re: [PHP-DEV] header() allows arbitrary status codes

2021-12-22 Thread David Gebler
On Tue, Dec 21, 2021 at 6:59 PM Christoph M. Becker 
wrote:

> Hi all,
>
> a while ago it has been reported[1] that our header() function actually
> allows arbitrary status codes, which may even overflow.  Of course, that
> makes no sense, since the status code is supposed to be a three digit
> code.  So this ticket has been followed up by a pull request[2], and
> Jakub suggested to further restrict the status code to be in range 100 -
> 599.
>

Personally, I don't like restricting the status code to a number in the
100-599 range. As far as I know, RFC 7230 doesn't mandate anything beyond
the requirement of 3 digits and while 7231 may only specify semantics for
1xx-5xx, that doesn't mean there isn't a legitimate use-case in custom or
internal applications for status codes outside the usual semantics.

The overflow part is a legit bug which can and should be fixed, but I'd at
least question whether a user should be obliged to stick to conventional
HTTP semantics via the header() function, or even a strictly conformant
implementation of the standards defined 7320. Maybe this behaviour could be
default but overridable via a new, fourth optional parameter or something,
I don't know...but I can easily imagine someone having a legitimate context
in which they want to send status codes outside the usual range
representing custom semantics.


>
> Since this could break some pathological cases, I wanted to ask whether
> anybody objects to this change for the master branch (i.e. PHP 8.2).
>
> [1] 
> [2] 
>
> Christoph
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
>
>


Re: [PHP-DEV] header() allows arbitrary status codes

2021-12-22 Thread Christoph M. Becker
On 21.12.2021 at 20:09, Ayesh Karunaratne wrote:

>> a while ago it has been reported[1] that our header() function actually
>> allows arbitrary status codes, which may even overflow.  Of course, that
>> makes no sense, since the status code is supposed to be a three digit
>> code.  So this ticket has been followed up by a pull request[2], and
>> Jakub suggested to further restrict the status code to be in range 100 -
>> 599.
>>
>> Since this could break some pathological cases, I wanted to ask whether
>> anybody objects to this change for the master branch (i.e. PHP 8.2).
>>
>> [1] 
>> [2] 
>
> I think it is a useful improvement.  Should we adjust to
> http_response_code to match this behavior?

Oh, good catch!  I think we should.

Christoph

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



[PHP-DEV] Re: PHP-FPM process management woes

2021-12-22 Thread Christoph M. Becker
On 22.12.2021 at 22:44, Jakub Zelenka wrote:

> After thinking about this a bit more I think we would still need some way
> to provide the current functionality of the MINIT if that's moved on child
> level. The problem with not having such step is that opcache shm would be
> then segmented between children - each child process would have its own shm
> so it means that it would likely break things like opcache_reset that would
> work only for a single child but wouldn't have any impact on other
> children.

I think the children could still re-attach to OPcache (like on Windows),
and that might not even be that bad as on Windows (where ASLR can break
that).  However, forking late might break preloading (not sure), and
generally re-attaching isn't the nicest solution.

Christoph

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



[PHP-DEV] Re: PHP-FPM process management woes

2021-12-22 Thread Jakub Zelenka
>
>> My suggestion for a fix would be to emulate what Apache always did:
>>
>> One MINIT/MSHUTDOWN in the main control process (I think it needed that
>> to be able to implement php_admin_value and php_value), and then
>> additionally also in each worker process.
>>
>
> As I said above, it won't probably fully fix your problem but if you still
> want to try to tackle it and move the MINIT, the way that I would do it is
> to try to separate the whole sapi init logic and call it from the child
> init as the first thing. If you want to experiment with using
> php_admin_value before the module minit, then it might be worth to try put
> fpm_php_init_child (or just the defines) to sapi startup callback before
> calling php_module_startup so the main INIs are loads but it might be a bit
> tricky to get worker pool config - it might need some extra globals maybe
> for it. But not sure if that's gonna work (e.g. if the main INI files are
> loaded at the sapi startup stage) and what will be impact on extensions.
> I'd probably have to check it more and try it. Think it's also something
> that could happen in master branch only as it's changing some fundamental
> bits in FPM...
>
>
After thinking about this a bit more I think we would still need some way
to provide the current functionality of the MINIT if that's moved on child
level. The problem with not having such step is that opcache shm would be
then segmented between children - each child process would have its own shm
so it means that it would likely break things like opcache_reset that would
work only for a single child but wouldn't have any impact on other
children. Also apcu would be segmented and basically anything that uses shm
would be in some way impacted. The question is if maybe that single MINIT
is actually what's optimal for most extension and that symmetry is just for
some special cases which is actually my feeling. In such case it would be
probably better to keep MINIT as it is and introduce new step for that
symmetry with MSHUTDOWN.

By the way that shm sharing on master level is not ideal because this
should really be done on the pool level. I have been thinking about
something called pool manager (process under master that would manage pool
processes in the same or similar way as master does right know) for some
time. That would allow this kind of shm sharing except other things.

Regards

Jakub


Re: [PHP-DEV] What should we do with utf8_encode and utf8_decode?

2021-12-22 Thread Rowan Tommins

On 22/12/2021 14:45, Hans Henrik Bergan wrote:

I wonder if anyone depends on utf8_* without also depending on mb_* ? I
imagine that is exceedingly rare



On the contrary, anyone who uses mb_* functions is likely to use 
mb_convert_encoding rather than utf8_encode and utf8_decode.


In fact, the only legitimate uses of the functions I've seen are as a 
fallback for when ext/mbstring is not loaded, since they are always 
available (since PHP 7.2; before that, they were oddly part of ext/xml). 
There is a very small set of use cases where you really do know you have 
or want ISO 8859-1, and they are the most portable implementation.


Regards,

--
Rowan Tommins
[IMSoP]

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



Re: [PHP-DEV] [VOTE] Add array_group function

2021-12-22 Thread Jordan LeDoux
On Wed, Dec 22, 2021 at 1:39 AM Hassan Ahmed <7sno...@gmail.com> wrote:

> Hello Levi, maybe, I just was confused while I was rewriting it. any tips
> would be appreciated.
>

There's a lot of information about this that is missing from the RFC. It
says it "will be overwritten". Does that mean that it does a right union?
What exactly is the meaning of null for the parameters? That isn't covered.

The language on how it handles key conflicts is confusing. It says that it
"currently" handles it by "overwriting", but doesn't explain what that
means. It also doesn't *currently* do that, since it isn't currently part
of the language. You have to scroll down to the example to decipher that
"overwrite" means it takes the last value.

It says there's no backwards incompatible breaks, but that isn't strictly
true, as the `array_group()` function in the global namespace will now be
occupied. That *might* be a rather small BC break, but I'm not sure.

I think you are seeing no votes mostly because the RFC itself is simply
incomplete. It doesn't describe the use case, it doesn't describe the
implementation at all, and it doesn't describe the edge cases at all. I
think it could be an idea that people might support, but you probably need
to find someone who has done an RFC before to help you workshop it a bit.
These are all fixable problems in the RFC document, it could be that the
implementation and the idea are well received.

Jordan


Re: [PHP-DEV] What should we do with utf8_encode and utf8_decode?

2021-12-22 Thread Hans Henrik Bergan
I wonder if anyone depends on utf8_* without also depending on mb_* ? I
imagine that is exceedingly rare

On Wed, Dec 22, 2021, 15:26 Rowan Tommins  wrote:

> On 22/12/2021 10:45, Andreas Heigl wrote:
> > I just dug a bit deeper on the subject and found this RFC from 2016:
> >
> > https://wiki.php.net/rfc/remove_utf_8_decode_encode
> >
> > Perhaps we can just revive that one!
>
>
> As I say, I have a draft with lots more detail in, which I will tidy up
> after Christmas. I deliberately didn't link to it, because I want to
> re-read it myself before letting other people comment on it, and don't
> have the time right now.
>
> My current inclination is to deprecate in 8.next, and remove in 9.0, but
> I want to make sure the argument for that is solid before putting it to
> a vote.
>
> Regards,
>
> --
> Rowan Tommins
> [IMSoP]
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
>
>


Re: [PHP-DEV] What should we do with utf8_encode and utf8_decode?

2021-12-22 Thread Rowan Tommins

On 22/12/2021 10:45, Andreas Heigl wrote:

I just dug a bit deeper on the subject and found this RFC from 2016:

https://wiki.php.net/rfc/remove_utf_8_decode_encode

Perhaps we can just revive that one! 



As I say, I have a draft with lots more detail in, which I will tidy up 
after Christmas. I deliberately didn't link to it, because I want to 
re-read it myself before letting other people comment on it, and don't 
have the time right now.


My current inclination is to deprecate in 8.next, and remove in 9.0, but 
I want to make sure the argument for that is solid before putting it to 
a vote.


Regards,

--
Rowan Tommins
[IMSoP]

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



Re: [PHP-DEV] What should we do with utf8_encode and utf8_decode?

2021-12-22 Thread Andreas Heigl

Hey All.

On 22.12.21 10:08, Andreas Heigl wrote:

Hey all.

On 22.12.21 10:00, Rowan Tommins wrote:

[...]


On 22/12/2021 00:31, Kris Craig wrote:

Now might be a good time to make this into an RFC.  :)



I have a draft kicking around with a lot of analysis of current usage. 
I will try to pick it back up after Christmas.


I just dug a bit deeper on the subject and found this RFC from 2016:

https://wiki.php.net/rfc/remove_utf_8_decode_encode

Perhaps we can just revive that one!

Cheers

Andreas
--
  ,,,
 (o o)
+-ooO-(_)-Ooo-+
| Andreas Heigl   |
| mailto:andr...@heigl.org  N 50°22'59.5" E 08°23'58" |
| https://andreas.heigl.org   |
+-+
| https://hei.gl/appointmentwithandreas   |
+-+


OpenPGP_0xA8D5437ECE724FE5.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature


Re: [PHP-DEV] [VOTE] Add array_group function

2021-12-22 Thread Hassan Ahmed
Hello Levi, maybe, I just was confused while I was rewriting it. any tips
would be appreciated.

On Tue, Dec 21, 2021 at 5:58 PM Levi Morrison 
wrote:

> On Tue, Dec 21, 2021 at 1:48 AM Hassan Ahmed <7sno...@gmail.com> wrote:
> >
> > Hello all,
> >
> > I would like to start a voting phase for this RFC: Add array_group
> function
> > 
> >
> > Regards,
> >
> > ~Hassan
>
> This RFC has mistakes in its opening Discussion section as well as
> unnecessarily opinionated language in the introduction, which makes me
> doubt that it is ready for voting. Did anyone specifically review the
> RFC text for you?
>


Re: [PHP-DEV] What should we do with utf8_encode and utf8_decode?

2021-12-22 Thread Andreas Heigl

Hey all.

On 22.12.21 10:00, Rowan Tommins wrote:

On 21/12/2021 23:20, Wade Rossmann wrote:

I would suggest adding optional source/destination encoding parameters to
the functions, eg:

utf8_encode(string $string, string $source_encoding = "ISO-8859-1")
utf8_decode(string $string, string $destination_encoding = "ISO-8859-1")



That's an interesting idea, and definitely worth considering. In the 
much longer term, we could make the parameter mandatory rather than 
deprecating the entire function.


As you say, the challenge is how to implement the other encodings / what 
to do if ext/mbstring is not installed. It would be very tempting to 
support Windows-1252 directly, because it's just a few characters on top 
of the existing mappings, and is so commonly mistaken for ISO-8859-1. 
Anything else could then perhaps give a run-time error if ext/mbstring 
wasn't found. >


On 22/12/2021 00:31, Kris Craig wrote:

Now might be a good time to make this into an RFC.  :)



I have a draft kicking around with a lot of analysis of current usage. I 
will try to pick it back up after Christmas.



Regards,

To be quite honest: Despite the huge outcry that might provoke: I'd 
rather remove them today than keep them or deprecate them. And I'd 
declare the removal as a bug-fix!


Due to the way those functions are currently working they have caused 
more harm than actually good. One had to very explicitly know what they 
are doing to use them in the right way. And most certainly when they 
worked as expected that was more likely due to sheer luck than because 
someone knew what they were doing.


So giving those functions a continued lifetime either as an alias to 
mb_convert_encoding or by implementing the conversion to/from 
Windows-1252 would still leave people under the impression that it is a 
magic function.


I'd rather prefer to get rid of them and point people to the proper way 
of converting one character set to another one with all the possible 
mishaps that will occur.


Just my 0.02€

Cheers

Andreas
--
  ,,,
 (o o)
+-ooO-(_)-Ooo-+
| Andreas Heigl   |
| mailto:andr...@heigl.org  N 50°22'59.5" E 08°23'58" |
| https://andreas.heigl.org   |
+-+
| https://hei.gl/appointmentwithandreas   |
+-+


OpenPGP_0xA8D5437ECE724FE5.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature


Re: [PHP-DEV] What should we do with utf8_encode and utf8_decode?

2021-12-22 Thread Rowan Tommins

On 21/12/2021 23:20, Wade Rossmann wrote:

I would suggest adding optional source/destination encoding parameters to
the functions, eg:

utf8_encode(string $string, string $source_encoding = "ISO-8859-1")
utf8_decode(string $string, string $destination_encoding = "ISO-8859-1")



That's an interesting idea, and definitely worth considering. In the 
much longer term, we could make the parameter mandatory rather than 
deprecating the entire function.


As you say, the challenge is how to implement the other encodings / what 
to do if ext/mbstring is not installed. It would be very tempting to 
support Windows-1252 directly, because it's just a few characters on top 
of the existing mappings, and is so commonly mistaken for ISO-8859-1. 
Anything else could then perhaps give a run-time error if ext/mbstring 
wasn't found.



On 22/12/2021 00:31, Kris Craig wrote:

Now might be a good time to make this into an RFC.  :)



I have a draft kicking around with a lot of analysis of current usage. I 
will try to pick it back up after Christmas.



Regards,

--
Rowan Tommins
[IMSoP]

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