Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Yasuo Ohgaki]

Hi Anatol,

On Wed, Oct 19, 2016 at 8:20 PM, Anatol Belski  wrote:
>> I won't have time to write RFC for this, probably. I have many other things 
>> that I
>> would like to improve, like session error status handling improvement that I
>> recently proposed.
>>
> I see. It's a pity you won't have time to write an RFC. I see one already in 
> place on the wiki though. I see also your several other patches hanging on 
> gihtub. IMHO it is a real waste of time to abandon the work you've done, 
> without really pulling it through. With uniqid(), maybe it'd be even the 
> right decision to return to your original RFC, or just to reduce it to comply 
> with the simple patch variant. I'm sure, that no one wants to lose the good 
> contributions, even though it might take some effort to reach the common 
> ground sometimes.

I meant I wouldn't have time for RFC only replace php_combined_lcg().
I'll address uniqid() improvement sometime in the future.

To all,

We constantly get "uniqid() is not unique" bug reports.
In the meantime, any objection for adding following note to uniqid() manual.

"Do not make assumption for uniqid() output format, entropy
especially. uniqid() output format may be changed to provide
reasonably unique ID in future versions."


Regards,

--
Yasuo Ohgaki
yohg...@ohgaki.net

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

Re: [PHP-DEV] header() removes all header of the same name.

[Stephen Reay]

Hi All,

Just to make my earlier point of view crystal clear: As a purely userland party 
and someone maintaining a PHP framework, I don’t think it’s acceptable to limit 
which headers header()/header_remove() can operate on, particularly when the 
problem you’re trying to ‘solve’ is simply incorrect use of the functions 
available. It *is* possible to achieve any outcome desired with *correct* use 
of the header, session and cookie functions (and assuming the $replace argument 
to header() works correctly).

I still believe the way to solve this issue is with better information about 
usage, not by removing existing functionality.

So, please do *not* consider this to be an acceptable solution.

Cheers

Stephen


> On 20 Oct 2016, at 13:58, Yasuo Ohgaki  wrote:
> 
> Hi Stas,
> 
> I posted an an idea for preventing accidental cookie deletion.
> 'Set-Cookie' is a HTTP header, but provide dedicated functions for it. I 
> pasted
> it with a little modification.
> What do you think?
> 
> Bottom line is I would like to prevent lost session ID  by header()
> in the future.
> 
> Implement cookie_*() functions in 7.x, then prohibit 'Set-Cookie' for
> header() in 8.x
> 
> On Thu, Oct 20, 2016 at 1:39 PM, Stanislav Malyshev  
> wrote:
>>> There is 2 issues.
>>> - header() removes all headers of the same name including 'Set-Cookie'
>>> - header() ignores replace flag. (This one is easy to fix)
>> 
>> We have the flag, so if it doesn't work it should be fixed. Also, one
>> should use setcookie() for cookies, usually.
> 
> 
> Another idea for session ID cookie and Set-Cookie header protection.
> 
> Since we have setcookie() function, how about to have cookie
> dedicated functions for cookie header manipulation.
> 
> I'm about to create new feature request as follows:
> -
> Protect session ID and other cookies from header(), header_remove()
> -
> header() removes any previously defined headers.
> header('Set-Cookie: something') / header_remove() deletes session ID
> and other Set-Cookie headers. Cookies should be protected from
> header()/header_remove().
> 
> Instead, create new cookie functions
> 
> cookie_set() - Set cookie header (setcookie() alias)
> cookie_set_raw() - Set cookie header (setrawcookie alias)
> cookie_custom() - Set cookie with custom style.
>  (The same as header(sprintf('Set-Cookie:
> %s', $something));
> cookie_list() - Mostly the same as headers_list()
> cookie_remove([string $name]) - Mostly the same as header_remove()
> Remove cookie header. $name parameter is cookie name to be deleted.
> 
> Protect Set-Cookie headers from header() and header_remove()
> --
> 
> This implementation is cleaner because core to session
> dependency is not required. It is also good to have naming standard
> confirming cookie function names. i.e. Cookie functions should be
> named cookie_*() according to CODING_STANDARDS.
> 
> --
> Yasuo Ohgaki
> yohg...@ohgaki.net
> 
> -- 
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
> 


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

Re: [PHP-DEV] header() removes all header of the same name.

[Yasuo Ohgaki]

Hi Stephen,

On Thu, Oct 20, 2016 at 4:48 PM, Stephen Reay  wrote:
>
> Just to make my earlier point of view crystal clear: As a purely userland 
> party and someone maintaining a PHP framework, I don’t think it’s acceptable 
> to limit which headers header()/header_remove() can operate on, particularly 
> when the problem you’re trying to ‘solve’ is simply incorrect use of the 
> functions available. It *is* possible to achieve any outcome desired with 
> *correct* use of the header, session and cookie functions (and assuming the 
> $replace argument to header() works correctly).
>
> I still believe the way to solve this issue is with better information about 
> usage, not by removing existing functionality.
>
> So, please do *not* consider this to be an acceptable solution.

The idea is to separate HTTP header handling functions.

 - header*() for any HTTP headers except 'Set-Cookie'
 - cookie*() for only 'Set-Cookie' header

Developer does not lose anything.

As you mention, custom 'Set-Cookie' header is for advanced users.
Those people wouldn't have problems with new API.

With this new API, we'll have standard confirming function names for
cookie functions as a bonus, too. (I'm enthusiastic to clean up and
have consistent function names as you might know.)

Regards,

--
Yasuo Ohgaki
yohg...@ohgaki.net

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

Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Kalle Sommer Nielsen]

Hi Yasuo

2016-10-20 9:18 GMT+02:00 Yasuo Ohgaki :
> "Do not make assumption for uniqid() output format, entropy
> especially. uniqid() output format may be changed to provide
> reasonably unique ID in future versions."

Sounds reasonable to me; although I would phrase it a little
differently, something along the lines of:

The uniquid cannot be relied on to be unique and
there can occur collisions, even with the
more_entrophy set to &true;.

As for the in future version, although we may do that, I don't think
we should document something that is not in the core yet. What do you
think?

-- 
regards,

Kalle Sommer Nielsen
ka...@php.net

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

Re: [PHP-DEV] header() removes all header of the same name.

[Stephen Reay]

Hi Yasuo,


> On 20 Oct 2016, at 15:10, Yasuo Ohgaki  wrote:
> 
> Hi Stephen,
> 
> On Thu, Oct 20, 2016 at 4:48 PM, Stephen Reay  wrote:
>> 
>> Just to make my earlier point of view crystal clear: As a purely userland 
>> party and someone maintaining a PHP framework, I don’t think it’s acceptable 
>> to limit which headers header()/header_remove() can operate on, particularly 
>> when the problem you’re trying to ‘solve’ is simply incorrect use of the 
>> functions available. It *is* possible to achieve any outcome desired with 
>> *correct* use of the header, session and cookie functions (and assuming the 
>> $replace argument to header() works correctly).
>> 
>> I still believe the way to solve this issue is with better information about 
>> usage, not by removing existing functionality.
>> 
>> So, please do *not* consider this to be an acceptable solution.
> 
> The idea is to separate HTTP header handling functions.
> 
> - header*() for any HTTP headers except 'Set-Cookie'
> - cookie*() for only 'Set-Cookie' header
> 
> Developer does not lose anything.

The Developer *loses* what s/he has *now* - a fully functional header() 
function.

> 
> As you mention, custom 'Set-Cookie' header is for advanced users.
> Those people wouldn't have problems with new API.

Those people shouldn’t have problems using header() properly.

> 
> With this new API, we'll have standard confirming function names for
> cookie functions as a bonus, too. (I'm enthusiastic to clean up and
> have consistent function names as you might know.)
> 

I don’t have a problem with improved function names. I have a problem with 
removing functionality that is basically only ever going to be used by advanced 
developers, because it has *slightly* non obvious behaviour that could be fixed 
by a simple documentation note.

Please understand: *no* “solution" where header() loses the ability to write 
any arbitrary header will be acceptable in my opinion.

> Regards,
> 
> --
> Yasuo Ohgaki
> yohg...@ohgaki.net
> 

Cheers

Stephen


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

Re: [PHP-DEV] header() removes all header of the same name.

[Yasuo Ohgaki]

Hi Stephen,

On Thu, Oct 20, 2016 at 5:23 PM, Stephen Reay  wrote:
> Please understand: *no* “solution" where header() loses the ability to write 
> any arbitrary header will be acceptable in my opinion.

Thank you for feedback.
I'll include vote option for prohibiting 'Set-Cookie' for header*()

Regards,

--
Yasuo Ohgaki
yohg...@ohgaki.net

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

Re: [PHP-DEV] header() removes all header of the same name.

[Niklas Keller]

2016-10-20 10:28 GMT+02:00 Yasuo Ohgaki :

> Hi Stephen,
>
> On Thu, Oct 20, 2016 at 5:23 PM, Stephen Reay 
> wrote:
> > Please understand: *no* “solution" where header() loses the ability to
> write any arbitrary header will be acceptable in my opinion.
>
> Thank you for feedback.
> I'll include vote option for prohibiting 'Set-Cookie' for header*()
>
> Regards,
>
> --
> Yasuo Ohgaki
> yohg...@ohgaki.net
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
>
Hi Yasuo,

same here, it's not acceptable to limit header and restrict `set_cookie`.
Just think about all those frameworks that would have to specialcase
setting headers now and have to use the cookie API then.

If you want to protect the session cookie header, why not simply set it
right before the first output? That'd make it also non-overrideable, but
leaves header() intact. But I guess it's harder to implement.

Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Yasuo Ohgaki]

Hi Kalle,

On Thu, Oct 20, 2016 at 5:17 PM, Kalle Sommer Nielsen  wrote:
> 2016-10-20 9:18 GMT+02:00 Yasuo Ohgaki :
>> "Do not make assumption for uniqid() output format, entropy
>> especially. uniqid() output format may be changed to provide
>> reasonably unique ID in future versions."
>
> Sounds reasonable to me; although I would phrase it a little
> differently, something along the lines of:
>
> The uniquid cannot be relied on to be unique and
> there can occur collisions, even with the
> more_entrophy set to &true;.

I added warnings to uniqid() manual recently. It's visible now, could
you check this?

http://php.net/manual/en/function.uniqid.php

Warnings are based on following facts.

uniqid(); // without entropy

usleep(1) is called to get unique timestamp, but NTP can disturb and
uniqid() can result in the same ID.

uniqid('', TRUE); // with entropy

It's better, but entropy is based on system timestamp and there is no
usleep(1), so uniqid() is more sensitive to system clock adjustment by
NTP, and uniqid() can result in the same ID.

Collision is unlikely, but it not that unlikely with true CSPRNG based
entropy. Therefore, I made warning a little strong. With CSPRNG, we
may use more gentle warning. IMO.

> As for the in future version, although we may do that, I don't think
> we should document something that is not in the core yet. What do you
> think?

Entropy is some random value by definition, so we may tell users "Make
no assumption for entropy" at least. IMO.
Is this reasonable to you?

Regards,

--
Yasuo Ohgaki
yohg...@ohgaki.net

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

Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Yasuo Ohgaki]

Hi Kalle,

I forgot to mention one more thing.

On Thu, Oct 20, 2016 at 6:28 PM, Yasuo Ohgaki  wrote:
> Warnings are based on following facts.
>
> uniqid(); // without entropy
>
> usleep(1) is called to get unique timestamp, but NTP can disturb and
> uniqid() can result in the same ID.
>
> uniqid('', TRUE); // with entropy
>
> It's better, but entropy is based on system timestamp and there is no
> usleep(1), so uniqid() is more sensitive to system clock adjustment by
> NTP, and uniqid() can result in the same ID.
>
> Collision is unlikely, but it not that unlikely with true CSPRNG based
> entropy. Therefore, I made warning a little strong. With CSPRNG, we
> may use more gentle warning. IMO.

Application requires unique ID under across multi process/thread
tasks, it will have more chance to have collided unique ID.

Regards,

--
Yasuo Ohgaki
yohg...@ohgaki.net

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

Re: [PHP-DEV] header() removes all header of the same name.

[Yasuo Ohgaki]

Hi Niklas,

On Thu, Oct 20, 2016 at 6:01 PM, Niklas Keller  wrote:
>
> same here, it's not acceptable to limit header and restrict `set_cookie`.
> Just think about all those frameworks that would have to specialcase setting
> headers now and have to use the cookie API then.
>
> If you want to protect the session cookie header, why not simply set it
> right before the first output? That'd make it also non-overrideable, but
> leaves header() intact. But I guess it's harder to implement.

Although, I prefer to have completely separate API, we have to
implement vote result. So vote no for "Disabling 'Set-Cookie' for
header*()" vote option.

Regarding about delaying session cookie header, it is possible to use
output buffer to delay output so that session module can send HTTP
header at request shutdown. However, it will break almost all session
enabled applications that require immediate output. Therefore, it's
easy to implement, but not possible for this reason.

Regards,

--
Yasuo Ohgaki
yohg...@ohgaki.net

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

Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Pierre Joye]

On Thu, Oct 20, 2016 at 4:44 PM, Yasuo Ohgaki  wrote:

> Application requires unique ID under across multi process/thread
> tasks, it will have more chance to have collided unique ID.

uniqid fill(s|ed) some needs or maybe still fits for some.

However for modern application with many concurrent requests or nodes,
it does not fit anymore, since long. Do we need to fix a broken hammer
to fix the screw? I do not think so.

I suggested already to simply improve it if we can do it without
breaking BC and recommend to use something designed for such tasks,
UUID. ramsey/uuid is one of them, other are available.

Cheers,
-- 
Pierre

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

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

Re: [PHP-DEV] header() removes all header of the same name.

[Niklas Keller]

2016-10-20 11:57 GMT+02:00 Yasuo Ohgaki :

> Hi Niklas,
>
> On Thu, Oct 20, 2016 at 6:01 PM, Niklas Keller  wrote:
> >
> > same here, it's not acceptable to limit header and restrict `set_cookie`.
> > Just think about all those frameworks that would have to specialcase
> setting
> > headers now and have to use the cookie API then.
> >
> > If you want to protect the session cookie header, why not simply set it
> > right before the first output? That'd make it also non-overrideable, but
> > leaves header() intact. But I guess it's harder to implement.
>
> Although, I prefer to have completely separate API, we have to
> implement vote result. So vote no for "Disabling 'Set-Cookie' for
> header*()" vote option.
>

I don't have a vote. But this breaks BC. It might remove surprisings when
using sessions, but having header() not being able to set `set-cookie`
headers adds new surprisings.


> Regarding about delaying session cookie header, it is possible to use
> output buffer to delay output so that session module can send HTTP
> header at request shutdown. However, it will break almost all session
> enabled applications that require immediate output. Therefore, it's
> easy to implement, but not possible for this reason.


I meant squeeze in right before output or on first flush() call. There must
be a thing that sets a "already output" flag that prevents further headers.
We could use that mechanism to buffer all headers and just send them out
there and have a hook for the session module.

Regards, Niklas


> Regards,
>
> --
> Yasuo Ohgaki
> yohg...@ohgaki.net
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
>

[PHP-DEV] Re: [PHP-CVS] com php-src: This is a very very old thing dated back to year 2009. MYSQL_OPT_READ_TIMEOUT was never a macro in mysqlnd but an enum value. So this never actually worked correct

[Matteo Beccati]

Hi,

I have fixed the test for now, but I believe a mention to the new
constant should be added to NEWS and UPGRADING in master.


Cheers

On 18/10/2016 10:59, Matteo Beccati wrote:
> Hi Andrey,
> 
> you're probably aware of it by now, but in any case, this commit (or
> others related) seems to cause a test failure:
> 
> https://revive.beccati.com/bamboo/browse/PHP-SRC-TESTS-969/test/case/19172577
> 
> 
> Cheers
> 
> On 17/10/2016 17:58, Andrey Hristov wrote:
>> Commit:9594e364ed49d575dc27b887a996af10d18577ce
>> Author:Andrey Hristov  Mon, 17 Oct 
>> 2016 18:58:14 +0300
>> Parents:   e1f5b6d8dfe1543205d5b45d3dcf1d34f5e2e420
>> Branches:  master
>>
>> Link:   
>> http://git.php.net/?p=php-src.git;a=commitdiff;h=9594e364ed49d575dc27b887a996af10d18577ce
>>
>> Log:
>> This is a very very old thing dated back to year 2009.
>> MYSQL_OPT_READ_TIMEOUT was never a macro in mysqlnd but an enum value.
>> So this never actually worked correctly. mysqlnd provides these so it is
>> safe to have them when mysqlnd used.
>>
>> Changed paths:
>>   M  ext/mysqli/mysqli_api.c
>>   M  ext/mysqlnd/mysqlnd.c
>>   M  ext/mysqlnd/mysqlnd_net.c
>>
>>
>> Diff:
>> diff --git a/ext/mysqli/mysqli_api.c b/ext/mysqli/mysqli_api.c
>> index 4f66069..b806820 100644
>> --- a/ext/mysqli/mysqli_api.c
>> +++ b/ext/mysqli/mysqli_api.c
>> @@ -1759,14 +1759,14 @@ static int mysqli_options_get_option_zval_type(int 
>> option)
>>  #ifdef MYSQL_OPT_PROTOCOL
>>  case MYSQL_OPT_PROTOCOL:
>>  #endif /* MySQL 4.1.0 */
>> -#ifdef MYSQL_OPT_READ_TIMEOUT
>> +#if MYSQL_VERSION_ID > 40101 || defined(MYSQLI_USE_MYSQLND)
>>  case MYSQL_OPT_READ_TIMEOUT:
>>  case MYSQL_OPT_WRITE_TIMEOUT:
>>  case MYSQL_OPT_GUESS_CONNECTION:
>>  case MYSQL_OPT_USE_EMBEDDED_CONNECTION:
>>  case MYSQL_OPT_USE_REMOTE_CONNECTION:
>>  case MYSQL_SECURE_AUTH:
>> -#endif /* MySQL 4.1.1 */
>> +#endif
>>  #ifdef MYSQL_OPT_RECONNECT
>>  case MYSQL_OPT_RECONNECT:
>>  #endif /* MySQL 5.0.13 */
>> diff --git a/ext/mysqlnd/mysqlnd.c b/ext/mysqlnd/mysqlnd.c
>> index 4c8f27f..4290f9e 100644
>> --- a/ext/mysqlnd/mysqlnd.c
>> +++ b/ext/mysqlnd/mysqlnd.c
>> @@ -2333,10 +2333,8 @@ MYSQLND_METHOD(mysqlnd_conn_data, 
>> set_client_option)(MYSQLND_CONN_DATA * const c
>>  }
>>  switch (option) {
>>  case MYSQL_OPT_COMPRESS:
>> -#ifdef WHEN_SUPPORTED_BY_MYSQLI
>>  case MYSQL_OPT_READ_TIMEOUT:
>>  case MYSQL_OPT_WRITE_TIMEOUT:
>> -#endif
>>  case MYSQLND_OPT_SSL_KEY:
>>  case MYSQLND_OPT_SSL_CERT:
>>  case MYSQLND_OPT_SSL_CA:
>> diff --git a/ext/mysqlnd/mysqlnd_net.c b/ext/mysqlnd/mysqlnd_net.c
>> index 0fa6710..8ede642 100644
>> --- a/ext/mysqlnd/mysqlnd_net.c
>> +++ b/ext/mysqlnd/mysqlnd_net.c
>> @@ -820,13 +820,13 @@ MYSQLND_METHOD(mysqlnd_net, 
>> set_client_option)(MYSQLND_NET * const net, enum mys
>>  break;
>>  }
>>  case MYSQL_OPT_READ_TIMEOUT:
>> +DBG_INF("MYSQL_OPT_READ_TIMEOUT");
>>  net->data->options.timeout_read = *(unsigned int*) 
>> value;
>>  break;
>> -#ifdef WHEN_SUPPORTED_BY_MYSQLI
>>  case MYSQL_OPT_WRITE_TIMEOUT:
>> +DBG_INF("MYSQL_OPT_WRITE_TIMEOUT");
>>  net->data->options.timeout_write = *(unsigned int*) 
>> value;
>>  break;
>> -#endif
>>  case MYSQL_OPT_COMPRESS:
>>  net->data->options.flags |= 
>> MYSQLND_NET_FLAG_USE_COMPRESSION;
>>  break;
>>
>>
> 
> 


-- 
Matteo Beccati

Development & Consulting - http://www.beccati.com/

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

Re: [PHP-DEV] header() removes all header of the same name.

[Stephen Reay]

Hi Niklas,

There is even a userland hook for the specific functionality you mention: 
header_register_callback(). 

But I would argue that no fix is necessary. If you as a developer call 
session_start(), and then later call header(‘Set-Cookie:…’) with replace left 
as true, I think it’s safe to assume you’re either doing it deliberately, or 
that you’ll go read the documentation on sessions and header() to discover the 
problem. (Or possibly file a bug which will be marked as not-a-bug and refer 
you to the documentation).

The *only* solution that retains full control for the developer, is no change. 
Any “magic” about “untouchable” cookie headers (e.g. forcing the session cookie 
header after userland cookie headers) takes away options for the developer.

If *anything* the BC break being discussed should be to invert the default 
value for the $replace argument to header().

Cheers

Stephen 

> On 20 Oct 2016, at 17:39, Niklas Keller  wrote:
> 
> 2016-10-20 11:57 GMT+02:00 Yasuo Ohgaki  >:
> Hi Niklas,
> 
> On Thu, Oct 20, 2016 at 6:01 PM, Niklas Keller  > wrote:
> >
> > same here, it's not acceptable to limit header and restrict `set_cookie`.
> > Just think about all those frameworks that would have to specialcase setting
> > headers now and have to use the cookie API then.
> >
> > If you want to protect the session cookie header, why not simply set it
> > right before the first output? That'd make it also non-overrideable, but
> > leaves header() intact. But I guess it's harder to implement.
> 
> Although, I prefer to have completely separate API, we have to
> implement vote result. So vote no for "Disabling 'Set-Cookie' for
> header*()" vote option.
> 
> I don't have a vote. But this breaks BC. It might remove surprisings when 
> using sessions, but having header() not being able to set `set-cookie` 
> headers adds new surprisings.
>  
> Regarding about delaying session cookie header, it is possible to use
> output buffer to delay output so that session module can send HTTP
> header at request shutdown. However, it will break almost all session
> enabled applications that require immediate output. Therefore, it's
> easy to implement, but not possible for this reason.
> 
> I meant squeeze in right before output or on first flush() call. There must 
> be a thing that sets a "already output" flag that prevents further headers. 
> We could use that mechanism to buffer all headers and just send them out 
> there and have a hook for the session module.
> 
> Regards, Niklas
>  
> Regards,
> 
> --
> Yasuo Ohgaki
> yohg...@ohgaki.net 
> 
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php 
> 

Re: [PHP-DEV] header() removes all header of the same name.

[Yasuo Ohgaki]

Hi Niklas,

On Thu, Oct 20, 2016 at 7:39 PM, Niklas Keller  wrote:
> 016-10-20 11:57 GMT+02:00 Yasuo Ohgaki :
>>
>> Hi Niklas,
>>
>> On Thu, Oct 20, 2016 at 6:01 PM, Niklas Keller  wrote:
>> >
>> > same here, it's not acceptable to limit header and restrict
>> > `set_cookie`.
>> > Just think about all those frameworks that would have to specialcase
>> > setting
>> > headers now and have to use the cookie API then.
>> >
>> > If you want to protect the session cookie header, why not simply set it
>> > right before the first output? That'd make it also non-overrideable, but
>> > leaves header() intact. But I guess it's harder to implement.
>>
>> Although, I prefer to have completely separate API, we have to
>> implement vote result. So vote no for "Disabling 'Set-Cookie' for
>> header*()" vote option.
>
>
> I don't have a vote. But this breaks BC. It might remove surprisings when
> using sessions, but having header() not being able to set `set-cookie`
> headers adds new surprisings.

That's why we have minor releases and document.

Besides, I noted "add API 7.x, disallow header('Set-Cookie') by 8.0".
There are years to prepare for the change even if we decide to do so.

>
>>
>> Regarding about delaying session cookie header, it is possible to use
>> output buffer to delay output so that session module can send HTTP
>> header at request shutdown. However, it will break almost all session
>> enabled applications that require immediate output. Therefore, it's
>> easy to implement, but not possible for this reason.
>
>
> I meant squeeze in right before output or on first flush() call. There must
> be a thing that sets a "already output" flag that prevents further headers.
> We could use that mechanism to buffer all headers and just send them out
> there and have a hook for the session module.

The reason why I think of this new API proposal is to remove
core(main/SAPI.c) dependency to session. Your idea requires the
dependency (main/SAPI.c) like my the other proposal. Technically, it's
possible and it can be simpler by detecting and preventing session ID
deletion/modification.

Regards,

--
Yasuo Ohgaki
yohg...@ohgaki.net

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

Re: [PHP-DEV] [RFC] Driver-Specific PDO Param Types

[Matteo Beccati]

Hi Adam,

On 19/10/2016 00:05, Adam Baratz wrote:
> I've created an RFC to change how types are defined in PDO:
> https://wiki.php.net/rfc/driver-specific-pdo-param-types
> 
> Please share your feedback. I'm happy to hear thoughts about the pdo_dblib
> example, but the RFC is more about the possibility of driver-specific types
> than these particular ones.

I've had a cursory look. Looks good to me and it doesn't impact the
other drivers.

One little nitpick is that technically the PDO_PARAM_INPUT_OUTPUT flags
is now in the PDO_PARAM_DRIVER_SPECIFIC range and you add custom flags
that one day might overlap with newly added generic ones.

So maybe we need two driver specific ranges, e.g.

PDO_PARAM_DRIVER_SPECIFIC = 1000,

/* magic flag to denote a parameter as being input/output */
PDO_PARAM_INPUT_OUTPUT = 0x0001

/* this defines the start of the range for driver specific flags */
PDO_PARAM_DRIVER_SPECIFIC_FLAG = 0x0100

Since you're changing the API number, it shouldn't be a problem to
modify PDO_PARAM_INPUT_OUTPUT, and this should leave plenty of room for
new generic flags and both driver specific params and flags.


Cheers
-- 
Matteo Beccati

Development & Consulting - http://www.beccati.com/

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

Re: [PHP-DEV] header() removes all header of the same name.

[Yasuo Ohgaki]

Hi Stephen,

On Thu, Oct 20, 2016 at 8:24 PM, Stephen Reay  wrote:
> The *only* solution that retains full control for the developer, is no
> change. Any “magic” about “untouchable” cookie headers (e.g. forcing the
> session cookie header after userland cookie headers) takes away options for
> the developer.

My cookie*() functions proposal allows developers to remove header by
cookie_remove() and can send any cookie header by cookie_custom().
Therefore, developers have full control if they have to.

The only pain is that users may have to use cookie*() functions if we
disallow header('Set-Cookie') which will be a vote option. If there is
fully functional cookie*() functions, it will mitigate wrong
header('Set-Cookie') usage regardless of the vote result, hopefully.

Regards,

--
Yasuo Ohgaki
yohg...@ohgaki.net

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

Re: [PHP-DEV] header() removes all header of the same name.

[Stephen Reay]

Hi Yasuo,

As with Niklas, I have no vote, so my *only* option to prevent what I consider 
to be a bad decision, is to post to this thread and hope that enough of those 
who *do* have voting rights, reject the proposal.

I understand what you’re proposing. But honestly I don’t even agree with the 
premise that there is a *problem* that needs to be fixed.

Did you know that if you manually set the Content-Length header to less than 
the actual body length, many browsers (Safari, Chrome, and FF definitely) will 
stop reading/processing the response at the length you specify?
So should we also prevent setting Content-Length via header() ?

Honestly I don’t understand how this is still a discussion. The developer 
failed to set the $replace argument to false. At most I would expect this to 
result in a documentation note warning about the use of header(‘Set-Cookie…’).


I appreciate you trying to make improvements, and I’d *definitely* be in favour 
of the function naming cleanup you mentioned earlier, but all of this 
“protected” headers and extra function calls, because someone forgot to type “, 
false” seems ridiculous to me honestly.


Cheers

Stephen



> On 20 Oct 2016, at 18:41, Yasuo Ohgaki  wrote:
> 
> Hi Stephen,
> 
> On Thu, Oct 20, 2016 at 8:24 PM, Stephen Reay  
> wrote:
>> The *only* solution that retains full control for the developer, is no
>> change. Any “magic” about “untouchable” cookie headers (e.g. forcing the
>> session cookie header after userland cookie headers) takes away options for
>> the developer.
> 
> My cookie*() functions proposal allows developers to remove header by
> cookie_remove() and can send any cookie header by cookie_custom().
> Therefore, developers have full control if they have to.
> 
> The only pain is that users may have to use cookie*() functions if we
> disallow header('Set-Cookie') which will be a vote option. If there is
> fully functional cookie*() functions, it will mitigate wrong
> header('Set-Cookie') usage regardless of the vote result, hopefully.
> 
> Regards,
> 
> --
> Yasuo Ohgaki
> yohg...@ohgaki.net
> 


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

[PHP-DEV] Re: [PHP-CVS] com php-src: This is a very very old thing dated backto year 2009. MYSQL_OPT_READ_TIMEOUT was never a macro in mysqlnd but an enumvalue. So this never actually worked correctly

[Christoph M. Becker]

On 20.10.2016 at 12:57, Matteo Beccati wrote:

> I have fixed the test for now, 

Thanks!

> but I believe a mention to the new
> constant should be added to NEWS and UPGRADING in master.

In my opinion it is not necessary to add that to NEWS, but it's quite
important to add the info to UPGRADING, as the migration guides are
based on this file (and otherwise this addition will most likely be
overlooked).

Cheers,
Christoph

> Cheers
> 
> On 18/10/2016 10:59, Matteo Beccati wrote:
>> Hi Andrey,
>>
>> you're probably aware of it by now, but in any case, this commit (or
>> others related) seems to cause a test failure:
>>
>> https://revive.beccati.com/bamboo/browse/PHP-SRC-TESTS-969/test/case/19172577
>>
>>
>> Cheers
>>
>> On 17/10/2016 17:58, Andrey Hristov wrote:
>>> Commit:9594e364ed49d575dc27b887a996af10d18577ce
>>> Author:Andrey Hristov  Mon, 17 Oct 
>>> 2016 18:58:14 +0300
>>> Parents:   e1f5b6d8dfe1543205d5b45d3dcf1d34f5e2e420
>>> Branches:  master
>>>
>>> Link:   
>>> http://git.php.net/?p=php-src.git;a=commitdiff;h=9594e364ed49d575dc27b887a996af10d18577ce
>>>
>>> Log:
>>> This is a very very old thing dated back to year 2009.
>>> MYSQL_OPT_READ_TIMEOUT was never a macro in mysqlnd but an enum value.
>>> So this never actually worked correctly. mysqlnd provides these so it is
>>> safe to have them when mysqlnd used.
>>>
>>> Changed paths:
>>>   M  ext/mysqli/mysqli_api.c
>>>   M  ext/mysqlnd/mysqlnd.c
>>>   M  ext/mysqlnd/mysqlnd_net.c
>>>
>>>
>>> Diff:
>>> diff --git a/ext/mysqli/mysqli_api.c b/ext/mysqli/mysqli_api.c
>>> index 4f66069..b806820 100644
>>> --- a/ext/mysqli/mysqli_api.c
>>> +++ b/ext/mysqli/mysqli_api.c
>>> @@ -1759,14 +1759,14 @@ static int mysqli_options_get_option_zval_type(int 
>>> option)
>>>  #ifdef MYSQL_OPT_PROTOCOL
>>>  case MYSQL_OPT_PROTOCOL:
>>>  #endif /* MySQL 4.1.0 */
>>> -#ifdef MYSQL_OPT_READ_TIMEOUT
>>> +#if MYSQL_VERSION_ID > 40101 || defined(MYSQLI_USE_MYSQLND)
>>> case MYSQL_OPT_READ_TIMEOUT:
>>> case MYSQL_OPT_WRITE_TIMEOUT:
>>> case MYSQL_OPT_GUESS_CONNECTION:
>>> case MYSQL_OPT_USE_EMBEDDED_CONNECTION:
>>> case MYSQL_OPT_USE_REMOTE_CONNECTION:
>>> case MYSQL_SECURE_AUTH:
>>> -#endif /* MySQL 4.1.1 */
>>> +#endif
>>>  #ifdef MYSQL_OPT_RECONNECT
>>> case MYSQL_OPT_RECONNECT:
>>>  #endif /* MySQL 5.0.13 */
>>> diff --git a/ext/mysqlnd/mysqlnd.c b/ext/mysqlnd/mysqlnd.c
>>> index 4c8f27f..4290f9e 100644
>>> --- a/ext/mysqlnd/mysqlnd.c
>>> +++ b/ext/mysqlnd/mysqlnd.c
>>> @@ -2333,10 +2333,8 @@ MYSQLND_METHOD(mysqlnd_conn_data, 
>>> set_client_option)(MYSQLND_CONN_DATA * const c
>>> }
>>> switch (option) {
>>> case MYSQL_OPT_COMPRESS:
>>> -#ifdef WHEN_SUPPORTED_BY_MYSQLI
>>> case MYSQL_OPT_READ_TIMEOUT:
>>> case MYSQL_OPT_WRITE_TIMEOUT:
>>> -#endif
>>> case MYSQLND_OPT_SSL_KEY:
>>> case MYSQLND_OPT_SSL_CERT:
>>> case MYSQLND_OPT_SSL_CA:
>>> diff --git a/ext/mysqlnd/mysqlnd_net.c b/ext/mysqlnd/mysqlnd_net.c
>>> index 0fa6710..8ede642 100644
>>> --- a/ext/mysqlnd/mysqlnd_net.c
>>> +++ b/ext/mysqlnd/mysqlnd_net.c
>>> @@ -820,13 +820,13 @@ MYSQLND_METHOD(mysqlnd_net, 
>>> set_client_option)(MYSQLND_NET * const net, enum mys
>>> break;
>>> }
>>> case MYSQL_OPT_READ_TIMEOUT:
>>> +   DBG_INF("MYSQL_OPT_READ_TIMEOUT");
>>> net->data->options.timeout_read = *(unsigned int*) 
>>> value;
>>> break;
>>> -#ifdef WHEN_SUPPORTED_BY_MYSQLI
>>> case MYSQL_OPT_WRITE_TIMEOUT:
>>> +   DBG_INF("MYSQL_OPT_WRITE_TIMEOUT");
>>> net->data->options.timeout_write = *(unsigned int*) 
>>> value;
>>> break;
>>> -#endif
>>> case MYSQL_OPT_COMPRESS:
>>> net->data->options.flags |= 
>>> MYSQLND_NET_FLAG_USE_COMPRESSION;
>>> break;
>>>
>>>
>>
>>
> 
> 


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

Re: [PHP-DEV] header() removes all header of the same name.

[Niklas Keller]

2016-10-20 13:41 GMT+02:00 Yasuo Ohgaki :

> Hi Stephen,
>
> On Thu, Oct 20, 2016 at 8:24 PM, Stephen Reay 
> wrote:
> > The *only* solution that retains full control for the developer, is no
> > change. Any “magic” about “untouchable” cookie headers (e.g. forcing the
> > session cookie header after userland cookie headers) takes away options
> for
> > the developer.
>
> My cookie*() functions proposal allows developers to remove header by
> cookie_remove() and can send any cookie header by cookie_custom().
> Therefore, developers have full control if they have to.
>
> The only pain is that users may have to use cookie*() functions if we
> disallow header('Set-Cookie') which will be a vote option. If there is
> fully functional cookie*() functions, it will mitigate wrong
> header('Set-Cookie') usage regardless of the vote result, hopefully.
>

What about extensions to the `set-cookie` header? Take `SameSite` as a
recent example. The `setcookie` API doesn't cover that. Besides that, the
current `setcookie` API is awful, people just added more and more
parameters.

Before we even discuss disallowing `header("set-cookie")`, we should have a
sane cookie API, e.g. one that like `setcookie($name, $value, $flags)`.

That's also the way we implemented it in Aerys (
https://github.com/amphp/aerys/blob/9a7327f062aa678408dfe4f4c3c7f479db16f187/lib/Response.php#L49-L58).
It's a simple wrapper around `addHeader` to make life easier, but it
doesn't restrict developers to call `setHeader` and replace all
`set-cookie` headers.

Regards, Niklas

Re: [PHP-DEV] header() removes all header of the same name.

[Yasuo Ohgaki]

Hi Ptephen,

On Thu, Oct 20, 2016 at 9:15 PM, Stephen Reay  wrote:
> As with Niklas, I have no vote, so my *only* option to prevent what I 
> consider to be a bad decision, is to post to this thread and hope that enough 
> of those who *do* have voting rights, reject the proposal.

It's okay, but aren't I and framework developers on the same side?

I don't want to get bug report that session lost or some important
cookie lost somehow.

Framework developers don't want to get bug report that user logouts or
some important cookie lost somehow.

It's better that we have header*() and cookie*() function for
dedicated purpose, isn't it?

Regards,

--
Yasuo Ohgaki
yohg...@ohgaki.net

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

Re: [PHP-DEV] header() removes all header of the same name.

[Yasuo Ohgaki]

Hi Niklas,

On Thu, Oct 20, 2016 at 9:21 PM, Niklas Keller  wrote:
> 2016-10-20 13:41 GMT+02:00 Yasuo Ohgaki :
>>
>> Hi Stephen,
>>
>> On Thu, Oct 20, 2016 at 8:24 PM, Stephen Reay 
>> wrote:
>> > The *only* solution that retains full control for the developer, is no
>> > change. Any “magic” about “untouchable” cookie headers (e.g. forcing the
>> > session cookie header after userland cookie headers) takes away options
>> > for
>> > the developer.
>>
>> My cookie*() functions proposal allows developers to remove header by
>> cookie_remove() and can send any cookie header by cookie_custom().
>> Therefore, developers have full control if they have to.
>>
>> The only pain is that users may have to use cookie*() functions if we
>> disallow header('Set-Cookie') which will be a vote option. If there is
>> fully functional cookie*() functions, it will mitigate wrong
>> header('Set-Cookie') usage regardless of the vote result, hopefully.
>
>
> What about extensions to the `set-cookie` header? Take `SameSite` as a
> recent example. The `setcookie` API doesn't cover that. Besides that, the
> current `setcookie` API is awful, people just added more and more
> parameters.
>
> Before we even discuss disallowing `header("set-cookie")`, we should have a
> sane cookie API, e.g. one that like `setcookie($name, $value, $flags)`.

Sure I'll. I don't like it either.
Array can be used for this.

>
> That's also the way we implemented it in Aerys
> (https://github.com/amphp/aerys/blob/9a7327f062aa678408dfe4f4c3c7f479db16f187/lib/Response.php#L49-L58).
> It's a simple wrapper around `addHeader` to make life easier, but it doesn't
> restrict developers to call `setHeader` and replace all `set-cookie`
> headers.

The function does it :)

Regards,

--
Yasuo Ohgaki
yohg...@ohgaki.net

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

Re: [PHP-DEV] header() removes all header of the same name.

[Stephen Reay]

Hi Yasuo,

> On 20 Oct 2016, at 19:21, Yasuo Ohgaki  wrote:
> 
> Hi Ptephen,
> 
> On Thu, Oct 20, 2016 at 9:15 PM, Stephen Reay  
> wrote:
>> As with Niklas, I have no vote, so my *only* option to prevent what I 
>> consider to be a bad decision, is to post to this thread and hope that 
>> enough of those who *do* have voting rights, reject the proposal.
> 
> It's okay, but aren't I and framework developers on the same side?

Apparently not. I expect people using my work to read the documentation. I 
don’t remove mysql support because they don’t understand why a “DELETE * FROM 
foo” query deleted all their data. Similarly I don’t see why it’s a bug to 
remove headers when you leave $replace to the default of true.

> 
> I don't want to get bug report that session lost or some important
> cookie lost somehow.

Why is your concern so focussed on solving problems for inexperienced 
developers, who are effectively using functions incorrectly, at the expense of 
experienced developers who are doing the right thing?
This response effectively encourages bad behaviour (did the reporter even check 
the docs for header() to see why it’s replacing the session cookie?

How many bug reports do you think you’re going to get saying “header() no 
longer supports set-cookie headers”. 


> 
> Framework developers don't want to get bug report that user logouts or
> some important cookie lost somehow.
> 
> It's better that we have header*() and cookie*() function for
> dedicated purpose, isn't it?

Having dedicated functions for setting cookies, yes that makes sense.
Removing that functionality from header() makes no sense. A cookie is still a 
header.


> 
> Regards,
> 
> --
> Yasuo Ohgaki
> yohg...@ohgaki.net
> 




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

[PHP-DEV] BAD Benchmark Results for PHP Master 2016-10-20

[lp_benchmark_robot]

Results for project PHP master, build date 2016-10-20 06:26:14+03:00
commit: 03cd0f6
previous commit:0ffd0a0
revision date:  2016-10-20 01:17:55+03:00
environment:Haswell-EP
cpu:Intel(R) Xeon(R) CPU E5-2699 v3 @ 2.30GHz 2x18 cores, 
stepping 2, LLC 45 MB
mem:128 GB
os: CentOS 7.1
kernel: Linux 3.10.0-229.4.2.el7.x86_64

Baseline results were generated using release php-7.0.0, with hash 60fffd2 from
2015-12-01 04:16:47+00:00

---
benchmark   relative   change since   change since  
current rev run
std_dev*   last run   baseline  
   with PGO
---
:-|   Wordpress 4.2.2 cgi -T1  0.07% -0.15%  0.15%  
  7.44%
:-|   Drupal 7.36 cgi -T1  0.17%  0.25% -0.56%  
  4.53%
:-|   MediaWiki 1.23.9 cgi -T5000  0.00%  0.00%  0.51%  
  3.58%
:-(   bench.php cgi -T100  0.15% -1.25% 30.02%  
 -3.97%
:-(  micro_bench.php cgi -T10  0.05% -2.55% 11.94%  
  5.35%
:-|  mandelbrot.php cgi -T100  0.03% -0.65% 31.29%  
  5.37%
---

* Relative Standard Deviation (Standard Deviation/Average)

If this is not displayed properly please visit our results page here: 
http://languagesperformance.intel.com/bad-benchmark-results-for-php-master-2016-10-20/

Note: Benchmark results for Wordpress, Drupal, MediaWiki are measured in
fetches/second while all others are measured in seconds.
More details on measurements methodology at: 
https://01.org/lp/documentation/php-environment-setup.

Subject Label Legend:
Attributes are determined based on the performance evolution of the workloads
compared to the previous measurement iteration.
NEUTRAL: performance did not change by more than 1% for any workload
GOOD: performance improved by more than 1% for at least one workload and there
is no regression greater than 1%
BAD: performance dropped by more than 1% for at least one workload and there is
no improvement greater than 1%
UGLY: performance improved by more than 1% for at least one workload and also
dropped by more than 1% for at least one workload


Our lab does a nightly source pull and build of the PHP project and measures
performance changes against the previous stable version and the previous nightly
measurement. This is provided as a service to the community so that quality
issues with current hardware can be identified quickly.

Intel technologies' features and benefits depend on system configuration and may
require enabled hardware, software or service activation. Performance varies
depending on system configuration.


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

Re: [PHP-DEV] header() removes all header of the same name.

[Christoph M. Becker]

On 20.10.2016 at 14:15, Stephen Reay wrote:

> As with Niklas, I have no vote, so my *only* option to prevent what I 
> consider to be a bad decision, is to post to this thread and hope that enough 
> of those who *do* have voting rights, reject the proposal.
> 
> I understand what you’re proposing. But honestly I don’t even agree with the 
> premise that there is a *problem* that needs to be fixed.
> 
> Did you know that if you manually set the Content-Length header to less than 
> the actual body length, many browsers (Safari, Chrome, and FF definitely) 
> will stop reading/processing the response at the length you specify?
> So should we also prevent setting Content-Length via header() ?
> 
> Honestly I don’t understand how this is still a discussion. The developer 
> failed to set the $replace argument to false. At most I would expect this to 
> result in a documentation note warning about the use of header(‘Set-Cookie…’).
> 
> I appreciate you trying to make improvements, and I’d *definitely* be in 
> favour of the function naming cleanup you mentioned earlier, but all of this 
> “protected” headers and extra function calls, because someone forgot to type 
> “, false” seems ridiculous to me honestly.

Full ACK.

-- 
Christoph M. Becker

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

Re: [PHP-DEV] header() removes all header of the same name.

[Stanislav Malyshev]

Hi!

> The idea is to separate HTTP header handling functions.
> 
>  - header*() for any HTTP headers except 'Set-Cookie'
>  - cookie*() for only 'Set-Cookie' header

This does not look like a good design. First of all, HTTP spec allows
multiple instances of any header. Second, making function with unobvious
gotcha branch is usually bad design. Third, we are solving non-existing
problem here - people should just use existing functions correctly and
everything would be fine.
Let's not spend any more time on this.

-- 
Stas Malyshev
smalys...@gmail.com

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

Re: [PHP-DEV] header() removes all header of the same name.

[Yasuo Ohgaki]

Hi Stephen,

On Thu, Oct 20, 2016 at 9:41 PM, Stephen Reay  wrote:
>> I don't want to get bug report that session lost or some important
>> cookie lost somehow.
>
> Why is your concern so focussed on solving problems for inexperienced 
> developers, who are effectively using functions incorrectly, at the expense 
> of experienced developers who are doing the right thing?
> This response effectively encourages bad behaviour (did the reporter even 
> check the docs for header() to see why it’s replacing the session cookie?

The root cause of misuse is header() and setcookie() difference even
if both manipulate HTTP header.

 - header()  - Removes HTTP headers previously defined by default.
 - setcookie() - Appends 'Set-Cookie' HTTP header by default. Unlike
header(), no remove feature at all.

API design is inappropriate, IMHO.
I would like to help users by providing reasonable/expectable  APIs.
Current header() and setcookie() behavior is reasonable for a
individual feature, but mixing them seems not nice.

There are 3 people not in favor of 'Set-Cookie' protections in header()
Having consistent standard confirming function name means more to me,
I may remove 'Set-Cookie' header vote option, if nobody really cares
it, since I would like to have smooth RFC process.

Regards,

--
Yasuo Ohgaki
yohg...@ohgaki.net

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

Re: [PHP-DEV] header() removes all header of the same name.

[Yasuo Ohgaki]

Hi Stats,

On Fri, Oct 21, 2016 at 5:54 AM, Stanislav Malyshev  wrote:
>
>> The idea is to separate HTTP header handling functions.
>>
>>  - header*() for any HTTP headers except 'Set-Cookie'
>>  - cookie*() for only 'Set-Cookie' header
>
> This does not look like a good design. First of all, HTTP spec allows
> multiple instances of any header. Second, making function with unobvious
> gotcha branch is usually bad design. Third, we are solving non-existing
> problem here - people should just use existing functions correctly and
> everything would be fine.
> Let's not spend any more time on this.

OK. 4 people not in favor of Set-Cookie restriction for header*().

What about API clean up?
Since we have setcookie()/setrawcookie() already, we may clean up
current cookie API

e.g.
- cookie_set/setcookie($name, [$value, [array $options]])
  (Keep current signature also)
- cookie_set_raw/setrawcookie($name, [$value, [array $options]])
  (Keep current signature also)
- cookie_remove($name), cookie_list()
  (These may be optional to you)

Regards,

--
Yasuo Ohgaki
yohg...@ohgaki.net

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

Re: [PHP-DEV] header() removes all header of the same name.

[Guy Marriott]

FWIW Yasuo, I also think this is a bad idea. If you remove the ability to
set cookie _headers_ with the header function then the function needs a
more appropriate name - perhaps headerExceptCookie.

That makes 5 people opposed - 100% of the individuals who have responded in
this thread.

On Fri, Oct 21, 2016 at 10:41 AM, Yasuo Ohgaki  wrote:

> Hi Stats,
>
> On Fri, Oct 21, 2016 at 5:54 AM, Stanislav Malyshev 
> wrote:
> >
> >> The idea is to separate HTTP header handling functions.
> >>
> >>  - header*() for any HTTP headers except 'Set-Cookie'
> >>  - cookie*() for only 'Set-Cookie' header
> >
> > This does not look like a good design. First of all, HTTP spec allows
> > multiple instances of any header. Second, making function with unobvious
> > gotcha branch is usually bad design. Third, we are solving non-existing
> > problem here - people should just use existing functions correctly and
> > everything would be fine.
> > Let's not spend any more time on this.
>
> OK. 4 people not in favor of Set-Cookie restriction for header*().
>
> What about API clean up?
> Since we have setcookie()/setrawcookie() already, we may clean up
> current cookie API
>
> e.g.
> - cookie_set/setcookie($name, [$value, [array $options]])
>   (Keep current signature also)
> - cookie_set_raw/setrawcookie($name, [$value, [array $options]])
>   (Keep current signature also)
> - cookie_remove($name), cookie_list()
>   (These may be optional to you)
>
> Regards,
>
> --
> Yasuo Ohgaki
> yohg...@ohgaki.net
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
>

Re: [PHP-DEV] header() removes all header of the same name.

[Rick Widmer]

On 10/20/2016 4:58 PM, Guy Marriott wrote:

FWIW Yasuo, I also think this is a bad idea. If you remove the ability to
set cookie _headers_ with the header function then the function needs a
more appropriate name - perhaps headerExceptCookie.

That makes 5 people opposed - 100% of the individuals who have responded in
this thread.


6.



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

Re: [PHP-DEV] header() removes all header of the same name.

[Yasuo Ohgaki]

Hi Niklas and all,

On Thu, Oct 20, 2016 at 9:21 PM, Niklas Keller  wrote:
> Before we even discuss disallowing `header("set-cookie")`, we should have a
> sane cookie API, e.g. one that like `setcookie($name, $value, $flags)`.
>
> That's also the way we implemented it in Aerys
> (https://github.com/amphp/aerys/blob/9a7327f062aa678408dfe4f4c3c7f479db16f187/lib/Response.php#L49-L58).
> It's a simple wrapper around `addHeader` to make life easier, but it doesn't
> restrict developers to call `setHeader` and replace all `set-cookie`
> headers.

We choose current API for reason. It does not look pretty.
This is patch allow array config for 3rd param for setcookie().

https://gist.github.com/yohgaki/b86e07cd450777422c1a467166cd2fd3

I suppose some of us will have opinions having this kind of code(s).

Any comments?

Regards,

--
Yasuo Ohgaki
yohg...@ohgaki.net

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

Re: [PHP-DEV] header() removes all header of the same name.

[Yasuo Ohgaki]

On Fri, Oct 21, 2016 at 9:35 AM, Yasuo Ohgaki  wrote:
> On Thu, Oct 20, 2016 at 9:21 PM, Niklas Keller  wrote:
>> Before we even discuss disallowing `header("set-cookie")`, we should have a
>> sane cookie API, e.g. one that like `setcookie($name, $value, $flags)`.
>>
>> That's also the way we implemented it in Aerys
>> (https://github.com/amphp/aerys/blob/9a7327f062aa678408dfe4f4c3c7f479db16f187/lib/Response.php#L49-L58).
>> It's a simple wrapper around `addHeader` to make life easier, but it doesn't
>> restrict developers to call `setHeader` and replace all `set-cookie`
>> headers.
>
> We choose current API for reason. It does not look pretty.
> This is patch allow array config for 3rd param for setcookie().
>
> https://gist.github.com/yohgaki/b86e07cd450777422c1a467166cd2fd3
>
> I suppose some of us will have opinions having this kind of code(s).
>
> Any comments?

Execution example.

[yohgaki@dev github-php-src]$ cat t13.php
1, 'path'=>'foo',
'expires'=>time()+999, 'secure'=>1, 'domain'=>'example.com']);
setcookie('A', 'B', ['httponly'=>1] );
setcookie('A', 'B', 999);
setcookie('A', 'B', time()+999);

echo 'OK';
[yohgaki@dev github-php-src]$ ./php-cgi t13.php
X-Powered-By: PHP/7.2.0-dev
Set-Cookie: A=B; expires=Fri, 21-Oct-2016 02:55:31 GMT; Max-Age=999;
path=foo; domain=example.com; secure; HttpOnly
Set-Cookie: A=B; HttpOnly
Set-Cookie: A=B; expires=Thu, 01-Jan-1970 00:16:39 GMT; Max-Age=-1477016533
Set-Cookie: A=B; expires=Fri, 21-Oct-2016 02:55:31 GMT; Max-Age=999
Content-type: text/html; charset=UTF-8

OK

If PHP has named parameter, we don't need this patch.
Dose anyone working on named parameter?

One issue of this patch is strict types. It ruins strictly typed
parameter because array option parameters won't be checked by PHP.

--
Yasuo Ohgaki
yohg...@ohgaki.net

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

Re: [PHP-DEV] header() removes all header of the same name.

[Stephen Reay]

Is it normal to alter (or support multiple) function signatures like this, when 
you want to improve the name *and* improve the signature? Wouldn’t you just 
leave setcookie() as-is, introduce the new cookie_* functions, and then 
deprecate set cookie later? (ala mysql => mysqli)

As for the specifics - I kind of like.. Niklas (I think?) suggestion where the 
flags array accepts either key => value pairs, or non-keyed flag values. Any 
non-string key is ignored and the value used as a ‘flag’ (e.g. HttpOnly, 
Secure). Any non-string value would be casted to string.

This would obviously require slightly different usage by developers - the user 
would need to send a date/time (either a string or object that will cast to a 
string in the right format) instead of a timestamp for expires. If you want to 
special-case it, you could type-check for an instance of \DateTimeInterface and 
run ->format(\DateTime::COOKIE) instead of just casting to string, but I don’t 
think I’d consider that to be essential really. If the user can generate a UNIX 
timestamp, they should be able to format it to RFC1123 themselves too, no?

While you’re looking at this. DateTime::COOKIE (and DATE_COOKIE) seem to be 
using RFC850 format, but with a 4-digit year. Besides being a bit of a 
mis-match of formats, RFC850 is considered “obsolete” now, and perhaps should 
be replaced by RFC1123 (basically, no dashes, short day name).


Cheers

Stephen


> On 21 Oct 2016, at 09:51, Yasuo Ohgaki  wrote:
> 
> On Fri, Oct 21, 2016 at 9:35 AM, Yasuo Ohgaki  wrote:
>> On Thu, Oct 20, 2016 at 9:21 PM, Niklas Keller  wrote:
>>> Before we even discuss disallowing `header("set-cookie")`, we should have a
>>> sane cookie API, e.g. one that like `setcookie($name, $value, $flags)`.
>>> 
>>> That's also the way we implemented it in Aerys
>>> (https://github.com/amphp/aerys/blob/9a7327f062aa678408dfe4f4c3c7f479db16f187/lib/Response.php#L49-L58).
>>> It's a simple wrapper around `addHeader` to make life easier, but it doesn't
>>> restrict developers to call `setHeader` and replace all `set-cookie`
>>> headers.
>> 
>> We choose current API for reason. It does not look pretty.
>> This is patch allow array config for 3rd param for setcookie().
>> 
>> https://gist.github.com/yohgaki/b86e07cd450777422c1a467166cd2fd3
>> 
>> I suppose some of us will have opinions having this kind of code(s).
>> 
>> Any comments?
> 
> Execution example.
> 
> [yohgaki@dev github-php-src]$ cat t13.php
>  setcookie('A', 'B', ['httponly'=>1, 'path'=>'foo',
> 'expires'=>time()+999, 'secure'=>1, 'domain'=>'example.com']);
> setcookie('A', 'B', ['httponly'=>1] );
> setcookie('A', 'B', 999);
> setcookie('A', 'B', time()+999);
> 
> echo 'OK';
> [yohgaki@dev github-php-src]$ ./php-cgi t13.php
> X-Powered-By: PHP/7.2.0-dev
> Set-Cookie: A=B; expires=Fri, 21-Oct-2016 02:55:31 GMT; Max-Age=999;
> path=foo; domain=example.com; secure; HttpOnly
> Set-Cookie: A=B; HttpOnly
> Set-Cookie: A=B; expires=Thu, 01-Jan-1970 00:16:39 GMT; Max-Age=-1477016533
> Set-Cookie: A=B; expires=Fri, 21-Oct-2016 02:55:31 GMT; Max-Age=999
> Content-type: text/html; charset=UTF-8
> 
> OK
> 
> If PHP has named parameter, we don't need this patch.
> Dose anyone working on named parameter?
> 
> One issue of this patch is strict types. It ruins strictly typed
> parameter because array option parameters won't be checked by PHP.
> 
> --
> Yasuo Ohgaki
> yohg...@ohgaki.net
> 


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

Re: [PHP-DEV] header() removes all header of the same name.

[Yasuo Ohgaki]

Hi Stephen,

On Fri, Oct 21, 2016 at 1:38 PM, Stephen Reay  wrote:
> Is it normal to alter (or support multiple) function signatures like this, 
> when you want to improve the name *and* improve the signature? Wouldn’t you 
> just leave setcookie() as-is, introduce the new cookie_* functions, and then 
> deprecate set cookie later? (ala mysql => mysqli)

I'm lazy enough not to add new function entry point because the patch
is to show how it will look like.

Making aliases make life a little easier for both user and developer.
I don't think we will deprecate (I mean deprecate and remove in the
future) old functions. It will be there to avoid needless
incompatibility.

>
> As for the specifics - I kind of like.. Niklas (I think?) suggestion where 
> the flags array accepts either key => value pairs, or non-keyed flag values. 
> Any non-string key is ignored and the value used as a ‘flag’ (e.g. HttpOnly, 
> Secure). Any non-string value would be casted to string.
>
> This would obviously require slightly different usage by developers - the 
> user would need to send a date/time (either a string or object that will cast 
> to a string in the right format) instead of a timestamp for expires. If you 
> want to special-case it, you could type-check for an instance of 
> \DateTimeInterface and run ->format(\DateTime::COOKIE) instead of just 
> casting to string, but I don’t think I’d consider that to be essential 
> really. If the user can generate a UNIX timestamp, they should be able to 
> format it to RFC1123 themselves too, no?

I have an idea to enable HttpOnly by default. Most applications will
be safer by this change without any problems, but some applications
may need to disable HttpOnly. So it's better to stick with key =>
value pairs in case we change the default.

I cannot think of date generation use case. Is there good use case?

>
> While you’re looking at this. DateTime::COOKIE (and DATE_COOKIE) seem to be 
> using RFC850 format, but with a 4-digit year. Besides being a bit of a 
> mis-match of formats, RFC850 is considered “obsolete” now, and perhaps should 
> be replaced by RFC1123 (basically, no dashes, short day name).

Good idea. It's not updated since 90's, I guess. The same topic pops
up on occasion.

https://tools.ietf.org/html/rfc6265#section-5.1.1
Do we have this algorithm somewhere already?

Regards,

--
Yasuo Ohgaki
yohg...@ohgaki.net

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