Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove cookie doesn't seem to work in sub-sub paths like '/foo/bar' #34

Open
Zerowalker opened this issue Jul 31, 2024 · 2 comments
Open

Comments

@Zerowalker
Copy link

I noticed that removing cookies didn't work for me, and after troubleshooting I think I have identified it as being related to the paths.
When you remove cookies, the remove part just sets the max_age and expired, but ignores the path.
This seems to work if it's just / or /foo, but not if there's any more paths for some reason.

Here's an example that doesn't work, based on the private/signed example,
it should create and remove the cookie every other turn, but it doesn't work.
Change the path to just / or /foo and it works.
I am aware i specify the path when building the cookie, but that should not break it, as that makes the remove "unstable".

use axum::{routing::get, Router};
use std::net::SocketAddr;
use std::sync::OnceLock;
use tower_cookies::{Cookie, CookieManagerLayer, Cookies, Key};

const COOKIE_NAME: &str = "visited_private";
static KEY: OnceLock<Key> = OnceLock::new();

#[tokio::main]
async fn main() {
    let my_key: &[u8] = &[0; 64]; // Your real key must be cryptographically random
    KEY.set(Key::from(my_key)).ok();

    let app = Router::new()
        .route("/foo/bar", get(handler))
        .layer(CookieManagerLayer::new());

    let addr = SocketAddr::from(([127, 0, 0, 1], 3000));
    let listener = tokio::net::TcpListener::bind(&addr).await.unwrap();
    axum::serve(listener, app.into_make_service())
        .await
        .unwrap();
}

pub fn create_cookie() -> Cookie<'static> {
    let ck = Cookie::build(Cookie::new(COOKIE_NAME, ""))
        .path("/")
        .build();
    ck
}

async fn handler(cookies: Cookies) -> String {
    let key = KEY.get().unwrap();
    let private_cookies = cookies.signed(key); // You can use `cookies.signed` as well

    let ck = private_cookies.get(COOKIE_NAME);

    if let Some(ck) = ck {
        private_cookies.remove(ck);
        "Counter has been reset".into()
    } else {
        private_cookies.add(create_cookie());
        "".into()
    }
}
@imbolc
Copy link
Owner

imbolc commented Aug 22, 2024

Hi :) Sorry for the late response. Let me know if you've solved this already.

I remember there was already an issue when someone tried to remove the cookie using the extracted one. I guess it's about the path, notice it's None:

[examples/bug.rs:39:9] ck = Cookie {
    cookie_string: Some(
        " visited_private=thNnmggU2ex3L5XXeMNfxf8Wl8STcVZTxscSFEKSxa0=",
    ),
    name: Indexed(
        1,
        16,
    ),
    value: Concrete(
        "",
    ),
    expires: None,
    max_age: None,
    domain: None,
    path: None,
    secure: None,
    http_only: None,
    same_site: None,
    partitioned: None,
}

If I just create a new cookie the code works:

private_cookies.remove(create_cookie());

But I personally always call Cookie::make_removal just in case.

I don't have time right now to investigate the issue. But if you'd like to dig into it, I'd start with trying to reproduce the bug using just underlying cookie::CookieJar to be sure the bug is belong to this crate.

@Zerowalker
Copy link
Author

No worries!:)

Haven't solved it, but great discovery about the extracted one being the issue here.

I did a small test with cookie::CookieJar and it doesn't seem to change the path when getting it.

fn test() {
       use cookie::{Cookie, CookieJar};

       let mut jar = CookieJar::new();

       let ck = Cookie::build(("name", "gnu")).path("/").build();
       jar.add(ck.clone());
       let gck = jar.get("name").unwrap();

       println!("{:?}\n", ck);
       println!("{:?}", gck);
   }

this will print out this:

Cookie { cookie_string: None, name: Concrete("name"), value: Concrete("gnu"), expires: None, max_age: None, domain: None, path: Some(Concrete("/")), secure: None, http_only: None, same_site: None, partitioned: None }

Cookie { cookie_string: None, name: Concrete("name"), value: Concrete("gnu"), expires: None, max_age: None, domain: None, path: Some(Concrete("/")), secure: None, http_only: None, same_site: None, partitioned: None }

Which compared to the tower_cookies does have it's path intact.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants