Skip to content
This repository has been archived by the owner on Feb 10, 2024. It is now read-only.

fix: parseCookies error when cookie value has '=' #339

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

yes1am
Copy link

@yes1am yes1am commented Feb 14, 2022

fix issue: #338

});
const pairs = s.split(';')

for (let i = 0; i < pairs.length; i++) {

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
});

Copy link
Author

@yes1am yes1am Jan 9, 2023

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(';')

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]) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious why you chose to have undefined to be on the left hand side of this statement. Also are you sure == is the correct operator?

image

You could use === instead. This would ensure that your parsed key is actually undefined


for (let i = 0; i < pairs.length; i++) {
const pair = pairs[i];
const index = pair.indexOf('=')
Copy link

@Akolyte01 Akolyte01 Jan 6, 2023

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] === '"') {

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?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants