Repository: capricorn86/happy-dom
Pull request #851 (2023-04-11 22:44:19)

Task/836 wrong patch 677 cookie


Patch:
diff --git a/packages/happy-dom/src/cookie/Cookie.ts b/packages/happy-dom/src/cookie/Cookie.ts
index 783810f6..195b54a4 100644
--- a/packages/happy-dom/src/cookie/Cookie.ts
+++ b/packages/happy-dom/src/cookie/Cookie.ts
@@ -1,61 +1,54 @@
-const CookiePairRegex = /([^=]+)(?:=([\s\S]*))?/;
+import DOMException from '../exception/DOMException';
+import DOMExceptionNameEnum from '../exception/DOMExceptionNameEnum';
+import CookieSameSiteEnum from './CookieSameSiteEnum';
+import { URL } from 'url';
 
 /**
  * Cookie.
  */
 export default class Cookie {
-	private pairs: { [key: string]: string } = {};
-	//
+	// Required
 	public key = '';
-	public value = '';
-	public size = 0;
+	public value: string | null = null;
+	public originURL: URL;
+
 	// Optional
 	public domain = '';
 	public path = '';
-	public expriesOrMaxAge: Date = null;
+	public expires: Date | null = null;
 	public httpOnly = false;
 	public secure = false;
-	public sameSite = '';
+	public sameSite: CookieSameSiteEnum = CookieSameSiteEnum.lax;
 
 	/**
 	 * Constructor.
 	 *
+	 * @param originURL Origin URL.
 	 * @param cookie Cookie.
 	 */
-	constructor(cookie: string) {
-		let match: RegExpExecArray | null;
+	constructor(originURL, cookie: string) {
+		const parts = cookie.split(';');
+		const [key, value] = parts.shift().split('=');
 
-		const parts = cookie.split(';').filter(Boolean);
+		this.originURL = originURL;
+		this.key = key.trim();
+		this.value = value !== undefined ? value : null;
 
-		// Part[0] is the key-value pair.
-		match = new RegExp(CookiePairRegex).exec(parts[0]);
-		if (!match) {
-			throw new Error(`Invalid cookie: ${cookie}`);
-		}
-		this.key = match[1].trim();
-		this.value = match[2];
-		// Set key is empty if match[2] is undefined.
-		if (!match[2] && parts[0][this.key.length] !== '=') {
-			this.value = this.key;
-			this.key = '';
+		if (!this.key) {
+			throw new DOMException(`Invalid cookie: ${cookie}.`, DOMExceptionNameEnum.syntaxError);
 		}
-		this.pairs[this.key] = this.value;
-		this.size = this.key.length + this.value.length;
-		// Attribute.
-		for (const part of parts.slice(1)) {
-			match = new RegExp(CookiePairRegex).exec(part);
-			if (!match) {
-				throw new Error(`Invalid cookie: ${part}`);
-			}
-			const key = match[1].trim();
-			const value = match[2];
 
-			switch (key.toLowerCase()) {
+		for (const part of parts) {
+			const keyAndValue = part.split('=');
+			const key = keyAndValue[0].trim().toLowerCase();
+			const value = keyAndValue[1];
+
+			switch (key) {
 				case 'expires':
-					this.expriesOrMaxAge = new Date(value);
+					this.expires = new Date(value);
 					break;
 				case 'max-age':
-					this.expriesOrMaxAge = new Date(parseInt(value, 10) * 1000 + Date.now());
+					this.expires = new Date(parseInt(value, 10) * 1000 + Date.now());
 					break;
 				case 'domain':
 					this.domain = value;
@@ -70,53 +63,42 @@ export default class Cookie {
 					this.secure = true;
 					break;
 				case 'samesite':
-					this.sameSite = value;
+					switch (value.toLowerCase()) {
+						case 'strict':
+							this.sameSite = CookieSameSiteEnum.strict;
+							break;
+						case 'lax':
+							this.sameSite = CookieSameSiteEnum.lax;
+							break;
+						case 'none':
+							this.sameSite = CookieSameSiteEnum.none;
+					}
 					break;
-				default:
-					continue; // Skip.
 			}
-			// Skip unknown key-value pair.
-			if (
-				['expires', 'max-age', 'domain', 'path', 'httponly', 'secure', 'samesite'].indexOf(
-					key.toLowerCase()
-				) === -1
-			) {
-				continue;
-			}
-			this.pairs[key] = value;
 		}
 	}
 
 	/**
-	 * Returns a raw string of the cookie.
-	 */
-	public rawString(): string {
-		return Object.keys(this.pairs)
-			.map((key) => {
-				if (key) {
-					return `${key}=${this.pairs[key]}`;
-				}
-				return this.pairs[key];
-			})
-			.join('; ');
-	}
-
-	/**
+	 * Returns cookie string.
 	 *
+	 * @returns Cookie string.
 	 */
-	public cookieString(): string {
-		if (this.key) {
+	public toString(): string {
+		if (this.value !== null) {
 			return `${this.key}=${this.value}`;
 		}
-		return this.value;
+
+		return this.key;
 	}
 
 	/**
+	 * Returns "true" if expired.
 	 *
+	 * @returns "true" if expired.
 	 */
 	public isExpired(): boolean {
 		// If the expries/maxage is set, then determine whether it is expired.
-		if (this.expriesOrMaxAge && this.expriesOrMaxAge.getTime() < Date.now()) {
+		if (this.expires && this.expires.getTime() < Date.now()) {
 			return true;
 		}
 		// If the expries/maxage is not set, it's a session-level cookie that will expire when the browser is closed.
@@ -125,34 +107,19 @@ export default class Cookie {
 	}
 
 	/**
+	 * Validate cookie.
 	 *
+	 * @see https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie#cookie_prefixes
+	 * @returns "true" if valid.
 	 */
-	public isHttpOnly(): boolean {
-		return this.httpOnly;
-	}
-
-	/**
-	 *
-	 */
-	public isSecure(): boolean {
-		return this.secure;
-	}
-
-	/**
-	 * Parse a cookie string.
-	 *
-	 * @param cookieString
-	 */
-	public static parse(cookieString: string): Cookie {
-		return new Cookie(cookieString);
-	}
-
-	/**
-	 * Stringify a Cookie object.
-	 *
-	 * @param cookie
-	 */
-	public static stringify(cookie: Cookie): string {
-		return cookie.toString();
+	public validate(): boolean {
+		const lowerKey = this.key.toLowerCase();
+		if (lowerKey.startsWith('__secure-') && !this.secure) {
+			return false;
+		}
+		if (lowerKey.startsWith('__host-') && (!this.secure || this.path !== '/' || this.domain)) {
+			return false;
+		}
+		return true;
 	}
 }
diff --git a/packages/happy-dom/src/cookie/CookieJar.ts b/packages/happy-dom/src/cookie/CookieJar.ts
index 06a55706..e8219d42 100644
--- a/packages/happy-dom/src/cookie/CookieJar.ts
+++ b/packages/happy-dom/src/cookie/CookieJar.ts
@@ -1,5 +1,6 @@
-import Location from 'src/location/Location';
 import Cookie from './Cookie';
+import CookieSameSiteEnum from './CookieSameSiteEnum';
+import { URL } from 'url';
 
 /**
  * CookieJar.
@@ -11,72 +12,70 @@ export default class CookieJar {
 	private cookies: Cookie[] = [];
 
 	/**
-	 * Validate cookie.
+	 * Adds cookie string.
 	 *
-	 * @param cookie
+	 * @param originURL Origin URL.
+	 * @param cookieString Cookie string.
 	 */
-	private validateCookie(cookie: Cookie): boolean {
-		if (cookie.key.toLowerCase().startsWith('__secure-') && !cookie.isSecure()) {
-			return false;
-		}
-		if (
-			cookie.key.toLowerCase().startsWith('__host-') &&
-			(!cookie.isSecure() || cookie.path !== '/' || cookie.domain)
-		) {
-			return false;
-		}
-		return true;
-	}
-
-	/**
-	 * Set cookie.
-	 *
-	 * @param cookieString
-	 */
-	public setCookiesString(cookieString: string): void {
+	public addCookieString(originURL: URL, cookieString: string): void {
 		if (!cookieString) {
 			return;
 		}
-		const newCookie = new Cookie(cookieString);
-		if (!this.validateCookie(newCookie)) {
+
+		const newCookie = new Cookie(originURL, cookieString);
+
+		if (!newCookie.validate()) {
 			return;
 		}
-		this.cookies
-			.filter((cookie) => cookie.key === newCookie.key)
-			.forEach((cookie) => {
-				this.cookies.splice(this.cookies.indexOf(cookie), 1);
-			});
-		this.cookies.push(newCookie);
+
+		for (let i = 0, max = this.cookies.length; i < max; i++) {
+			if (
+				this.cookies[i].key === newCookie.key &&
+				this.cookies[i].originURL.hostname === newCookie.originURL.hostname &&
+				// Cookies with or without values are treated differently in the browser.
+				// Therefore, the cookie should only be replaced if either both has a value or if both has no value.
+				// The cookie value is null if it has no value set.
+				// This is a bit unlogical, so it would be nice with a link to the spec here.
+				typeof this.cookies[i].value === typeof newCookie.value
+			) {
+				this.cookies.splice(i, 1);
+				break;
+			}
+		}
+
+		if (!newCookie.isExpired()) {
+			this.cookies.push(newCookie);
+		}
 	}
 
 	/**
-	 * Get cookie.
+	 * Get cookie string.
 	 *
-	 * @param location Location.
+	 * @param targetURL Target URL.
 	 * @param fromDocument If true, the caller is a document.
 	 * @returns Cookie string.
 	 */
-	public getCookiesString(location: Location, fromDocument: boolean): string {
-		const cookies = this.cookies.filter((cookie) => {
-			// Skip when use document.cookie and the cookie is httponly.
-			if (fromDocument && cookie.isHttpOnly()) {
-				return false;
-			}
-			if (cookie.isExpired()) {
-				return false;
-			}
-			if (cookie.isSecure() && location.protocol !== 'https:') {
-				return false;
-			}
-			if (cookie.domain && !location.hostname.endsWith(cookie.domain)) {
-				return false;
-			}
-			if (cookie.path && !location.pathname.startsWith(cookie.path)) {
-				return false;
+	public getCookieString(targetURL: URL, fromDocument: boolean): string {
+		let cookieString = '';
+
+		for (const cookie of this.cookies) {
+			if (
+				(!fromDocument || !cookie.httpOnly) &&
+				!cookie.isExpired() &&
+				(!cookie.secure || targetURL.protocol === 'https:') &&
+				(!cookie.domain || targetURL.hostname.endsWith(cookie.domain)) &&
+				(!cookie.path || targetURL.pathname.startsWith(cookie.path)) &&
+				// @see https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie#samesitesamesite-value
+				((cookie.sameSite === CookieSameSiteEnum.none && cookie.secure) ||
+					cookie.originURL.hostname === targetURL.hostname)
+			) {
+				if (cookieString) {
+					cookieString += '; ';
+				}
+				cookieString += cookie.toString();
 			}
-			// TODO: Check same site behaviour.
-			return true;
-		});
-		return cookies.map((cookie) => cookie.cookieString()).join('; ');
+		}
+
+		return cookieString;
 	}
 }
diff --git a/packages/happy-dom/src/cookie/CookieSameSiteEnum.ts b/packages/happy-dom/src/cookie/CookieSameSiteEnum.ts
new file mode 100644
index 00000000..7cfbb3b5
--- /dev/null
+++ b/packages/happy-dom/src/cookie/CookieSameSiteEnum.ts
@@ -0,0 +1,6 @@
+enum CookieSameSiteEnum {
+	strict = 'Strict',
+	lax = 'Lax',
+	none = 'None'
+}
+export default CookieSameSiteEnum;
diff --git a/packages/happy-dom/src/fetch/Fetch.ts b/packages/happy-dom/src/fetch/Fetch.ts
index f25cad72..250887ed 100644
--- a/packages/happy-dom/src/fetch/Fetch.ts
+++ b/packages/happy-dom/src/fetch/Fetch.ts
@@ -16,6 +16,7 @@ import { Socket } from 'net';
 import Stream from 'stream';
 import DataURIParser from './data-uri/DataURIParser';
 import FetchCORSUtility from './utilities/FetchCORSUtility';
+import CookieJar from '../cookie/CookieJar';
 
 const SUPPORTED_SCHEMAS = ['data:', 'http:', 'https:'];
 const REDIRECT_STATUS_CODES = [301, 302, 303, 307, 308];
@@ -557,7 +558,10 @@ export default class Fetch {
 			this.request.credentials === 'include' ||
 			(this.request.credentials === 'same-origin' && !isCORS)
 		) {
-			const cookie = document.defaultView.document.cookie;
+			const cookie = document.defaultView.document._cookie.getCookieString(
+				this.ownerDocument.defaultView.location,
+				false
+			);
 			if (cookie) {
 				headers.set('Cookie', cookie);
 			}
@@ -614,13 +618,7 @@ export default class Fetch {
 				// Handles setting cookie headers to the document.
 				// "set-cookie" and "set-cookie2" are not allowed in response headers according to spec.
 				if (lowerKey === 'set-cookie' || lowerKey === 'set-cookie2') {
-					const isCORS = FetchCORSUtility.isCORS(this.ownerDocument.location, this.request._url);
-					if (
-						this.request.credentials === 'include' ||
-						(this.request.credentials === 'same-origin' && !isCORS)
-					) {
-						this.ownerDocument.cookie = header;
-					}
+					(<CookieJar>this.ownerDocument['_cookie']).addCookieString(this.request._url, header);
 				} else {
 					headers.append(key, header);
 				}
diff --git a/packages/happy-dom/src/nodes/document/Document.ts b/packages/happy-dom/src/nodes/document/Document.ts
index df1b717b..4d0ae465 100644
--- a/packages/happy-dom/src/nodes/document/Document.ts
+++ b/packages/happy-dom/src/nodes/document/Document.ts
@@ -290,7 +290,7 @@ export default class Document extends Node implements IDocument {
 	 * @returns Cookie.
 	 */
 	public get cookie(): string {
-		return this._cookie.getCookiesString(this.defaultView.location, true);
+		return this._cookie.getCookieString(this.defaultView.location, true);
 	}
 
 	/**
@@ -299,7 +299,7 @@ export default class Document extends Node implements IDocument {
 	 * @param cookie Cookie string.
 	 */
 	public set cookie(cookie: string) {
-		this._cookie.setCookiesString(cookie);
+		this._cookie.addCookieString(this.defaultView.location, cookie);
 	}
 
 	/**
diff --git a/packages/happy-dom/src/xml-http-request/XMLHttpRequest.ts b/packages/happy-dom/src/xml-http-request/XMLHttpRequest.ts
index 5d1bcb85..08854095 100644
--- a/packages/happy-dom/src/xml-http-request/XMLHttpRequest.ts
+++ b/packages/happy-dom/src/xml-http-request/XMLHttpRequest.ts
@@ -558,7 +558,7 @@ export default class XMLHttpRequest extends XMLHttpRequestEventTarget {
 			accept: '*/*',
 			referer: location.href,
 			'user-agent': navigator.userAgent,
-			cookie: document.cookie
+			cookie: document._cookie.getCookieString(location, false)
 		};
 	}
 
@@ -973,13 +973,17 @@ export default class XMLHttpRequest extends XMLHttpRequestEventTarget {
 	 * @param headers String array.
 	 */
 	private _setCookies(headers: string[] | HTTP.IncomingHttpHeaders): void {
+		const originURL = new URL(this._settings.url, this._ownerDocument.defaultView.location);
 		for (const header of ['set-cookie', 'set-cookie2']) {
 			if (Array.isArray(headers[header])) {
 				for (const cookie of headers[header]) {
-					this._ownerDocument.defaultView.document._cookie.setCookiesString(cookie);
+					this._ownerDocument.defaultView.document._cookie.addCookieString(originURL, cookie);
 				}
 			} else if (headers[header]) {
-				this._ownerDocument.defaultView.document._cookie.setCookiesString(headers[header]);
+				this._ownerDocument.defaultView.document._cookie.addCookieString(
+					originURL,
+					headers[header]
+				);
 			}
 		}
 	}


Test patch:
diff --git a/packages/happy-dom/test/cookie/CookieJar.test.ts b/packages/happy-dom/test/cookie/CookieJar.test.ts
new file mode 100644
index 00000000..41020831
--- /dev/null
+++ b/packages/happy-dom/test/cookie/CookieJar.test.ts
@@ -0,0 +1,116 @@
+import CookieJar from '../../src/cookie/CookieJar';
+import { URL } from 'url';
+
+describe('CookieJar', () => {
+	let cookieJar: CookieJar;
+
+	beforeEach(() => {
+		cookieJar = new CookieJar();
+	});
+
+	afterEach(() => {
+		jest.restoreAllMocks();
+	});
+
+	describe('addCookieString()', () => {
+		it('Adds cookie string.', () => {
+			const expires = 60 * 1000 + Date.now();
+			const originURL = new URL('https://example.com/path/to/page/');
+			const maxAge = 60;
+
+			cookieJar.addCookieString(originURL, `key1=value1; Expires=${new Date(expires).toString()};`);
+			cookieJar.addCookieString(originURL, `key2=value2; Max-Age=${maxAge};`);
+			cookieJar.addCookieString(originURL, `key3=value3; Domain=example.com;`);
+			cookieJar.addCookieString(originURL, `key4=value4; Domain=other.com;`);
+			cookieJar.addCookieString(originURL, `key5=value5; Domain=other.com; SameSite=None;`);
+			cookieJar.addCookieString(originURL, `key6=value6; Domain=other.com; SameSite=None; Secure;`);
+			cookieJar.addCookieString(originURL, `key7=value7; Path=path/to/page/;`);
+			cookieJar.addCookieString(originURL, `key8=value8; HttpOnly;`);
+			cookieJar.addCookieString(originURL, `key9=value9; Secure;`);
+			cookieJar.addCookieString(originURL, `key10=value10; SameSite=None; Secure;`);
+			cookieJar.addCookieString(originURL, `key10;`);
+
+			expect(cookieJar.getCookieString(new URL('https://example.com/path/to/page/'), false)).toBe(
+				'key1=value1; key2=value2; key3=value3; key7=value7; key8=value8; key9=value9; key10=value10; key10'
+			);
+
+			expect(cookieJar.getCookieString(new URL('https://example.com/path/to/page/'), true)).toBe(
+				'key1=value1; key2=value2; key3=value3; key7=value7; key9=value9; key10=value10; key10'
+			);
+
+			expect(cookieJar.getCookieString(new URL('http://example.com/path/to/page/'), false)).toBe(
+				'key1=value1; key2=value2; key3=value3; key7=value7; key8=value8; key10'
+			);
+
+			expect(cookieJar.getCookieString(new URL('https://other.com/path/to/page/'), false)).toBe(
+				'key6=value6; key10=value10'
+			);
+
+			cookieJar.addCookieString(originURL, `key10=newValue10`);
+
+			expect(cookieJar.getCookieString(new URL('https://example.com/path/to/page/'), false)).toBe(
+				'key1=value1; key2=value2; key3=value3; key7=value7; key8=value8; key9=value9; key10; key10=newValue10'
+			);
+
+			expect(cookieJar.getCookieString(new URL('https://other.com/path/to/page/'), false)).toBe(
+				'key6=value6'
+			);
+
+			jest.spyOn(Date, 'now').mockImplementation(() => expires + 1000);
+
+			expect(cookieJar.getCookieString(new URL('https://example.com/path/to/page/'), false)).toBe(
+				'key3=value3; key7=value7; key8=value8; key9=value9; key10; key10=newValue10'
+			);
+
+			cookieJar.addCookieString(originURL, `key10; Expires=${new Date(expires).toString()};`);
+
+			expect(cookieJar.getCookieString(new URL('https://example.com/path/to/page/'), false)).toBe(
+				'key3=value3; key7=value7; key8=value8; key9=value9; key10=newValue10'
+			);
+
+			cookieJar.addCookieString(originURL, `key10=; Expires=${new Date(expires).toString()};`);
+
+			expect(cookieJar.getCookieString(new URL('https://example.com/path/to/page/'), false)).toBe(
+				'key3=value3; key7=value7; key8=value8; key9=value9'
+			);
+		});
+
+		it('Validates secure cookie keys.', () => {
+			const originURL = new URL('https://example.com/path/to/page/');
+			const targetURL = new URL('https://example.com/path/to/page/');
+
+			cookieJar.addCookieString(originURL, `__secure-key=value`);
+
+			expect(cookieJar.getCookieString(targetURL, false)).toBe('');
+
+			cookieJar.addCookieString(originURL, `__secure-key=value; Secure;`);
+
+			expect(cookieJar.getCookieString(targetURL, false)).toBe('__secure-key=value');
+		});
+
+		it('Validates host cookie keys.', () => {
+			const originURL = new URL('https://example.com/path/to/page/');
+			const targetURL = new URL('https://example.com/path/to/page/');
+
+			cookieJar.addCookieString(originURL, `__host-key=value`);
+
+			expect(cookieJar.getCookieString(targetURL, false)).toBe('');
+
+			cookieJar.addCookieString(originURL, `__host-key=value; Secure;`);
+
+			expect(cookieJar.getCookieString(targetURL, false)).toBe('');
+
+			cookieJar.addCookieString(originURL, `__host-key=value; Secure; Path=/path/to/page/;`);
+
+			expect(cookieJar.getCookieString(targetURL, false)).toBe('');
+
+			cookieJar.addCookieString(originURL, `__host-key=value; Secure; Domain=example.com; Path=/;`);
+
+			expect(cookieJar.getCookieString(targetURL, false)).toBe('');
+
+			cookieJar.addCookieString(originURL, `__host-key=value; Secure; Path=/;`);
+
+			expect(cookieJar.getCookieString(targetURL, false)).toBe('__host-key=value');
+		});
+	});
+});
diff --git a/packages/happy-dom/test/fetch/Fetch.test.ts b/packages/happy-dom/test/fetch/Fetch.test.ts
index 0fc4e318..49aba43b 100644
--- a/packages/happy-dom/test/fetch/Fetch.test.ts
+++ b/packages/happy-dom/test/fetch/Fetch.test.ts
@@ -993,6 +993,7 @@ describe('Fetch', () => {
 				options: { method: string; headers: { [k: string]: string } };
 			} | null = null;
 
+			window.happyDOM.setURL('https://localhost:8080');
 			window.document.cookie = 'test=cookie';
 
 			mockModule('https', {
@@ -1033,6 +1034,7 @@ describe('Fetch', () => {
 					headers: {
 						Accept: '*/*',
 						Connection: 'close',
+						Referer: 'https://localhost:8080/',
 						'User-Agent': window.navigator.userAgent,
 						'Accept-Encoding': 'gzip, deflate, br'
 					}
@@ -1282,6 +1284,8 @@ describe('Fetch', () => {
 		});
 
 		it('Sets document cookie string if the response contains a "Set-Cookie" header if request cridentials are set to "include".', async () => {
+			window.happyDOM.setURL('https://localhost:8080');
+
 			mockModule('https', {
 				request: () => {
 					return {
diff --git a/packages/happy-dom/test/nodes/document/Document.test.ts b/packages/happy-dom/test/nodes/document/Document.test.ts
index d893cc3d..aa261f34 100644
--- a/packages/happy-dom/test/nodes/document/Document.test.ts
+++ b/packages/happy-dom/test/nodes/document/Document.test.ts
@@ -219,7 +219,7 @@ describe('Document', () => {
 		});
 
 		it('Unset previous cookie.', () => {
-			document.cookie = 'name=Dave; expires=Thu, 01 Jan 2025 00:00:00 GMT;';
+			document.cookie = 'name=Dave; expires=Thu, 01 Jan 2030 00:00:00 GMT;';
 			expect(document.cookie).toBe('name=Dave');
 			document.cookie = 'name=; expires=Thu, 01 Jan 1970 00:00:00 GMT;';
 			expect(document.cookie).toBe('');