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

Remove url-parse and punycode #429

Merged
merged 9 commits into from
Jul 18, 2024
Merged
Show file tree
Hide file tree
Changes from 8 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
15 changes: 12 additions & 3 deletions lib/__tests__/cookieJar.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1068,7 +1068,7 @@ describe('setCookie errors', () => {
it('should throw an error if domain is set to a public suffix', async () => {
const cookieJar = new CookieJar()
await expect(
cookieJar.setCookie('i=9; Domain=kyoto.jp; Path=/', 'kyoto.jp'),
cookieJar.setCookie('i=9; Domain=kyoto.jp; Path=/', 'http://kyoto.jp'),
).rejects.toThrowError('Cookie has domain set to a public suffix')
})

Expand Down Expand Up @@ -1104,6 +1104,13 @@ describe('setCookie errors', () => {
})
expect(cookies).toEqual([httpCookie])
})

it('should throw when URL is missing protocol', async () => {
const cookieJar = new CookieJar()
await expect(
cookieJar.setCookie('L=12; Domain=example.ch; Path=/', 'example.ch'),
).rejects.toThrow(new TypeError('Invalid URL'))
})
})

describe('loose mode', () => {
Expand Down Expand Up @@ -1489,25 +1496,27 @@ describe('Synchronous API on async CookieJar', () => {
})

describe('validation errors invoke callbacks', () => {
it('getCookies', () => {
it('getCookies', (done) => {
const invalidUrl = {}
const cookieJar = new CookieJar()
// @ts-expect-error deliberately trigger validation error
void cookieJar.getCookies(invalidUrl, (err) => {
expect(err).toMatchObject({
message: '`url` argument is not a string or URL.',
})
done()
colincasey marked this conversation as resolved.
Show resolved Hide resolved
})
})

it('setCookie', () => {
it('setCookie', (done) => {
const invalidUrl = {}
const cookieJar = new CookieJar()
// @ts-expect-error deliberately trigger validation error
void cookieJar.setCookie('a=b', invalidUrl, (err) => {
expect(err).toMatchObject({
message: '`url` argument is not a string or URL.',
})
done()
})
})
})
Expand Down
5 changes: 5 additions & 0 deletions lib/__tests__/domainMatch.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,11 @@ describe('domainMatch', () => {
['com', 'net', false], // same len, non-match
['com', 'com', true], // "are identical" rule
['NOTATLD', 'notaTLD', true], // "are identical" rule (after canonicalization)

// non-ASCII hostnames
['🫠.com', 'xn--129h.com', true], // Emoji!
['ρһіѕһ.info', 'xn--2xa01ac71bc.info', true], // Greek + Cyrillic characters
['猫.cat', 'xn--z7x.cat', true], // Japanese characters
])('domainMatch(%s, %s) => %s', (string, domain, expectedValue) => {
expect(domainMatch(string, domain)).toBe(expectedValue)
})
Expand Down
20 changes: 13 additions & 7 deletions lib/cookie/canonicalDomain.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { toASCII } from 'punycode/punycode.js'
import { IP_V6_REGEX_OBJECT } from './constants'
import type { Nullable } from '../utils'

Expand Down Expand Up @@ -39,17 +38,24 @@ export function canonicalDomain(
if (domainName == null) {
return undefined
}
let _str = domainName.trim().replace(/^\./, '') // S4.1.2.3 & S5.2.3: ignore leading .
let str = domainName.trim().replace(/^\./, '') // S4.1.2.3 & S5.2.3: ignore leading .

if (IP_V6_REGEX_OBJECT.test(_str)) {
_str = _str.replace('[', '').replace(']', '')
if (IP_V6_REGEX_OBJECT.test(str)) {
if (!str.startsWith('[')) {
str = '[' + str
}
if (!str.endsWith(']')) {
str = str + ']'
}
return new URL(`http://${str}`).hostname.slice(1, -1) // remove [ and ]
}

// convert to IDN if any non-ASCII characters
// eslint-disable-next-line no-control-regex
if (/[^\u0001-\u007f]/.test(_str)) {
_str = toASCII(_str)
if (/[^\u0001-\u007f]/.test(str)) {
return new URL(`http://${str}`).hostname
}

return _str.toLowerCase()
// ASCII-only domain - not canonicalized with new URL() because it may be a malformed URL
return str.toLowerCase()
}
27 changes: 13 additions & 14 deletions lib/cookie/cookieJar.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import urlParse from 'url-parse'

import { getPublicSuffix } from '../getPublicSuffix'
import * as validators from '../validators'
import { ParameterError } from '../validators'
Expand Down Expand Up @@ -190,14 +188,14 @@ export interface CreateCookieJarOptions {
const SAME_SITE_CONTEXT_VAL_ERR =
'Invalid sameSiteContext option for getCookies(); expected one of "strict", "lax", or "none"'

function getCookieContext(url: unknown): URL | urlParse<string> {
function getCookieContext(url: unknown): URL {
if (url instanceof URL) {
return url
} else if (typeof url === 'string') {
try {
return urlParse(decodeURI(url))
return new URL(decodeURI(url))
} catch {
return urlParse(url)
return new URL(url)
}
} else {
throw new ParameterError('`url` argument is not a string or URL.')
Expand Down Expand Up @@ -437,7 +435,7 @@ export class CookieJar {
}
const promiseCallback = createPromiseCallback(callback)
const cb = promiseCallback.callback
let context
let context: URL

try {
if (typeof url === 'string') {
Expand Down Expand Up @@ -475,7 +473,6 @@ export class CookieJar {
const host = canonicalDomain(context.hostname) ?? null
const loose = options?.loose || this.enableLooseMode

let err
let sameSiteContext = null
if (options?.sameSiteContext) {
sameSiteContext = checkSameSiteContext(options.sameSiteContext)
Expand All @@ -488,7 +485,7 @@ export class CookieJar {
if (typeof cookie === 'string' || cookie instanceof String) {
const parsedCookie = Cookie.parse(cookie.toString(), { loose: loose })
if (!parsedCookie) {
err = new Error('Cookie failed to parse')
const err = new Error('Cookie failed to parse')
return options?.ignoreError
? promiseCallback.resolve(undefined)
: promiseCallback.reject(err)
Expand All @@ -497,7 +494,7 @@ export class CookieJar {
} else if (!(cookie instanceof Cookie)) {
// If you're seeing this error, and are passing in a Cookie object,
// it *might* be a Cookie object from another loaded version of tough-cookie.
err = new Error(
const err = new Error(
'First argument to setCookie must be a Cookie object or string',
)

Expand Down Expand Up @@ -526,7 +523,7 @@ export class CookieJar {
: null
if (suffix == null && !IP_V6_REGEX_OBJECT.test(cookie.domain)) {
// e.g. "com"
err = new Error('Cookie has domain set to a public suffix')
const err = new Error('Cookie has domain set to a public suffix')

return options?.ignoreError
? promiseCallback.resolve(undefined)
Expand All @@ -549,7 +546,7 @@ export class CookieJar {
if (
!domainMatch(host ?? undefined, cookie.cdomain() ?? undefined, false)
) {
err = new Error(
const err = new Error(
`Cookie not in this host's domain. Cookie:${
cookie.cdomain() ?? 'null'
} Request:${host ?? 'null'}`,
Expand Down Expand Up @@ -581,7 +578,7 @@ export class CookieJar {

// S5.3 step 10
if (options?.http === false && cookie.httpOnly) {
err = new Error("Cookie is HttpOnly and this isn't an HTTP API")
const err = new Error("Cookie is HttpOnly and this isn't an HTTP API")
return options.ignoreError
? promiseCallback.resolve(undefined)
: promiseCallback.reject(err)
Expand All @@ -598,7 +595,9 @@ export class CookieJar {
// exact match for request-uri's host's registered domain, then
// abort these steps and ignore the newly created cookie entirely."
if (sameSiteContext === 'none') {
err = new Error('Cookie is SameSite but this is a cross-origin request')
const err = new Error(
'Cookie is SameSite but this is a cross-origin request',
)
return options?.ignoreError
? promiseCallback.resolve(undefined)
: promiseCallback.reject(err)
Expand Down Expand Up @@ -820,7 +819,7 @@ export class CookieJar {
}
const promiseCallback = createPromiseCallback(callback)
const cb = promiseCallback.callback
let context
let context: URL

try {
if (typeof url === 'string') {
Expand Down
38 changes: 2 additions & 36 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 1 addition & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,6 @@
"@microsoft/api-extractor": "^7.47.0",
"@types/jest": "^29.5.12",
"@types/node": "^14.18.63",
"@types/punycode": "^2.1.4",
"@types/url-parse": "^1.4.11",
"async": "3.2.5",
"eslint": "^8.57.0",
"eslint-config-prettier": "^9.1.0",
Expand All @@ -134,8 +132,6 @@
"vows": "^0.8.3"
},
"dependencies": {
"punycode": "^2.3.1",
"tldts": "^6.1.28",
"url-parse": "^1.5.10"
"tldts": "^6.1.28"
}
}
2 changes: 1 addition & 1 deletion test/cookie_jar_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -531,7 +531,7 @@ vows
const cj = new CookieJar();
cj.setCookie(
"i=9; Domain=kyoto.jp; Path=/",
"kyoto.jp",
"http://kyoto.jp",
this.callback
);
},
Expand Down