-
Notifications
You must be signed in to change notification settings - Fork 14
fix: parseCookies error when cookie value has '=' #339
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,39 @@ | ||
const parseCookies = (s: string): { [key: string]: string } => { | ||
if (s.length === 0) return {}; | ||
const parsed: { [key: string]: string } = {}; | ||
const pattern = new RegExp("\\s*;\\s*"); | ||
s.split(pattern).forEach((i) => { | ||
const [encodedKey, encodedValue] = i.split("="); | ||
const key = decodeURIComponent(encodedKey); | ||
const value = decodeURIComponent(encodedValue); | ||
parsed[key] = value; | ||
}); | ||
const pairs = s.split(';') | ||
|
||
for (let i = 0; i < pairs.length; i++) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you are only using i to get the pair. Why not use forEach here?
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In fact, I was copying the code from here and thought it would solve my problem and also pass the all tests: https://github.com/jshttp/cookie/blob/04be428b438605b48ad6af503227b817c07b9b52/index.js#L48-L82 |
||
const pair = pairs[i]; | ||
const index = pair.indexOf('=') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this logic could be simplified using split with a limit
|
||
|
||
// skip things that don't look like key=value | ||
if (index < 0) { | ||
continue; | ||
} | ||
|
||
const key = pair.substring(0, index).trim() | ||
|
||
// only assign once | ||
if (undefined == parsed[key]) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
let val = pair.substring(index + 1, pair.length).trim() | ||
|
||
// quoted values | ||
if (val[0] === '"') { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. doesn't look like the previous implementation does this. What use case is this for? |
||
val = val.slice(1, -1) | ||
} | ||
|
||
// catch error: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/decodeURIComponent#catching_errors | ||
try { | ||
const _key = decodeURIComponent(key); | ||
const _val = decodeURIComponent(val); | ||
parsed[_key] = decodeURIComponent(_val); | ||
} catch (error) { | ||
parsed[key] = val | ||
} | ||
} | ||
} | ||
|
||
return parsed; | ||
}; | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't this just change the bug from breaking with '=' values to breaking with ';' values?