Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: remove cookies that expire at epoch time of 0 #457

Merged
merged 1 commit into from
Sep 19, 2024
Merged

Conversation

colincasey
Copy link
Contributor

The logic for checking and removing expired cookies fails when the cookie has an expiry date set to the epoch time of 0 (i.e.; Thu, 01 Jan 1970 00:00:00 GMT). This is because the branch check that triggers cookie removal was testing expiryTime which is falsy for a value of 0 and that short-circuits the logic.

This PR updates the logic to narrow the type of expiryTime from number | undefined to number using expiryTime != undefined which allows the 0 value to proceed to the subsequent condition and trigger the cookie removal.

Fixes #455

The logic for checking and removing expired cookies fails when the cookie has an expiry date set to the epoch time of `0` (i.e.; `Thu, 01 Jan 1970 00:00:00 GMT`). This is because the branch check that triggers cookie removal was testing `expiryTime` which is `falsy` for a value of `0` and that short-circuits the logic.

This PR updates the logic to narrow the type of `expiryTime` from `number | undefined` to `number` using `expiryTime != undefined` which allows the `0` value to proceed to the subsequent condition and trigger the cookie removal.

Fixes #455
@colincasey colincasey self-assigned this Sep 16, 2024
@colincasey colincasey merged commit 039663c into master Sep 19, 2024
10 checks passed
@colincasey colincasey deleted the fix_issue_455 branch September 19, 2024 13:45
@EZaykov
Copy link

EZaykov commented Dec 21, 2024

Iam not sure but it seems like this fix did not showed up (at least in compiled js). New to github sry xD

In node_modules its still if (expireCheck && expiryTime && expiryTime <= now).

Made small patch in my store for now:

function patchExpireTime(cookie: Cookie): Cookie {
	const original = cookie.expiryTime;

	cookie.expiryTime = (now?: Date) => {
		const result = original.call(cookie, now);

		if (result === 0) {
			return 1;
		}

		return result;
	};

	return cookie;
}

Waiting for update

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

Successfully merging this pull request may close these issues.

Cookies with expiry time set to epoch zero are matched and not expired
3 participants