Removing cookies by name

[imbolc]

Maybe you're willing to accept something like this here?

The api would become

CookieJar::remove(&mut self, name: impl Into<RemovalCookie>);

pub enum RemovalCookie {
    Cookie(Cookie<'static>),
    Name(Cow<'static, str>),
}

impl From<&'static str> for RemovalCookie {}
impl From<String> for RemovalCookie {}
impl From<Cookie<'static>> for RemovalCookie {}

The implementation is here: https://github.com/imbolc/tower-cookies/blob/2bbd9226188c6a18c80086b397aa34fcc42d8f68/src/lib.rs#L198

[Zerowalker]

I second this, being the one that pushed @imbolc into making this by asking/requesting this at https://github.com/imbolc/tower-cookies/issues/14

[SergioBenitez]

I'm not sure what's being proposed, exactly. Given the API you're suggesting, it seems like the crux of the proposal is to allow cookies to be removed via either 1) an &str name (this is new) or 2) a Cookie (as it is today). Am I understanding correctly?

It seems that given https://github.com/imbolc/tower-cookies/issues/14#issuecomment-1086573478, the reason the API is as it is today is understood: to avoid the common case where a user thinks a cookie will be removed but, because of a mismatched path or domain, the browser will fail to do so because the incorrect removal cookie was generated. I think any API that makes it trivial to remove a Cookie without specifying a path and/or domain will bring about difficulties.

Given that, perhaps we could consider an API that looks like:

fn remove_named<P, D>(&mut self, name: &str, path: P, domain: D)
    where P: Into<Option<&str>>, D: Into<Option<&str>>;

Such an API use would look like:

let mut jar = some_jar;
jar.remove_named("cookie_name", None, None);
jar.remove_named("cookie_name", "/", None);
jar.remove_named("cookie_name", None, "crates.io");
jar.remove_named("cookie_name", "/", "crates.io");

This is in contrast to the API now:

let mut jar = some_jar;
jar.remove(Cookie::named("cookie_name"));
jar.remove(Cookie::build("cookie_name", "").path("/").finish());
jar.remove(Cookie::build("cookie_name", "").domain("crates.io").finish());
jar.remove(Cookie::build("cookie_name", "").path("/").domain("crates.io").finish());

The first API suffers from unnamed parameters in an API that may be security sensitive while being pithy. The latter suffers from verbosity but is fairly clear otherwise. The latter also has the advantage that you can take a Cookie from the client and pass it directly to remove() and be guaranteed that the cookie will be properly removed. This is an important feature.

Both seem unideal in some ways, so let me propose yet another idea:

// Change `remove()` to:
fn remove<C: Into<Cookie<'_>>(&mut self, cookie: C);

// Change `Cookie::named()` to:
pub fn named<N>(name: N) -> CookieBuilder<'c>
    where N: Into<Cow<'c, str>>;

// Add the following:
impl Into<Cookie<'_>> for CookieBuilder<'_>;

Now, the API use would look like:

let mut jar = some_jar;
jar.remove(Cookie::named("cookie_name"));
jar.remove(Cookie::named("cookie_name").path("/"));
jar.remove(Cookie::named("cookie_name").domain("crates.io"));
jar.remove(Cookie::named("cookie_name").path("/").domain("crates.io"));

This is almost as pithy as the remove_named API while also being extremely clear and affording the "pass an existing cookie to be removed". What do you think? Would this resolve the original concern, or is there a strong desire to be able to remove without creating a Cookie at all?

[imbolc]

1. an &str name (this is new) or 2) a Cookie (as it is today). Am I understanding correctly?

Exactly

user thinks a cookie will be removed but, because of a mismatched path or domain, the browser will fail to do so because the incorrect removal cookie was generated

Right. I've learned about path and domain years ago, but frameworks I used afterwards let me happily forget about them e.g.:

• Django HttpResponse.delete_cookie(key, path='/', domain=None, samesite=None)
• ExpressJs res.clearCookie(name [, options])

Even Cookie::new doesn't require domain and path to be specified, so I don't see why remove should:

    pub fn new<N, V>(name: N, value: V) -> Self
    {
        Cookie {
            ...
            domain: None,
            path: None,

I think any API that makes it trivial to remove a Cookie without specifying a path and/or domain will bring about difficulties.

Would you maybe elaborate on this? In particular I don't see how passing Cookie::named("cookie_name") instead of cookie_name makes the removal less trivial.

The first API suffers from unnamed parameters in an API that may be security sensitive while being pithy. The latter suffers from verbosity but is fairly clear otherwise.

Agreed, I certainly wouldn't use jar.remove_named("cookie_name", None, None);. It's not clear if it's (path, domain) or (domain, path) and the compiler won't help here as both are strings.

Would this resolve the original concern, or is there a strong desire to be able to remove without creating a Cookie at all?

It's nice to remove a need for .finish() regardless of the issue. But my idea is about making the api more intuitive and it seems to me that people, coming from other frameworks, expect removal by a string.

[SergioBenitez]

Finally address and released in v0.18.0-rc.0 - CHANGELOG incoming, but docs and message in 49ff7b07811ce823d649287f2b560900e3352f8c should suffice for now.

If you could give a try, I'd be grateful.