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

2016-10-24 Thread Yasuo Ohgaki
Hi all,

I didn't answer this question and would like to make my point of view clear.

On Thu, Oct 20, 2016 at 9:41 PM, Stephen Reay  wrote:
> 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?

The reason why I'm focusing on problems for inexperienced developers
is productivity with PHP.
IMHO, it is better to remove gocha whenever it is possible.

It's okay to read manual and search net to solve "obvious problem in
code". However, if 10K developers spend 10 hours to solve a problem,
100K hours of productivity with PHP is lost. The change may have small
impact, but small things add up. As long as there is reasonable
alternative way to implement advanced behaviors and small impact on
existing codes, it is better to provide easy and safe default
behaviors.

Making PHP easy to use and a productive language worths in the long
run. This is the reason why some of my proposals are focusing on
making PHP easy to use and safe to use by default.

e.g. Provide correct and safe session management by default, prevent
insane session usage and raise errors for them, add DbC support,
make uniqid more unique, consistent function names, disallow script
inclusion attacks, keep/improve URL rewriter rather than depreciating it
(URL rewriter is _very_ useful to keep private site private, i.e. Disallow
all cross site requests, therefore disallow CSRF, XSS completely.
PHP 7.1 has dedicated output buffer and setting for user URL rewrite.
It's easier and safer to use with PHP 7.1), etc.

PHP is popular because it is easy to use and productive. Let's keep
this and improve! Other languages/platforms are catching up.

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.

2016-10-22 Thread Stanislav Malyshev
Hi!

> 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)

These two already exist, and I see little value in changing their names
or introducing a slightly different form of it.

> - cookie_remove($name), cookie_list()
>   (These may be optional to you)

Isn't it the same as setcookie with empty value? But in general, I don't
have much objection to 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.

2016-10-21 Thread Yasuo Ohgaki
Hi Stephen,

On Fri, Oct 21, 2016 at 5:23 PM, Stephen Reay  wrote:
>> On 21 Oct 2016, at 13:32, Yasuo Ohgaki  wrote:
>>
>> 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.
>>
>
> I’m not quite sure I understand. I thought the whole point of new 
> standardised function names is to allow eventual deprecation and removal of 
> the inconsistent/confusing names?

In PHP4, we might have removed some functions on occasion. I don't
remember well. We have removed really unneeded functions in PHP5 like
register_globals related functions. I renamed number of pgsql
functions in PHP4, but I don't remove any aliases and I probably will
not.

Alias removal is very unlikely but possible. However, we have RFC for
such changes. Chance for removing setcookie() is almost none. We
cannot remove even problematic uniqid() probably.

Regarding to have array parameters, session_start() is made to accept
array parameter like this, but session_start() accepts void parameter
before. Array is used because it's very annoying to have setcookie()
like parameters.

>>>
>>> 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.
>>
>
> That’s a good point. So perhaps anything with a boolean value becomes a 
> keyword only? (i.e. keep it generic, with as little special-case handling of 
> keywords/named values as possible)

Oops, I mixed up session cookie and other cookie. I do have an idea to
set HttpOnly by default for session ID cookie, but not for other
cookies set by setcookie(). Rather than HttpOnly, we may have to
consider enabling "Secure" for setcookie() by default in near future.
Almost all web will enable TLS soon.

https://github.com/amphp/aerys/blob/9a7327f062aa678408dfe4f4c3c7f479db16f187/lib/Response.php#L49-L58
This is interface, so I cannot tell what it mean by "unkeyed values".
['HttpOnly'] means enable 'HttpOnly', probably.
(I'm willing to add new function entry points for consistency, but I
think API itself should be as simple as possible. It may sounds
contradictory with my filter validation improvement RFC, but the
proposal was made so that only implements mandatory API changes for
the purpose)

I would like to keep minimum feature, consistency and expandability.
'option_name' => 'option_value' format should be kept access all
functions. We do change some flags on occasion. Not only flipping
true/false default, but also bool to integer. i.e. on/off flags to
number of  int constant options.

>
>> I cannot think of date generation use case. Is there good use case?
>
> What do you mean? You can’t think of a situation where the developer would be 
> setting a cookie expiry date, or you can’t think of a reason to cast a date 
> time object to a string?

OK. You would like to use DateTime object's string.

Although we don't follow current RFC, but RFC has very specific rule
for cookie's expiration time range and format. Allowing string
expiration date is problematic and DateTime::timestamp() can be used
to set proper UNIX timestamp. So current way is good.

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.

2016-10-21 Thread Lester Caine
On 21/10/16 05:38, 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)

Or perhaps a new object model for managing header that can be developed
along side the procedural model ... what is being proposed for
setcookie() certainly seems more in line with that style of programming?

-- 
Lester Caine - G8HFL
-
Contact - http://lsces.co.uk/wiki/?page=contact
L.S.Caine Electronic Services - http://lsces.co.uk
EnquirySolve - http://enquirysolve.com/
Model Engineers Digital Workshop - http://medw.co.uk
Rainbow Digital Media - http://rainbowdigitalmedia.co.uk

-- 
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.

2016-10-21 Thread Stephen Reay

> On 21 Oct 2016, at 13:32, Yasuo Ohgaki  wrote:
> 
> 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.
> 

I’m not quite sure I understand. I thought the whole point of new standardised 
function names is to allow eventual deprecation and removal of the 
inconsistent/confusing names?


>> 
>> 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.
> 

That’s a good point. So perhaps anything with a boolean value becomes a keyword 
only? (i.e. keep it generic, with as little special-case handling of 
keywords/named values as possible)

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

What do you mean? You can’t think of a situation where the developer would be 
setting a cookie expiry date, or you can’t think of a reason to cast a date 
time object to a string?

Right now, it *only* accepts a unix timestamp, and then converts to a string 
(effectively doing date(DATE_COOKIE, $expires)). If you’re designing an api to 
support modern use, I would suggest that supporting DateTimeInterface objects 
or strings makes much more sense that requiring a unix timestamp. Like I said, 
I could live without the object support, and just let it cast any value to 
string, but *requiring* a timestamp only to convert back to a string seems odd 
to me. If you want to be universal, treat int’s as timestamps, cast objects to 
a string and send strings (casted or not) as-is.

> 
>> 
>> 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?

I don’t know the internals, but DATE_RFC1123/DateTime::RFC1123 is available in 
userland and seems correct to me.

> 
> 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.

2016-10-21 Thread 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



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

2016-10-20 Thread 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.

2016-10-20 Thread 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.

2016-10-20 Thread 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.

2016-10-20 Thread 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.

2016-10-20 Thread 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.

2016-10-20 Thread 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.

2016-10-20 Thread 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.

2016-10-20 Thread 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.

2016-10-20 Thread 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.

2016-10-20 Thread 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



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

2016-10-20 Thread 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.

2016-10-20 Thread 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.

2016-10-20 Thread 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.

2016-10-20 Thread 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



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

2016-10-20 Thread 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.

2016-10-20 Thread 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] header() removes all header of the same name.

2016-10-20 Thread 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.

2016-10-20 Thread 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
>
>


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

2016-10-20 Thread 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] header() removes all header of the same name.

2016-10-20 Thread 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] header() removes all header of the same name.

2016-10-20 Thread 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.

2016-10-20 Thread 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.

2016-10-20 Thread 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] header() removes all header of the same name.

2016-10-20 Thread 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.

2016-10-20 Thread Yasuo Ohgaki
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



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

2016-10-19 Thread Stanislav Malyshev
Hi!

> 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.

> Possible resolutions:
> 
>  - Prohibit 'Set-Cookie' for header() and force users to use setcookie()
>  - Mitigate by disabling replace flag by default. (This is not a good idea, 
> IMO)

I don't think we should do either.

> I would like to prohibit 'Set-Cookie' by header() because it may
> remove session ID cookie as well as auto login cookie, etc. If we
> leave released version as it is now, I would like to prohibit
> 'Set-Cookie' by header() in PHP 7.1.

I don't think it's a good idea. If somebody is using header(), it should
work like header() works. If you don't like how it works, use setcookie.

> Problem with this may be that user cannot modify 'Set-Cookie' header
> line as user want.
> 
> $ php -r 'setcookie("REMEMBERME=value; expires=Sat, 03-Sep-2020
> 05:38:43 GMT; path=/; domain=aaa");'
> PHP Warning:  Cookie names cannot contain any of the following '=,;
> \t\r\n\013\014' in Command line code on line 1

You are using setcookie() wrong here. See: http://php.net/setcookie

-- 
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.

2016-10-18 Thread Stephen Reay
Hi Yasuo,

I agree there are probably a lot using the default, but I think it’s reasonable 
to expect anyone using a header(‘Set-Cookie:..’); call rather than setcookie() 
to be aware of the 2nd argument for header(), so this solution sounds good to 
me.

Cheers

Stephen 

> On 18 Oct 2016, at 18:08, Yasuo Ohgaki  wrote:
> 
> Hi Stephen,
> 
> On Tue, Oct 18, 2016 at 5:54 PM, Stephen Reay  wrote:
>> If the replace flag was fixed, isn’t this then just a case of making sure 
>> userland sets replace to false if they want existing set-cookie headers 
>> retained?
> 
> Yes and no.
> 
> If users use the replace flag correctly, then it will work. However, I
> don't expect users set replace flag correctly. If replace flag's
> default was opposite, it would work better.
> 
>> Removing the ability to write a custom Set-Cookie header introduces a bigger 
>> problem than the current one, IMO.
> 
> OK. Let's just fix the replace flag and document removing 'Set-Cookie'
> header by header() may result in unwanted results.
> 
> Everyone is ok with this?
> 
> 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.

2016-10-18 Thread Yasuo Ohgaki
Hi Stephen,

On Tue, Oct 18, 2016 at 5:54 PM, Stephen Reay  wrote:
> If the replace flag was fixed, isn’t this then just a case of making sure 
> userland sets replace to false if they want existing set-cookie headers 
> retained?

Yes and no.

If users use the replace flag correctly, then it will work. However, I
don't expect users set replace flag correctly. If replace flag's
default was opposite, it would work better.

> Removing the ability to write a custom Set-Cookie header introduces a bigger 
> problem than the current one, IMO.

OK. Let's just fix the replace flag and document removing 'Set-Cookie'
header by header() may result in unwanted results.

Everyone is ok with this?

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.

2016-10-18 Thread Stephen Reay
(Apologies for the dupe, re-sending for the list.)

If the replace flag was fixed, isn’t this then just a case of making sure 
userland sets replace to false if they want existing set-cookie headers 
retained?

Removing the ability to write a custom Set-Cookie header introduces a bigger 
problem than the current one, IMO.

> On 18 Oct 2016, at 14:31, Yasuo Ohgaki  wrote:
> 
> Hi all,
> 
> I understand why header() is made to remove all headers of the same
> name. This is needed in some cases, but it does not work well for some
> cases.
> 
> We need to decide what to do with
> https://bugs.php.net/bug.php?id=72997
> 
> 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)
> 
> Since header() enables 'replace flag' by default, it removes all
> 'Set-Cookie' headers sent previously by default. It can easily disturb
> security related cookies to work. i.e. Session ID cookie, Auto Login
> cookie. This bug would be very hard to find for normal users, too.
> 
> Restoring older behavior (Removing only one header) cannot be a
> resolution because it can still disturb security related cookies.
> 
> Possible resolutions:
> 
> - Prohibit 'Set-Cookie' for header() and force users to use setcookie()
> - Mitigate by disabling replace flag by default. (This is not a good idea, 
> IMO)
> 
> Both resolution requires BC, but this is better to be fixed ASAP.
> 
> Non-BC resolution could be:
> - "Ask users to use setcookie() always for 'Set-Cookie'".
> 
> I would like to prohibit 'Set-Cookie' by header() because it may
> remove session ID cookie as well as auto login cookie, etc. If we
> leave released version as it is now, I would like to prohibit
> 'Set-Cookie' by header() in PHP 7.1.
> 
> Problem with this may be that user cannot modify 'Set-Cookie' header
> line as user want.
> 
> $ php -r 'setcookie("REMEMBERME=value; expires=Sat, 03-Sep-2020
> 05:38:43 GMT; path=/; domain=aaa");'
> PHP Warning:  Cookie names cannot contain any of the following '=,;
> \t\r\n\013\014' in Command line code on line 1
> 
> 
> Comments?
> 
> Regards,
> 
> --
> 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



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

2016-10-18 Thread Yasuo Ohgaki
Hi all,

I understand why header() is made to remove all headers of the same
name. This is needed in some cases, but it does not work well for some
cases.

We need to decide what to do with
https://bugs.php.net/bug.php?id=72997

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)

Since header() enables 'replace flag' by default, it removes all
'Set-Cookie' headers sent previously by default. It can easily disturb
security related cookies to work. i.e. Session ID cookie, Auto Login
cookie. This bug would be very hard to find for normal users, too.

Restoring older behavior (Removing only one header) cannot be a
resolution because it can still disturb security related cookies.

Possible resolutions:

 - Prohibit 'Set-Cookie' for header() and force users to use setcookie()
 - Mitigate by disabling replace flag by default. (This is not a good idea, IMO)

Both resolution requires BC, but this is better to be fixed ASAP.

Non-BC resolution could be:
  - "Ask users to use setcookie() always for 'Set-Cookie'".

I would like to prohibit 'Set-Cookie' by header() because it may
remove session ID cookie as well as auto login cookie, etc. If we
leave released version as it is now, I would like to prohibit
'Set-Cookie' by header() in PHP 7.1.

Problem with this may be that user cannot modify 'Set-Cookie' header
line as user want.

$ php -r 'setcookie("REMEMBERME=value; expires=Sat, 03-Sep-2020
05:38:43 GMT; path=/; domain=aaa");'
PHP Warning:  Cookie names cannot contain any of the following '=,;
\t\r\n\013\014' in Command line code on line 1


Comments?

Regards,

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

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