Skip to content
This repository was archived by the owner on Feb 10, 2024. It is now read-only.
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 33 additions & 7 deletions src/parse-cookies.ts
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(';')

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?


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

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


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

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

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?

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

Expand Down
1 change: 1 addition & 0 deletions test/parse-cookies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const tests1: Test[] = group("parse-cookies > ", [
assert.deepEqual(parseCookies("a=1;b=2"), { a: "1", b: "2" });
assert.deepEqual(parseCookies("a=1;b=2;c=3"), { a: "1", b: "2", c: "3" });
assert.deepEqual(parseCookies("%3D=%3D"), { "=": "=" });
assert.deepEqual(parseCookies("a=b=c;"), { a: "b=c"});
}),
]);

Expand Down