mirror of
https://github.com/better-auth/better-auth.git
synced 2026-05-22 22:32:01 -05:00
fix(cookies): relax Cookie separator and centralize parsing (#9543)
Co-authored-by: sbougerel <5677149+sbougerel@users.noreply.github.com>
This commit is contained in:
5
.changeset/fix-cookie-header-no-space-separator.md
Normal file
5
.changeset/fix-cookie-header-no-space-separator.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
"better-auth": patch
|
||||
---
|
||||
|
||||
`Cookie` headers without a space after `;` separators are now tolerated. Signed-in users behind proxies that strip this space were previously treated as logged-out.
|
||||
@@ -173,6 +173,69 @@ export function toCookieOptions(
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Cookie-name token char set per RFC 7230 §3.2.6.
|
||||
*
|
||||
* @see https://datatracker.ietf.org/doc/html/rfc7230#section-3.2.6
|
||||
*/
|
||||
const cookieNameRegex = /^[\w!#$%&'*.^`|~+-]+$/;
|
||||
|
||||
/**
|
||||
* Cookie-value char set per RFC 6265 §4.1.1, plus space and comma.
|
||||
*
|
||||
* @see https://datatracker.ietf.org/doc/html/rfc6265#section-4.1.1
|
||||
* @see https://github.com/golang/go/issues/7243
|
||||
*/
|
||||
const cookieValueRegex = /^[ !#-:<-[\]-~]*$/;
|
||||
|
||||
/**
|
||||
* Trim leading/trailing OWS (space / horizontal tab) per RFC 7230 §3.2.3.
|
||||
* Narrower than `String.prototype.trim()`, which strips CR/LF and other
|
||||
* whitespace and would let CTLs escape `cookieValueRegex`.
|
||||
*
|
||||
* @see https://datatracker.ietf.org/doc/html/rfc7230#section-3.2.3
|
||||
*/
|
||||
function trimOWS(s: string): string {
|
||||
let start = 0;
|
||||
let end = s.length;
|
||||
while (start < end) {
|
||||
const c = s.charCodeAt(start);
|
||||
if (c !== 0x20 && c !== 0x09) break;
|
||||
start++;
|
||||
}
|
||||
while (end > start) {
|
||||
const c = s.charCodeAt(end - 1);
|
||||
if (c !== 0x20 && c !== 0x09) break;
|
||||
end--;
|
||||
}
|
||||
return start === 0 && end === s.length ? s : s.slice(start, end);
|
||||
}
|
||||
|
||||
/**
|
||||
* Tolerates `;` separators without the SP that RFC 6265 §4.2.1 mandates,
|
||||
* since proxies and runtimes commonly strip it. Silently drops entries
|
||||
* whose name violates RFC 7230 token or whose value violates RFC 6265
|
||||
* cookie-octet (plus space and comma). Strips optional surrounding
|
||||
* double-quotes per RFC 6265 §4.1.1.
|
||||
*/
|
||||
export function parseCookies(cookie: string): Map<string, string> {
|
||||
const cookieMap = new Map<string, string>();
|
||||
if (cookie.length < 2) return cookieMap;
|
||||
for (const chunk of cookie.split(";")) {
|
||||
const eq = chunk.indexOf("=");
|
||||
if (eq === -1) continue;
|
||||
const key = trimOWS(chunk.slice(0, eq));
|
||||
let val = trimOWS(chunk.slice(eq + 1));
|
||||
if (val.length >= 2 && val[0] === '"' && val[val.length - 1] === '"') {
|
||||
val = val.slice(1, -1);
|
||||
}
|
||||
if (cookieNameRegex.test(key) && cookieValueRegex.test(val)) {
|
||||
cookieMap.set(key, val);
|
||||
}
|
||||
}
|
||||
return cookieMap;
|
||||
}
|
||||
|
||||
/**
|
||||
* Add or replace a cookie in the request `Cookie` header.
|
||||
*
|
||||
@@ -186,46 +249,42 @@ export function setRequestCookie(
|
||||
name: string,
|
||||
value: string,
|
||||
): void {
|
||||
const cookieMap = new Map<string, string>();
|
||||
for (const pair of (headers.get("cookie") || "").split(";")) {
|
||||
const trimmed = pair.trim();
|
||||
const eq = trimmed.indexOf("=");
|
||||
if (eq > 0) cookieMap.set(trimmed.slice(0, eq), trimmed.slice(eq + 1));
|
||||
}
|
||||
|
||||
const cookieMap = parseCookies(headers.get("cookie") || "");
|
||||
cookieMap.set(name, value);
|
||||
|
||||
headers.set(
|
||||
"cookie",
|
||||
Array.from(cookieMap, ([k, v]) => `${k}=${v}`).join("; "),
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Merge `Set-Cookie` header values into the target's `Cookie` header.
|
||||
* Mutates `target`.
|
||||
*
|
||||
* Name/value-level merge only. RFC 6265 §5 user-agent semantics
|
||||
* (expiration, domain/path scoping, ordering) are out of scope. Suitable
|
||||
* for single-request proxy, middleware, and test contexts.
|
||||
*/
|
||||
export function applySetCookies(
|
||||
target: Headers,
|
||||
setCookieValues: Iterable<string>,
|
||||
): void {
|
||||
const cookieMap = parseCookies(target.get("cookie") || "");
|
||||
for (const setCookie of setCookieValues) {
|
||||
for (const [name, attr] of parseSetCookieHeader(setCookie)) {
|
||||
cookieMap.set(name, attr.value);
|
||||
}
|
||||
}
|
||||
target.set(
|
||||
"cookie",
|
||||
Array.from(cookieMap, ([k, v]) => `${k}=${v}`).join("; "),
|
||||
);
|
||||
}
|
||||
|
||||
export function setCookieToHeader(headers: Headers) {
|
||||
return (context: { response: Response }) => {
|
||||
const setCookieHeader = context.response.headers.get("set-cookie");
|
||||
if (!setCookieHeader) {
|
||||
return;
|
||||
}
|
||||
|
||||
const cookieMap = new Map<string, string>();
|
||||
|
||||
const existingCookiesHeader = headers.get("cookie") || "";
|
||||
existingCookiesHeader.split(";").forEach((cookie) => {
|
||||
const [name, ...rest] = cookie!.trim().split("=");
|
||||
if (name && rest.length > 0) {
|
||||
cookieMap.set(name, rest.join("="));
|
||||
}
|
||||
});
|
||||
|
||||
const cookies = parseSetCookieHeader(setCookieHeader);
|
||||
cookies.forEach((value, name) => {
|
||||
cookieMap.set(name, value.value);
|
||||
});
|
||||
|
||||
const updatedCookies = Array.from(cookieMap.entries())
|
||||
.map(([name, value]) => `${name}=${value}`)
|
||||
.join("; ");
|
||||
headers.set("cookie", updatedCookies);
|
||||
if (!setCookieHeader) return;
|
||||
applySetCookies(headers, [setCookieHeader]);
|
||||
};
|
||||
}
|
||||
|
||||
@@ -2,6 +2,7 @@ import type { BetterAuthOptions } from "@better-auth/core";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
expireCookie,
|
||||
getChunkedCookie,
|
||||
getCookieCache,
|
||||
getCookies,
|
||||
getSessionCookie,
|
||||
@@ -9,6 +10,7 @@ import {
|
||||
} from "../cookies";
|
||||
import { getTestInstance } from "../test-utils/test-instance";
|
||||
import {
|
||||
applySetCookies,
|
||||
HOST_COOKIE_PREFIX,
|
||||
parseSetCookieHeader,
|
||||
SECURE_COOKIE_PREFIX,
|
||||
@@ -1439,6 +1441,141 @@ describe("parse cookies", () => {
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* @see https://github.com/better-auth/better-auth/issues/9465
|
||||
*/
|
||||
describe("Cookie header without whitespace after semicolon", () => {
|
||||
it("parseCookies returns each pair when separator is `;` only", () => {
|
||||
const cookieHeader =
|
||||
"better-auth.session_token=session-token.signature;better-auth.session_data=session-data.signature";
|
||||
|
||||
const parsed = parseCookies(cookieHeader);
|
||||
|
||||
expect(parsed.get("better-auth.session_token")).toBe(
|
||||
"session-token.signature",
|
||||
);
|
||||
expect(parsed.get("better-auth.session_data")).toBe(
|
||||
"session-data.signature",
|
||||
);
|
||||
});
|
||||
|
||||
it("parseCookies tolerates mixed `;`, `; `, and `;\\t` separators", () => {
|
||||
const cookieHeader = "a=1; b=2;c=3;\td=4";
|
||||
|
||||
const parsed = parseCookies(cookieHeader);
|
||||
|
||||
expect(parsed.get("a")).toBe("1");
|
||||
expect(parsed.get("b")).toBe("2");
|
||||
expect(parsed.get("c")).toBe("3");
|
||||
expect(parsed.get("d")).toBe("4");
|
||||
});
|
||||
|
||||
it("getSessionCookie finds the session cookie when separator is `;` only", () => {
|
||||
const headers = new Headers();
|
||||
headers.set(
|
||||
"cookie",
|
||||
"preference=dark;better-auth.session_token=token-123",
|
||||
);
|
||||
const request = new Request("https://example.com/api/auth/session", {
|
||||
headers,
|
||||
});
|
||||
|
||||
expect(getSessionCookie(request)).toBe("token-123");
|
||||
});
|
||||
|
||||
it("getChunkedCookie reconstructs chunks across `;`-only separators", () => {
|
||||
const headers = new Headers();
|
||||
headers.set(
|
||||
"cookie",
|
||||
"better-auth.session_data.0=chunkA;better-auth.session_data.1=chunkB",
|
||||
);
|
||||
const ctx = {
|
||||
getCookie: () => undefined,
|
||||
headers,
|
||||
} as unknown as Parameters<typeof getChunkedCookie>[0];
|
||||
|
||||
expect(getChunkedCookie(ctx, "better-auth.session_data")).toBe(
|
||||
"chunkAchunkB",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("parseCookies validation", () => {
|
||||
it("returns empty map for empty header", () => {
|
||||
expect(parseCookies("").size).toBe(0);
|
||||
});
|
||||
|
||||
it("rejects names containing characters outside RFC 7230 token", () => {
|
||||
const map = parseCookies("bad name=v1; ok=v2; bad,name=v3; bad:name=v4");
|
||||
expect(map.has("bad name")).toBe(false);
|
||||
expect(map.has("bad,name")).toBe(false);
|
||||
expect(map.has("bad:name")).toBe(false);
|
||||
expect(map.get("ok")).toBe("v2");
|
||||
});
|
||||
|
||||
it("rejects values containing control chars, double-quote, or backslash", () => {
|
||||
const map = parseCookies('a=ok; b=has\rcr; c=has"quote; d=has\\slash');
|
||||
expect(map.get("a")).toBe("ok");
|
||||
expect(map.has("b")).toBe(false);
|
||||
expect(map.has("c")).toBe(false);
|
||||
expect(map.has("d")).toBe(false);
|
||||
});
|
||||
|
||||
it("accepts values with space and comma (real-world deviation)", () => {
|
||||
const map = parseCookies("a=hello world; b=v1,v2");
|
||||
expect(map.get("a")).toBe("hello world");
|
||||
expect(map.get("b")).toBe("v1,v2");
|
||||
});
|
||||
|
||||
it("splits on first `=` only, preserving subsequent `=` in value", () => {
|
||||
const map = parseCookies("a=b=c=d");
|
||||
expect(map.get("a")).toBe("b=c=d");
|
||||
});
|
||||
|
||||
it("rejects entries with CR/LF in raw key or value (no trim escape)", () => {
|
||||
const map = parseCookies("a=ok\r; b\r=1; c=v\nv; d=ok");
|
||||
expect(map.has("a")).toBe(false);
|
||||
expect(map.has("b")).toBe(false);
|
||||
expect(map.has("c")).toBe(false);
|
||||
expect(map.get("d")).toBe("ok");
|
||||
});
|
||||
|
||||
it("strips double-quoted values per RFC 6265 §4.1.1", () => {
|
||||
const map = parseCookies('a="hello"; b=plain; c="with space"');
|
||||
expect(map.get("a")).toBe("hello");
|
||||
expect(map.get("b")).toBe("plain");
|
||||
expect(map.get("c")).toBe("with space");
|
||||
});
|
||||
});
|
||||
|
||||
describe("applySetCookies", () => {
|
||||
it("merges into empty Cookie header", () => {
|
||||
const headers = new Headers();
|
||||
applySetCookies(headers, ["a=1; Path=/"]);
|
||||
expect(headers.get("cookie")).toBe("a=1");
|
||||
});
|
||||
|
||||
it("strips Set-Cookie attributes (only name=value lands)", () => {
|
||||
const headers = new Headers();
|
||||
applySetCookies(headers, [
|
||||
"a=1; Path=/; HttpOnly; Secure; Max-Age=3600; SameSite=Lax",
|
||||
]);
|
||||
expect(headers.get("cookie")).toBe("a=1");
|
||||
});
|
||||
|
||||
it("merges multiple Set-Cookie values", () => {
|
||||
const headers = new Headers();
|
||||
applySetCookies(headers, ["a=1; Path=/", "b=2; Path=/"]);
|
||||
expect(headers.get("cookie")).toBe("a=1; b=2");
|
||||
});
|
||||
|
||||
it("last-wins on duplicate cookie name (existing + new)", () => {
|
||||
const headers = new Headers({ cookie: "a=old; b=keep" });
|
||||
applySetCookies(headers, ["a=new; Path=/"]);
|
||||
expect(headers.get("cookie")).toBe("a=new; b=keep");
|
||||
});
|
||||
});
|
||||
|
||||
describe("expireCookie", () => {
|
||||
it("preserves attributes", () => {
|
||||
const setCookie = vi.fn();
|
||||
|
||||
@@ -24,7 +24,7 @@ import { getDate } from "../utils/date";
|
||||
import { isPromise } from "../utils/is-promise";
|
||||
import { sec } from "../utils/time";
|
||||
import { isDynamicBaseURLConfig } from "../utils/url";
|
||||
import { SECURE_COOKIE_PREFIX } from "./cookie-utils";
|
||||
import { parseCookies, SECURE_COOKIE_PREFIX } from "./cookie-utils";
|
||||
import {
|
||||
createAccountStore,
|
||||
createSessionStore,
|
||||
@@ -356,17 +356,6 @@ export function deleteSessionCookie(
|
||||
}
|
||||
}
|
||||
|
||||
export function parseCookies(cookieHeader: string) {
|
||||
const cookies = cookieHeader.split("; ");
|
||||
const cookieMap = new Map<string, string>();
|
||||
|
||||
cookies.forEach((cookie) => {
|
||||
const [name, value] = cookie.split(/=(.*)/s);
|
||||
cookieMap.set(name!, value!);
|
||||
});
|
||||
return cookieMap;
|
||||
}
|
||||
|
||||
export type EligibleCookies = (string & {}) | (keyof BetterAuthCookies & {});
|
||||
|
||||
export const getSessionCookie = (
|
||||
|
||||
@@ -5,6 +5,7 @@ import { safeJSONParse } from "@better-auth/core/utils/json";
|
||||
import type { CookieOptions } from "better-call";
|
||||
import * as z from "zod";
|
||||
import { symmetricDecodeJWT, symmetricEncodeJWT } from "../crypto";
|
||||
import { parseCookies } from "./cookie-utils";
|
||||
|
||||
// Cookie size constants based on browser limits
|
||||
const ALLOWED_COOKIE_SIZE = 4096;
|
||||
@@ -21,30 +22,6 @@ interface Cookie {
|
||||
|
||||
type Chunks = Record<string, string>;
|
||||
|
||||
/**
|
||||
* Parse cookies from the request headers
|
||||
*/
|
||||
function parseCookiesFromContext(
|
||||
ctx: GenericEndpointContext,
|
||||
): Record<string, string> {
|
||||
const cookieHeader = ctx.headers?.get("cookie");
|
||||
if (!cookieHeader) {
|
||||
return {};
|
||||
}
|
||||
|
||||
const cookies: Record<string, string> = {};
|
||||
const pairs = cookieHeader.split("; ");
|
||||
|
||||
for (const pair of pairs) {
|
||||
const [name, ...valueParts] = pair.split("=");
|
||||
if (name && valueParts.length > 0) {
|
||||
cookies[name] = valueParts.join("=");
|
||||
}
|
||||
}
|
||||
|
||||
return cookies;
|
||||
}
|
||||
|
||||
/**
|
||||
* Extract the chunk index from a cookie name
|
||||
*/
|
||||
@@ -63,9 +40,9 @@ function readExistingChunks(
|
||||
ctx: GenericEndpointContext,
|
||||
): Chunks {
|
||||
const chunks: Chunks = {};
|
||||
const cookies = parseCookiesFromContext(ctx);
|
||||
const cookies = parseCookies(ctx.headers?.get("cookie") || "");
|
||||
|
||||
for (const [name, value] of Object.entries(cookies)) {
|
||||
for (const [name, value] of cookies) {
|
||||
if (name.startsWith(cookieName)) {
|
||||
chunks[name] = value;
|
||||
}
|
||||
@@ -247,16 +224,7 @@ export function getChunkedCookie(
|
||||
return null;
|
||||
}
|
||||
|
||||
const cookies: Record<string, string> = {};
|
||||
const pairs = cookieHeader.split("; ");
|
||||
for (const pair of pairs) {
|
||||
const [name, ...valueParts] = pair.split("=");
|
||||
if (name && valueParts.length > 0) {
|
||||
cookies[name] = valueParts.join("=");
|
||||
}
|
||||
}
|
||||
|
||||
for (const [name, val] of Object.entries(cookies)) {
|
||||
for (const [name, val] of parseCookies(cookieHeader)) {
|
||||
if (name.startsWith(cookieName + ".")) {
|
||||
const parts = name.split(".");
|
||||
const indexStr = parts.at(-1);
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import type { BetterAuthClientPlugin } from "@better-auth/core";
|
||||
import { parseCookies } from "../../cookies/cookie-utils";
|
||||
import { PACKAGE_VERSION } from "../../version";
|
||||
|
||||
/**
|
||||
@@ -16,12 +17,7 @@ function getCookieValue(name: string): string | null {
|
||||
if (typeof document === "undefined") {
|
||||
return null;
|
||||
}
|
||||
|
||||
const cookie = document.cookie
|
||||
.split("; ")
|
||||
.find((row) => row.startsWith(`${name}=`));
|
||||
|
||||
return cookie ? cookie.split("=")[1]! : null;
|
||||
return parseCookies(document.cookie).get(name) ?? null;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -2,7 +2,7 @@ import { APIError, BASE_ERROR_CODES } from "@better-auth/core/error";
|
||||
import { createOTP } from "@better-auth/utils/otp";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { createAuthClient } from "../../client";
|
||||
import { parseSetCookieHeader } from "../../cookies";
|
||||
import { applySetCookies, parseSetCookieHeader } from "../../cookies";
|
||||
import { symmetricDecrypt } from "../../crypto";
|
||||
import { convertSetCookieToCookie } from "../../test-utils/headers";
|
||||
import { getTestInstance } from "../../test-utils/test-instance";
|
||||
@@ -1925,30 +1925,8 @@ describe("two factor passwordless", async () => {
|
||||
|
||||
const applySetCookie = (headers: Headers, responseHeaders: Headers) => {
|
||||
const setCookieHeader = responseHeaders.get("set-cookie");
|
||||
if (!setCookieHeader) {
|
||||
return headers;
|
||||
}
|
||||
const existing = headers.get("cookie");
|
||||
const cookieMap = new Map<string, string>();
|
||||
if (existing) {
|
||||
existing.split("; ").forEach((pair) => {
|
||||
const [name, ...rest] = pair.split("=");
|
||||
if (!name) {
|
||||
return;
|
||||
}
|
||||
cookieMap.set(name, rest.join("="));
|
||||
});
|
||||
}
|
||||
const cookies = parseSetCookieHeader(setCookieHeader);
|
||||
cookies.forEach((cookie, name) => {
|
||||
cookieMap.set(name, cookie.value);
|
||||
});
|
||||
headers.set(
|
||||
"cookie",
|
||||
Array.from(cookieMap.entries())
|
||||
.map(([name, value]) => `${name}=${value}`)
|
||||
.join("; "),
|
||||
);
|
||||
if (!setCookieHeader) return headers;
|
||||
applySetCookies(headers, [setCookieHeader]);
|
||||
return headers;
|
||||
};
|
||||
|
||||
|
||||
@@ -1,3 +1,5 @@
|
||||
import { applySetCookies } from "../cookies/cookie-utils";
|
||||
|
||||
/**
|
||||
* converts set cookie containing headers to
|
||||
* cookie containing headers
|
||||
@@ -10,19 +12,8 @@ export function convertSetCookieToCookie(headers: Headers): Headers {
|
||||
}
|
||||
});
|
||||
|
||||
if (setCookieHeaders.length === 0) {
|
||||
return headers;
|
||||
}
|
||||
|
||||
const existingCookies = headers.get("cookie") || "";
|
||||
const cookies = existingCookies ? existingCookies.split("; ") : [];
|
||||
|
||||
setCookieHeaders.forEach((setCookie) => {
|
||||
const cookiePair = setCookie.split(";")[0]!;
|
||||
cookies.push(cookiePair.trim());
|
||||
});
|
||||
|
||||
headers.set("cookie", cookies.join("; "));
|
||||
if (setCookieHeaders.length === 0) return headers;
|
||||
|
||||
applySetCookies(headers, setCookieHeaders);
|
||||
return headers;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user