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

Mocked response cookie persists after sending max-age=0 #2272

Closed
4 tasks done
TinsFox opened this issue Sep 10, 2024 · 7 comments · Fixed by #2275
Closed
4 tasks done

Mocked response cookie persists after sending max-age=0 #2272

TinsFox opened this issue Sep 10, 2024 · 7 comments · Fixed by #2275
Assignees
Labels
bug Something isn't working needs:triage Issues that have not been investigated yet. scope:browser Related to MSW running in a browser

Comments

@TinsFox
Copy link

TinsFox commented Sep 10, 2024

Prerequisites

Environment check

  • I'm using the latest msw version
  • I'm using Node.js version 18 or higher

Browsers

Chromium (Chrome, Brave, etc.)

Reproduction repository

https://github.com/TinsFox/mws-example

Reproduction steps

  1. pnpm install
  2. pnpm dev
  3. open chrome devtools
  4. Click Get User button and observe the console output
  5. Click Login button and observe the cookie in devtool
  6. Click Get User button and observe the console output, you can find cookie in output
  7. Click Logout button
  8. Click the Get User button and observe the console output. You will find that the cookie is still not empty, but the cookie set when logging in before.

Current behavior

Whether I clear the cookies through the API or manually clear the cookies, when I clear the cookies and then request the API, I can still get the old cookies.

Expected behavior

After I clear the cookie, the cookie obtained by requesting the API should be empty instead of the previously set one.

@TinsFox TinsFox added bug Something isn't working needs:triage Issues that have not been investigated yet. scope:browser Related to MSW running in a browser labels Sep 10, 2024
@TinsFox
Copy link
Author

TinsFox commented Sep 10, 2024

After a few hours of troubleshooting, I got some useful information. Everything works fine in v2.2.2, but I'm using v2.4.4.

I found that MSW_COOKIE_STORE in localStorage will be read and written in v2.4.4, but v2.2.2 will not.

@kettanaito
Copy link
Member

@TinsFox, thanks for reporting this!

Your observation is correct. We've moved away from a custom cookie store to using CookieJar (see #2206).

At first glance, looks like the CookieJar isn't reacting to you sending a max-age=0 response. This is likely an issue on our end.

When MSW received that max-age=0 response, here's what it does. First, it propagates it to document.cookie:

for (const cookieString of responseCookiePairs) {
// No need to parse the cookie headers because it's defined
// as the valid cookie string to begin with.
document.cookie = cookieString
}
}

When the next request happens, here's where it decides which cookies will be exposed to that request:

return {
...cookiesFromDocument,
...storedCookiesObject,
...cookiesFromHeaders,
}

There are multiple cookie groups:

  • Cookies present in document.cookie and matching the request's credentials property;
  • Cookies present in the request's cookie header;
  • Cookies present in the cookie store matching this request URL.

@kettanaito kettanaito changed the title The cookie is not read by the browser in real time Mocked response cookie persists after sending max-age=0 Sep 10, 2024
@kettanaito
Copy link
Member

I've added a failing test in #2275, which confirms that we've got a bug.

@kettanaito
Copy link
Member

I can confirm that the incorrectly present cookie comes from the cookie store:

Screenshot 2024-09-10 at 16 52 37

document.cookie is also '' empty as expected.

If I take a look at the store, I can see two entries for the same mockedCookie after receiving the mocked response with max-age=0:

Screenshot 2024-09-10 at 16 54 04

However, after the final GET request, the cookie store is (correctly) empty. Maybe that suggests that we aren't removing the max-age=0 cookies at the right time (e.g. removing them on getting the values instead of receiving the value in the store).

@kettanaito
Copy link
Member

Pushed a fix in 517c8f2.

@kettanaito kettanaito self-assigned this Sep 10, 2024
@kettanaito
Copy link
Member

Released: v2.4.5 🎉

This has been released in v2.4.5!

Make sure to always update to the latest version (npm i msw@latest) to get the newest features and bug fixes.


Predictable release automation by @ossjs/release.

@TinsFox
Copy link
Author

TinsFox commented Sep 11, 2024

@kettanaito Thanks!

But it doesn’t seem to be completely repaired.

I forgot to add a scenario where I would manually clear cookies from devtool.

I have upgraded my reproduction repository to 2.4.5. Please check again if there is any problem during this operation. Thank you very much!

Here is my operation video
https://share.cleanshot.com/qr2d8X4l

@github-actions github-actions bot locked and limited conversation to collaborators Oct 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working needs:triage Issues that have not been investigated yet. scope:browser Related to MSW running in a browser
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants