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

Possible bug: when multiple "cookie" fileds present in Request.headers, instead of separated by ";" under a single "cookie" key, failed to fetch all of them into Request.cookies #573

Closed
gengjun opened this issue Dec 18, 2023 · 3 comments · Fixed by #575

Comments

@gengjun
Copy link

gengjun commented Dec 18, 2023

Describe the bug
I see this really odd behavior: when testing my site on my local desktop, without https enabled, I noticed if there're two cookies: one is my session.id, another one is added by a script (such as google analytics) or manually added by hands (google chrome->developer tools -> application -> Cookies), the requests are sent with two cookies under a single cookie key "cookie" when print out req:

"cookie": "test=test; session.id=SQotRLwglxKyE+Q8UxUmSvGGD7yyJqxD"

I guess this is "correct" behavior since the session can be fetched without problem and works as expected. But when the same thing is tested in production with https enabled, I noticed the two cookies are now in separate keys (yes, the HeaderMap's key is not unique), something like this:

        "cookie": "test=test",
        "cookie": "session.id=lfbGFuZwQAAAAAAAAAInpoIg%3D%3D"

If I print out req.cookies(), it only contains the first cookie:

req.cookies() = CookieJar {
    original_cookies: {
        DeltaCookie {
            cookie: Cookie {
                cookie_string: Some(
                    "test=test",
                ),
                name: Indexed(
                    0,
                    4,
                ),
                value: Indexed(
                    5,
                    9,
                ),
                expires: None,
                max_age: None,
                domain: None,
                path: None,
                secure: None,
                http_only: None,
                same_site: None,
            },
            removed: false,
        },
    },
    delta_cookies: {},
}

As a result, the session fetched from depot will be None. I think this is the relevant code:
request.rs

        // Set the request cookies, if they exist.
        #[cfg(feature = "cookie")]
        let cookies = if let Some(header) = headers.get("Cookie") {
            let mut cookie_jar = CookieJar::new();
            if let Ok(header) = header.to_str() {
                for cookie_str in header.split(';').map(|s| s.trim()) {
                    if let Ok(cookie) = Cookie::parse_encoded(cookie_str).map(|c| c.into_owned()) {
                        cookie_jar.add_original(cookie);
                    }
                }
            }
            cookie_jar
        } else {
            CookieJar::new()
        };

To Reproduce
As described above, I can only reproduce this with acme / https enabled.

Expected behavior
all cookies should be put into Request.cookies, thus session can be fetched from CookieStore as expected.

Desktop (please complete the following information):

  • OS: ubuntu 22.04.3 LTS
  • Browser: google chrome
  • Version: Version 117.0.5938.92 (Official Build) (64-bit)
@gengjun
Copy link
Author

gengjun commented Dec 19, 2023

maybe just use HeaderMap.get_all() here ?
http.HeaderMap

@chrislearn
Copy link
Member

I fixed it. please check gihub main branch

@gengjun
Copy link
Author

gengjun commented Dec 19, 2023

i tested, i think it's fixed! Thanks!

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

Successfully merging a pull request may close this issue.

2 participants