Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:99889
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain devilix.net designates 209.85.218.53 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <64.32.02884.7983D695@pb1.pair.com>
References: <64.32.02884.7983D695@pb1.pair.com>
Date: Tue, 18 Jul 2017 13:29:48 +0300
Message-ID: <CAPhkiZy1Xo4mj758mtjcMGnX2PV0VsiHa9uN_Ka=1xiiXAx7Mg@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 1:22 AM, Andreas Treichel <gmblar@gmail.com> wrote:
> Hi,
>
> i want some feedback, about the following idea before i write a rfc.
>
> Problem:
> The functions setcookie and setrawcookie has many parameters. Most of them
> are optional and extensions (e.g. same-site) make it even more messy.
>
> The functions setcookie and setrawcookie has 8 parameters.
>
> ...
>
> With the changes from https://wiki.php.net/rfc/same-site-cookie the
> functions setcookie and setrawcookie even has 9 parameters.
>
> ...
>
> How to change this?
> Add two new functions http_cookie_set and http_cookie_remove.
>

I think you are right on the money with this in general, but there's 3
things I would change:

> bool http_cookie_set(string $name, string $value [, array $options])
>
> $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.

> 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.

Anybody who'd use it, would have to read RFC1738 and/or RFC3986 to
know what they do.
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.

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

>
> bool http_cookie_remove(string $name)
>
> Why http_cookie_remove?
> I do not find it self explanatory that setcookie('foo', "") or
> setcookie('foo', NULL) delete a cookie.
>

3. http_cookie_remove() is an extra feature that you're bundling, but
has nothing to do with solving the original issue.

It MAY be something that people would want, but it also may not be,
and that makes it a contentions part of your would-be RFC.
There's certainly the possibility that some would vote against your
proposal entirely because of this ... just leave that for a future RFC
if this one passes.

---

That being said, again - I love the idea and would even like to work
with you on the RFC if you need any help.

Cheers,
Andrey.