fix: encode cookie string before forwarding request cookies

[Jungzl]

resolve #1814

[Fallsleep]

maybe this'll better, in response can get decoded cookie value

function getAllDocumentCookies() {
  // keep origin value
  return cookieUtils.parse(document.cookie, { decode: (str: string) => str})
}

export function getAllRequestCookies(request: Request): Record<string, string> {
  //other code
  for (const [name, value] of Object.entries(forwardedCookies)) {
    request.headers.append('cookie', `${name}=${value}`)
    // urldecode here
    if(value.indexOf("%") !== -1){
      forwardedCookies[name] = decodeURIComponent(value)
    }
  }

  return {
    ...forwardedCookies,
    ...cookiesFromHeaders,
  }
}

[kettanaito]

Thanks for working on this, @Jungzl!

I have one question: cookie encoding is necessary to support non-standard cookie values, like the original issue reports. Hence, I wonder if it doesn't make more sense to encode/decode the cookie on your side?

http.post('/login', () => {
  return new HttpResponse(null, {
    headers: {
      'Set-Cookie': `name=${encodeURIComponent('测试内容')}`
    }
  })
})

This way the cookie will persist in its encoded state. You can decode it anywhere you need its original value, which is what you're doing anyway if supplying header names/values with non-standard characters. Since it's you who are using that value, and you who needs to decode it anyway, it makes sense that you will be encoding/decoding it. This is quite a bold assumption on MSW's side.

From what I understand about the issue, this is a consumer problem. What are your thoughts on this?

[Jungzl]

It's true that the user can handle the encoding of the cookie, but in my case, another local development project (a nuxt project, which uses useCookie to handle the encoding automatically) added a non-standard cookie value to the cookie (Domain and Path were not set), and then shared the cookie in the current project, thus causing the issue mentioned in the issue. I think it would be a much better DX if cookie encoding could be handled automatically so I don't have to clear it every time.

[kettanaito]

but in my case, another local development project (a nuxt project, which uses useCookie to handle the encoding automatically) added a non-standard cookie value to the cookie

Doesn't this indicate an issue in that project you depend on? I wonder how the useCookie can even add a non-standard cookie name/value if JavaScript throws on that? I would like to learn more about your use case.

It may feel like a better DX to add the encoding/decoding to MSW for you, but there are a lot of other use cases to consider. I still think cookie encoding is a too strong assumption on MSW's side. Perhaps once I learn more about your use case, it'd be able to convince me.

[kettanaito]

So, based on the useCookie implementation on your end, looks like the code encodes the cookie value and proceeds with it. Then, once MSW stumbles upon that value, it will forward it to the request as-is, so no encoding issues would occur. Can you please write full reproduction steps for this? It'd help a lot. Thanks.

[Jungzl]

I created a minimal repo, which is project B.

So just pnpm dev and open the page, you'll see a successful request, then refresh the page, and the error will occur.

Basically, project A set some encoded cookies (the same effect in project B like below),

'Set-Cookie': `name=${encodeURIComponent('测试内容')}`

and in project B, MSW gets all cookies but not encode back, which occurs the error.

[Fallsleep]

values in document.cookie are usually urlencoded by other lib or backend api, cookieUtils.parse(document.cookie) in getAllDocumentCookies decode them, cookieUtils.parse use decodeURIComponent to decode cookie value when it has %.

If passed an object with decode function property, cookieUtils.parse will use this function instead of its own decode:

function decode(str) {
    return str.indexOf("%") !== -1 ? decodeURIComponent(str) : str;
}

So we can pass a function return origin value in getAllDocumentCookies:

function getAllDocumentCookies() {
  // keep origin value
  return cookieUtils.parse(document.cookie, { decode: (str: string) => str})
}

Then value in request.headers.append('cookie', ${name}=${value}) will keep the urlencoded value. For getting the decode in response resolver, just decodeURIComponent the value after request.headers.append:

request.headers.append('cookie', `${name}=${value}`)
if(value.indexOf("%") !== -1){
    forwardedCookies[name] = decodeURIComponent(value)
}

but it only avoid cookiesFromDocument, I don't know if value from cookiesFromStore is ok, maybe cookieUtils.serialize before append and decodeURIComponent after is better:

request.headers.append('cookie', cookieUtils.serialize(name, value))
if(value.indexOf("%") !== -1){
    forwardedCookies[name] = decodeURIComponent(value)
}

[Jungzl]

@Fallsleep Thanks for your suggestions.

[Fallsleep]

@Jungzl , your code request.headers.append('cookie', cookieUtils.serialize(name, value)) is done the work, { decode: (str: string) => str} and decodeURIComponent is not necessary , my mistake, I run into a wrong way to thought

[kettanaito]

I don't think particular substring detection in the cookie value is a reliable strategy here.

• Can we use decode option (or similar) in the cookieUtils.serialize() instead? Why not?

[Jungzl]

In fact, cookieUtils.serialize() use this detection (cookie/index-esm.js) internally. But I'll revert my last commit cause my previous commits already fulfilled my needs.

[kettanaito]

I didn't know that! Thanks for reverting to the previous state, it's much more straightforward now. Giving this a look...

[kettanaito]

This looks great! Thank you so much for tackling this issue, @Jungzl! Thanks @Fallsleep for discussion and help as well. Waiting for the CI and including this in the next release.

[kettanaito]

Released: v2.0.7 🎉

This has been released in v2.0.7!

• 📄 Release notes
• 📦 npm package

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.