Skip to content

Commit

Permalink
Accept URL parameter in getCookies and setCookie
Browse files Browse the repository at this point in the history
This is a version of PR #261 that is compatible with our v5 TypeScript code.
  • Loading branch information
colincasey committed Feb 5, 2024
1 parent aa1ee67 commit ddf37ed
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 27 deletions.
18 changes: 16 additions & 2 deletions lib/__tests__/cookieJar.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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',
Expand Down
55 changes: 30 additions & 25 deletions lib/cookie/cookieJar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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<Cookie | undefined>,
): void
setCookie(
cookie: string | Cookie,
url: string,
url: string | URL,
options: SetCookieOptions,
callback: Callback<Cookie | undefined>,
): void
setCookie(
cookie: string | Cookie,
url: string,
url: string | URL,
options?: SetCookieOptions,
): Promise<Cookie | undefined>
setCookie(
cookie: string | Cookie,
url: string,
url: string | URL,
options: SetCookieOptions | Callback<Cookie | undefined>,
callback?: Callback<Cookie | undefined>,
): unknown
setCookie(
cookie: string | Cookie,
url: string,
url: string | URL,
options?: SetCookieOptions | Callback<Cookie | undefined>,
callback?: Callback<Cookie | undefined>,
): unknown {
Expand All @@ -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
}
Expand Down Expand Up @@ -498,21 +501,21 @@ export class CookieJar {
// RFC6365 S5.4
getCookies(url: string, callback: Callback<Cookie[]>): void
getCookies(
url: string,
url: string | URL,
options: GetCookiesOptions | undefined,
callback: Callback<Cookie[]>,
): void
getCookies(
url: string,
url: string | URL,
options?: GetCookiesOptions | undefined,
): Promise<Cookie[]>
getCookies(
url: string,
url: string | URL,
options: GetCookiesOptions | undefined | Callback<Cookie[]>,
callback?: Callback<Cookie[]>,
): unknown
getCookies(
url: string,
url: string | URL,
options?: GetCookiesOptions | Callback<Cookie[]>,
callback?: Callback<Cookie[]>,
): unknown {
Expand All @@ -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 || '/'
Expand Down

0 comments on commit ddf37ed

Please sign in to comment.