From ddf37ed93b3e29f9bdca3be1279e1b6927037146 Mon Sep 17 00:00:00 2001 From: Colin Casey Date: Mon, 5 Feb 2024 11:55:46 -0400 Subject: [PATCH 1/3] Accept URL parameter in `getCookies` and `setCookie` This is a version of PR #261 that is compatible with our v5 TypeScript code. --- lib/__tests__/cookieJar.spec.ts | 18 +++++++++-- lib/cookie/cookieJar.ts | 55 ++++++++++++++++++--------------- 2 files changed, 46 insertions(+), 27 deletions(-) diff --git a/lib/__tests__/cookieJar.spec.ts b/lib/__tests__/cookieJar.spec.ts index d3f591d2..cb76d9ed 100644 --- a/lib/__tests__/cookieJar.spec.ts +++ b/lib/__tests__/cookieJar.spec.ts @@ -34,7 +34,6 @@ import { CookieJar } from '../cookie/cookieJar' import type { SerializedCookieJar } from '../cookie/constants' import { MemoryCookieStore } from '../memstore' import { Store } from '../store' -import { ParameterError } from '../validators' // ported from: // - test/api_test.js (cookie jar tests) @@ -1173,7 +1172,7 @@ it('should fix issue #145 - missing 2nd url parameter', () => { expect( // @ts-expect-error test case explicitly violates the expected function signature () => cookieJar.setCookie('x=y; Domain=example.com; Path=/'), - ).toThrow(ParameterError) + ).toThrowError('`url` argument is invalid') }) it('should fix issue #197 - CookieJar().setCookie throws an error when empty cookie is passed', async () => { @@ -1241,6 +1240,21 @@ it('should fix issue #154 - Expiry should not be affected by creation date', asy expect(updatedCookies[0]?.expiryTime()).toBe(now + 60 * 1000 + 1000) }) +it('should fix issue #261 - URL objects should be accepted in setCookie', async () => { + const jar = new CookieJar() + const url = new URL('https://example.com') + await jar.setCookie('foo=bar; Max-Age=60;', url) + const cookies = await jar.getCookies(url) + expect(cookies).toEqual([ + expect.objectContaining({ + key: 'foo', + value: 'bar', + path: '/', + domain: 'example.com', + }), + ]) +}) + // special use domains under a sub-domain describe.each(['local', 'example', 'invalid', 'localhost', 'test'])( 'when special use domain is dev.%s', diff --git a/lib/cookie/cookieJar.ts b/lib/cookie/cookieJar.ts index 6b60e099..08793ac0 100644 --- a/lib/cookie/cookieJar.ts +++ b/lib/cookie/cookieJar.ts @@ -2,14 +2,15 @@ import urlParse from 'url-parse' import { getPublicSuffix } from '../getPublicSuffix' import * as validators from '../validators' +import { ParameterError } from '../validators' import { Store } from '../store' import { MemoryCookieStore } from '../memstore' import { pathMatch } from '../pathMatch' import { Cookie } from './cookie' import { Callback, - ErrorCallback, createPromiseCallback, + ErrorCallback, inOperator, safeToString, } from '../utils' @@ -68,20 +69,18 @@ type CreateCookieJarOptions = { const SAME_SITE_CONTEXT_VAL_ERR = 'Invalid sameSiteContext option for getCookies(); expected one of "strict", "lax", or "none"' -function getCookieContext(url: string | URL) { - if (url instanceof URL && 'query' in url) { +function getCookieContext(url: unknown) { + if (url instanceof URL) { return url - } - - if (typeof url === 'string') { + } else if (typeof url === 'string') { try { return urlParse(decodeURI(url)) } catch { return urlParse(url) } + } else { + throw new ParameterError('`url` argument is invalid') } - - throw new Error('`url` argument is invalid') } function checkSameSiteContext(value: string) { @@ -197,29 +196,29 @@ export class CookieJar { // return `undefined` when `ignoreError` is true. But would that be excessive overloading? setCookie( cookie: string | Cookie, - url: string, + url: string | URL, callback: Callback, ): void setCookie( cookie: string | Cookie, - url: string, + url: string | URL, options: SetCookieOptions, callback: Callback, ): void setCookie( cookie: string | Cookie, - url: string, + url: string | URL, options?: SetCookieOptions, ): Promise setCookie( cookie: string | Cookie, - url: string, + url: string | URL, options: SetCookieOptions | Callback, callback?: Callback, ): unknown setCookie( cookie: string | Cookie, - url: string, + url: string | URL, options?: SetCookieOptions | Callback, callback?: Callback, ): unknown { @@ -230,18 +229,22 @@ export class CookieJar { const promiseCallback = createPromiseCallback(callback) const cb = promiseCallback.callback - validators.validate( - validators.isNonEmptyString(url), - callback, - safeToString(options), - ) + if (typeof url === 'string') { + validators.validate( + validators.isNonEmptyString(url), + callback, + safeToString(options), + ) + } + + const context = getCookieContext(url) + let err if (typeof url === 'function') { return promiseCallback.reject(new Error('No URL was specified')) } - const context = getCookieContext(url) if (typeof options === 'function') { options = defaultSetCookieOptions } @@ -498,21 +501,21 @@ export class CookieJar { // RFC6365 S5.4 getCookies(url: string, callback: Callback): void getCookies( - url: string, + url: string | URL, options: GetCookiesOptions | undefined, callback: Callback, ): void getCookies( - url: string, + url: string | URL, options?: GetCookiesOptions | undefined, ): Promise getCookies( - url: string, + url: string | URL, options: GetCookiesOptions | undefined | Callback, callback?: Callback, ): unknown getCookies( - url: string, + url: string | URL, options?: GetCookiesOptions | Callback, callback?: Callback, ): unknown { @@ -525,10 +528,12 @@ export class CookieJar { const promiseCallback = createPromiseCallback(callback) const cb = promiseCallback.callback - validators.validate(validators.isNonEmptyString(url), cb, url) + if (typeof url === 'string') { + validators.validate(validators.isNonEmptyString(url), cb, url) + } + const context = getCookieContext(url) validators.validate(validators.isObject(options), cb, safeToString(options)) validators.validate(typeof cb === 'function', cb) - const context = getCookieContext(url) const host = canonicalDomain(context.hostname) const path = context.pathname || '/' From a3ff77e80a9930bf4af81402d6c91fb04444b1bd Mon Sep 17 00:00:00 2001 From: Colin Casey Date: Wed, 21 Feb 2024 09:32:15 -0400 Subject: [PATCH 2/3] Update lib/cookie/cookieJar.ts Co-authored-by: Will Harney <62956339+wjhsf@users.noreply.github.com> --- lib/cookie/cookieJar.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cookie/cookieJar.ts b/lib/cookie/cookieJar.ts index 08793ac0..685f6ba0 100644 --- a/lib/cookie/cookieJar.ts +++ b/lib/cookie/cookieJar.ts @@ -79,7 +79,7 @@ function getCookieContext(url: unknown) { return urlParse(url) } } else { - throw new ParameterError('`url` argument is invalid') + throw new ParameterError('`url` argument is not a string or URL.') } } From 1cd55c7503255df47e3cb161f93ceaf22a8a3aa3 Mon Sep 17 00:00:00 2001 From: Colin Casey Date: Wed, 21 Feb 2024 09:34:07 -0400 Subject: [PATCH 3/3] Update cookieJar.spec.ts --- lib/__tests__/cookieJar.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/__tests__/cookieJar.spec.ts b/lib/__tests__/cookieJar.spec.ts index cb76d9ed..2a37d0b8 100644 --- a/lib/__tests__/cookieJar.spec.ts +++ b/lib/__tests__/cookieJar.spec.ts @@ -1172,7 +1172,7 @@ it('should fix issue #145 - missing 2nd url parameter', () => { expect( // @ts-expect-error test case explicitly violates the expected function signature () => cookieJar.setCookie('x=y; Domain=example.com; Path=/'), - ).toThrowError('`url` argument is invalid') + ).toThrowError('`url` argument is not a string or URL.') }) it('should fix issue #197 - CookieJar().setCookie throws an error when empty cookie is passed', async () => {