mswjs / msw

Seamless REST/GraphQL API mocking library for browser and Node.js.
https://mswjs.io
MIT License
15.69k stars 503 forks source link

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

Closed TinsFox closed 1 week ago

TinsFox commented 1 week ago

Prerequisites

Environment check

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 commented 1 week ago

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 commented 1 week ago

@TinsFox, thanks for reporting this!

Your observation is correct. We've moved away from a custom cookie store to using CookieJar (see https://github.com/mswjs/msw/pull/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:

https://github.com/mswjs/msw/blob/1263c0f03f2ea9b20b0bee80adaa261f0e0ed67c/src/core/utils/HttpResponse/decorators.ts#L67-L72

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

https://github.com/mswjs/msw/blob/1263c0f03f2ea9b20b0bee80adaa261f0e0ed67c/src/core/utils/request/getRequestCookies.ts#L67-L71

There are multiple cookie groups:

kettanaito commented 1 week ago

I've added a failing test in https://github.com/mswjs/msw/pull/2275, which confirms that we've got a bug.

kettanaito commented 1 week ago

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 commented 1 week ago

Pushed a fix in 517c8f265dee1e7e37c20d9966a58c8b2a5a1ddc.

kettanaito commented 1 week ago

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 commented 1 week ago

@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