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

Accept URL parameter in getCookies and setCookie #364

Merged
merged 3 commits into from
Feb 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 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 @@ -499,7 +498,7 @@
})

it('should be able to get the cookies for http://nodejs.org', async () => {
const cookies = await cookieJar.getCookies('http://nodejs.org')

Check warning on line 501 in lib/__tests__/cookieJar.spec.ts

View workflow job for this annotation

GitHub Actions / build (latest)

File has too many lines (1536). Maximum allowed is 500

Check warning on line 501 in lib/__tests__/cookieJar.spec.ts

View workflow job for this annotation

GitHub Actions / build (lts/*)

File has too many lines (1536). Maximum allowed is 500

Check warning on line 501 in lib/__tests__/cookieJar.spec.ts

View workflow job for this annotation

GitHub Actions / build (lts/-1)

File has too many lines (1536). Maximum allowed is 500
expect(cookies).toEqual([
expect.objectContaining({
key: 'f',
Expand Down Expand Up @@ -1173,7 +1172,7 @@
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 not a string or URL.')
})

it('should fix issue #197 - CookieJar().setCookie throws an error when empty cookie is passed', async () => {
Expand Down Expand Up @@ -1241,6 +1240,21 @@
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 not a string or URL.')
}

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
Loading