fix(cookies): preserve values and parse SameSite strictly · nodejs/undici@d0574cc · GitHub
Skip to content

Commit d0574cc

Browse files
committed
fix(cookies): preserve values and parse SameSite strictly
Signed-off-by: Matteo Collina <hello@matteocollina.com> (cherry picked from commit 5655ea4)
1 parent 0fa8086 commit d0574cc

4 files changed

Lines changed: 85 additions & 27 deletions

File tree

docs/docs/api/Cookies.md

Lines changed: 27 additions & 0 deletions

lib/web/cookies/parse.js

Lines changed: 17 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ const { collectASequenceOfCodePointsFast } = require('../infra')
44
const { maxNameValuePairSize, maxAttributeValueSize } = require('./constants')
55
const { isCTLExcludingHtab } = require('./util')
66
const assert = require('node:assert')
7-
const { unescape: qsUnescape } = require('node:querystring')
87

98
/**
109
* @description Parses the field-value attributes of a set-cookie header string.
@@ -82,7 +81,7 @@ function parseSetCookie (header) {
8281
// store arbitrary data in a cookie-value SHOULD encode that data, for
8382
// example, using Base64 [RFC4648].
8483
return {
85-
name, value: qsUnescape(value), ...parseUnparsedAttributes(unparsedAttributes)
84+
name, value, ...parseUnparsedAttributes(unparsedAttributes)
8685
}
8786
}
8887

@@ -280,32 +279,25 @@ function parseUnparsedAttributes (unparsedAttributes, cookieAttributeList = {})
280279
// If the attribute-name case-insensitively matches the string
281280
// "SameSite", the user agent MUST process the cookie-av as follows:
282281

283-
// 1. Let enforcement be "Default".
284-
let enforcement = 'Default'
285-
286282
const attributeValueLowercase = attributeValue.toLowerCase()
287-
// 2. If cookie-av's attribute-value is a case-insensitive match for
288-
// "None", set enforcement to "None".
289-
if (attributeValueLowercase.includes('none')) {
290-
enforcement = 'None'
291-
}
292283

293-
// 3. If cookie-av's attribute-value is a case-insensitive match for
294-
// "Strict", set enforcement to "Strict".
295-
if (attributeValueLowercase.includes('strict')) {
296-
enforcement = 'Strict'
284+
// 1. If cookie-av's attribute-value is a case-insensitive match for
285+
// "None", append an attribute to the cookie-attribute-list with an
286+
// attribute-name of "SameSite" and an attribute-value of "None".
287+
if (attributeValueLowercase === 'none') {
288+
cookieAttributeList.sameSite = 'None'
289+
} else if (attributeValueLowercase === 'strict') {
290+
// 2. If cookie-av's attribute-value is a case-insensitive match for
291+
// "Strict", append an attribute to the cookie-attribute-list with
292+
// an attribute-name of "SameSite" and an attribute-value of
293+
// "Strict".
294+
cookieAttributeList.sameSite = 'Strict'
295+
} else if (attributeValueLowercase === 'lax') {
296+
// 3. If cookie-av's attribute-value is a case-insensitive match for
297+
// "Lax", append an attribute to the cookie-attribute-list with an
298+
// attribute-name of "SameSite" and an attribute-value of "Lax".
299+
cookieAttributeList.sameSite = 'Lax'
297300
}
298-
299-
// 4. If cookie-av's attribute-value is a case-insensitive match for
300-
// "Lax", set enforcement to "Lax".
301-
if (attributeValueLowercase.includes('lax')) {
302-
enforcement = 'Lax'
303-
}
304-
305-
// 5. Append an attribute to the cookie-attribute-list with an
306-
// attribute-name of "SameSite" and an attribute-value of
307-
// enforcement.
308-
cookieAttributeList.sameSite = enforcement
309301
} else {
310302
cookieAttributeList.unparsed ??= []
311303

test/cookie/cookies.js

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ const {
2828
deleteCookie,
2929
getCookies,
3030
getSetCookies,
31+
parseCookie,
3132
setCookie,
3233
Headers
3334
} = require('../..')
@@ -600,6 +601,41 @@ test('Set-Cookie parser', () => {
600601
assert.deepEqual(getSetCookies(headers), [])
601602
})
602603

604+
test('Set-Cookie parser does not percent-decode cookie values', () => {
605+
assert.deepEqual(
606+
parseCookie(
607+
'token=legit%0d%0aSet-Cookie:%20evil=injected%3B%20Path%3D/'
608+
),
609+
{
610+
name: 'token',
611+
value: 'legit%0d%0aSet-Cookie:%20evil=injected%3B%20Path%3D/'
612+
}
613+
)
614+
615+
assert.deepEqual(parseCookie('data=prefix%00suffix'), {
616+
name: 'data',
617+
value: 'prefix%00suffix'
618+
})
619+
})
620+
621+
test('Set-Cookie parser only accepts exact SameSite values', () => {
622+
assert.deepEqual(parseCookie('a=b; SameSite=none'), {
623+
name: 'a',
624+
value: 'b',
625+
sameSite: 'None'
626+
})
627+
628+
assert.deepEqual(parseCookie('a=b; SameSite=StrictLax'), {
629+
name: 'a',
630+
value: 'b'
631+
})
632+
633+
assert.deepEqual(parseCookie('a=b; SameSite=NoneOfYourBusiness'), {
634+
name: 'a',
635+
value: 'b'
636+
})
637+
})
638+
603639
test('Cookie setCookie throws if headers is not of type Headers', () => {
604640
class Headers {
605641
[Symbol.toStringTag] = 'CustomHeaders'

test/cookie/npm-cookie.js

Lines changed: 5 additions & 2 deletions

0 commit comments

Comments
 (0)