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

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

Open
yes1am wants to merge 1 commit into
bouzuya:masterfrom
yes1am:fix_parseCookies
Open

fix: parseCookies error when cookie value has '='#339
yes1am wants to merge 1 commit into
bouzuya:masterfrom
yes1am:fix_parseCookies

Conversation

@yes1am

@yes1am yes1am commented Feb 14, 2022

Copy link
Copy Markdown

fix issue: #338

Comment thread src/parse-cookies.ts
});
const pairs = s.split(';')

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

Copy link
Copy Markdown

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

@yes1am yes1am Jan 9, 2023

Copy link
Copy Markdown
Author

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

Comment thread src/parse-cookies.ts
const value = decodeURIComponent(encodedValue);
parsed[key] = value;
});
const pairs = s.split(';')

Copy link
Copy Markdown

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?

Comment thread src/parse-cookies.ts
const key = pair.substring(0, index).trim()

// only assign once
if (undefined == parsed[key]) {

Copy link
Copy Markdown

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

Comment thread src/parse-cookies.ts

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

@Akolyte01 Akolyte01 Jan 6, 2023

Copy link
Copy Markdown

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

Comment thread src/parse-cookies.ts
let val = pair.substring(index + 1, pair.length).trim()

// quoted values
if (val[0] === '"') {

Copy link
Copy Markdown

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