Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:99909
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain devilix.net designates 209.85.218.52 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <86.8B.02884.8D93E695@pb1.pair.com>
References: <64.32.02884.7983D695@pb1.pair.com> <CAPhkiZy1Xo4mj758mtjcMGnX2PV0VsiHa9uN_Ka=1xiiXAx7Mg@mail.gmail.com>
 <86.8B.02884.8D93E695@pb1.pair.com>
Date: Tue, 18 Jul 2017 20:49:16 +0300
Message-ID: <CAPhkiZyuo+TtNNAd15hhQw07sA2QXaOD7xkHQ1Up7a3jT+FOsQ@mail.gmail.com>
To: Andreas Treichel <gmblar@gmail.com>
Cc: "internals@lists.php.net" <internals@lists.php.net>
Content-Type: text/plain; charset="UTF-8"
Subject: Re: [PHP-DEV] http_cookie_set and http_cookie_remove
From: narf@devilix.net (Andrey Andreev)

Hi Andreas,

On Tue, Jul 18, 2017 at 7:39 PM, Andreas Treichel <gmblar@gmail.com> wrote:
> Hello Andrey,
>
>>> $options are equal to the optional parameters of setcookie and
>>> setrawcookie.
>>> $options may contain:
>>>
>>> expires: int
>>> path: string
>>> domain: string
>>> secure: bool
>>> httponly: bool
>
>
>> 1. The wording here implies that these are the *only* attributes
>> allowed. In the interest of forward-compatibility, I'd allow arbitrary
>> attributes as well.
>
>
> This are the only supported options in the current implementation. Future
> extension like samesite cookies can add more options. Unknown options are
> ignored and trigger a warning.
>

That's what I was afraid of, and what I suggested be changed.

If we had a similar, array-of-attributes API that did NOT ignore or
trigger warnings for unknown attributes, everybody using PHP would've
been able to use SameSite already.

>>> encode is an additional option to remove the requirement of a raw and non
>>> raw function.
>>>
>>> encode: int
>>>     HTTP_COOKIE_ENCODE_NONE (same as setrawcookie)
>>>     HTTP_COOKIE_ENCODE_RFC1738 (same as setcookie)
>>>     HTTP_COOKIE_ENCODE_RFC3986
>
>
>> 2. I don't think this is necessary, nor that it belongs in the $options
>> array.
>
>
> Most users dont know the correct encoding for cookies. This idea is from the
> $enc_type parameter of http://php.net/http_build_query. The documentation of
> http_cookie_set() should explain it the same way.
>
> Maybe i can move it out of the $options array and add an extra parameter for
> the encoding, if the $options are the wrong location for this.
>

On another note, I'd also move the 'expire' option to a separate
parameter and remove $options to $attributes.

'expire' is not a known cookie attribute; PHP uses it to calculate the
Expires and Max-Age attributes ...

>
>> Anybody who'd use it, would have to read RFC1738 and/or RFC3986 to
>> know what they do.
>
>
> This is the same as setcookie(). No one has to read the rfc, which is not
> interested as it exactly works. HTTP_COOKIE_ENCODE_RFC1738 is the default
> for the encode option and encode the value the same ways as setcookie encode
> it.
>
> the default values for the options are the same as thr parameters for the
> current setcookie(). The default values for the $options:
>
> expires: int, default: 0
> path: string, default: ""
> domain: string, default: ""
> secure: bool, default: false
> httponly: bool, default: false
> encode: int, default: HTTP_COOKIE_ENCODE_RFC1738
>
>
>> And as the constant names aren't particularly short either, it is
>> easier for me to just apply an encoding function directly to $value
>> before passing it.
>
>
> The current names of the constants are not short, but in most cases i think
> you dont need it.
>
>
>> Also, RFC 6265 (section 4.1.1) only mentions Base64 as a suggestion
>> for encoding (and that's a SHOULD).
>> Link: https://tools.ietf.org/html/rfc6265#section-4.1.1
>
>
> http_cookie_set() use the same encoding per default as setcookie().
>

Sorry, but this is kind of pointless then. I liked your proposal,
because it's a chance to have a shiny new API that doesn't come with
all the legacy stuff already built into setcookie().

But if we want an array-based setcookie() alternative without changing
anything else, we can just change setcookie() to accept arrays. In
hindsight, if this is really what you wanted, then I have to agree
with Dan - that is building on foundation of sand.

Cheers,
Andrey.