-
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?
Conversation
}); | ||
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 comment
The 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?
s.split(';').forEach((pair) => {
const index = pair.indexOf('='):
// rest of the logic
});
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.
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 value = decodeURIComponent(encodedValue); | ||
parsed[key] = value; | ||
}); | ||
const pairs = s.split(';') |
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?
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 comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
for (let i = 0; i < pairs.length; i++) { | ||
const pair = pairs[i]; | ||
const index = pair.indexOf('=') |
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.
this logic could be simplified using split with a limit
const [key, val] = pair.split('=', 2);
if (val === undefined) {
continue;
}
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 comment
The 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?
fix issue: #338