dflydev-fig-cookies: SetCookie::expire does not work: missing Domain part

SetCookie::expire() does not work for response cookies because browsers expect a “Domain” part in the Set-Cookie header when expiring a cookie.

I have tested it with Firefox 49.0.1 and Chrome 53.0.2785.143 and both behave in the same (standard?) way: the minimum requirement for expiring and flushing a cookie is to have the cookie name, the “Expires” part in the past AND also a “Domain” part in the Set-Cookie header.

I could not really make a patch for this, since in response cookies no domain information is present. Any suggestion how to fix it in a canonical way?

About this issue

Original URL
State: open
Created 8 years ago
Comments: 24 (10 by maintainers)

Most upvoted comments

Hmm, sorry about that. Bad to read when you get back from a holiday. 😉

So if I understand this correctly, only the static methods (SetCookie::createExpired() and FigResponseCookies::expire()) don’t make any sense, because you’d need to set the domain first.

So, to expire a cookie that was previously created, you would probably write something like the following:

$expireCookie = $this->getCookieTheWayItWasCreated()->expire();

//... and now attach that to the response

I can only suggest to release a new minor version that deprecates these pointless static methods (as people probably couldn’t use them properly anyway), and then remove them completely with the next major release.

Again, sorry for the mess - I don’t think there’s much more we can do here.

franzliedke on Oct 18, 2016

For this fix, I’d recommend something like this. Happy to hear your thoughts on it:

    /**
     * @param ResponseInterface $response
     * @param string $cookieName
     * @param string $domain
     *
     * @return ResponseInterface
     */
    public static function expire(ResponseInterface $response, $cookieName, $domain = null)
    {
        $setCookie = SetCookie::createExpired($cookieName);

        if ($domain) {
            $setCookie = $setCookie->withDomain($domain);
        }

        return static::set($response, $setCookie);
    }

Since it is a static facade this will hopefully be somewhat BC as the $domain parameter will allow existing calls to FigRequestCookies::expire() without a $domain parameter to work.

Thoughts?

simensen on Oct 7, 2016

Sigh.

Somehow when #4 came through I knew that this was going to be trouble but I couldn’t quite put my finger on it. 😕 In order to be expired, cookies need to be defined exactly like they previously were with the exception of the value and the expiration time. So you cannot simply “delete a cookie” knowing just the name.

It would be nice to get @franzliedke feedback on this as hopefully they’ve used it by now and have overcome some of these issues?

At this point I’m tempted to suggest we try to find a way to rewrite this method entirely or drop it completely.

simensen on Oct 11, 2016