From a48c867f0444966a4c7a583dd877c326cab6f73e Mon Sep 17 00:00:00 2001 From: Will Harney <62956339+wjhsf@users.noreply.github.com> Date: Thu, 21 Mar 2024 15:46:55 -0400 Subject: [PATCH] clean up usage of null/undefined (#380) * Make usage of null/undefined more consistent. Primarily allowing either value for user input. * add explicit return type to canonicalDomain * add note for weird parse behavior * Update lib/cookie/cookie.ts Co-authored-by: Colin Casey * Replace `Nullable` with more precise `T | undefined` * add a bit of breathing room to the file size * Change `parse` to only return undefined on failure, nut null | undefined. * standardize helpers on returning undefined * update doc for return type * change fromJSON to return undefined instead of null * fix incorrect comparison * change date helpers to avoid null, for consistency * update fromJSON tests for new return type * Make test assertions more strict Apply suggestions from code review Co-authored-by: Colin Casey --------- Co-authored-by: Colin Casey --- lib/__tests__/cookieToAndFromJson.spec.ts | 2 +- lib/__tests__/domainMatch.spec.ts | 8 +- lib/__tests__/parse.spec.ts | 14 ++- lib/cookie/canonicalDomain.ts | 9 +- lib/cookie/cookie.ts | 32 +++---- lib/cookie/cookieJar.ts | 36 ++++---- lib/cookie/defaultPath.ts | 4 +- lib/cookie/domainMatch.ts | 15 ++-- lib/cookie/parseDate.ts | 81 +++++++++--------- lib/getPublicSuffix.ts | 5 +- lib/memstore.ts | 25 +++--- lib/permuteDomain.ts | 4 +- lib/store.ts | 62 +++++++------- lib/utils.ts | 3 + test/cookie_to_json_test.js | 3 +- test/parsing_test.js | 100 +++++++++++----------- 16 files changed, 204 insertions(+), 199 deletions(-) diff --git a/lib/__tests__/cookieToAndFromJson.spec.ts b/lib/__tests__/cookieToAndFromJson.spec.ts index aba74773..32d21006 100644 --- a/lib/__tests__/cookieToAndFromJson.spec.ts +++ b/lib/__tests__/cookieToAndFromJson.spec.ts @@ -47,7 +47,7 @@ describe('Cookie.fromJSON()', () => { }) it('should be able to handle a null value deserialization', () => { - expect(Cookie.fromJSON(null)).toBeNull() + expect(Cookie.fromJSON(null)).toBeUndefined() }) it('should be able to handle expiry, creation, or lastAccessed with Infinity during deserialization', () => { diff --git a/lib/__tests__/domainMatch.spec.ts b/lib/__tests__/domainMatch.spec.ts index bfc3afbe..edb00754 100644 --- a/lib/__tests__/domainMatch.spec.ts +++ b/lib/__tests__/domainMatch.spec.ts @@ -13,10 +13,10 @@ describe('domainMatch', () => { ['example.com', 'example.com.', false], // RFC6265 S4.1.2.3 // nulls and undefineds - [null, 'example.com', null], - ['example.com', null, null], - [null, null, null], - [undefined, undefined, null], + [null, 'example.com', undefined], + ['example.com', null, undefined], + [null, null, undefined], + [undefined, undefined, undefined], // suffix matching: ['www.example.com', 'example.com', true], // substr AND suffix diff --git a/lib/__tests__/parse.spec.ts b/lib/__tests__/parse.spec.ts index cb26c3c2..89e9d079 100644 --- a/lib/__tests__/parse.spec.ts +++ b/lib/__tests__/parse.spec.ts @@ -379,22 +379,22 @@ describe('Cookie.parse', () => { // empty string { input: ``, - output: null, + output: undefined, }, // missing string { input: undefined, - output: null, + output: undefined, }, // some string object { input: new String(''), - output: null, + output: undefined, }, // some empty string object { input: new String(), - output: null, + output: undefined, }, ])('Cookie.parse("$input")', (testCase) => { // Repeating the character in the input makes the jest output obnoxiously long, so instead we @@ -406,11 +406,7 @@ describe('Cookie.parse', () => { const value = input === undefined ? undefined : input.valueOf() const cookie = Cookie.parse(value as string, parseOptions) - if (output !== undefined) { - expect(cookie).toEqual(output && expect.objectContaining(output)) - } else { - expect(cookie).toBe(output) - } + expect(cookie).toEqual(output && expect.objectContaining(output)) if (cookie && typeof assertValidateReturns === 'boolean') { expect(cookie.validate()).toBe(assertValidateReturns) diff --git a/lib/cookie/canonicalDomain.ts b/lib/cookie/canonicalDomain.ts index 0535e68e..c709cbaa 100644 --- a/lib/cookie/canonicalDomain.ts +++ b/lib/cookie/canonicalDomain.ts @@ -1,10 +1,11 @@ -import * as punycode from 'punycode/punycode.js' +import { toASCII } from 'punycode/punycode.js' import { IP_V6_REGEX_OBJECT } from './constants' +import type { Nullable } from '../utils' // S5.1.2 Canonicalized Host Names -export function canonicalDomain(str: string | null): string | null { +export function canonicalDomain(str: Nullable): string | undefined { if (str == null) { - return null + return undefined } let _str = str.trim().replace(/^\./, '') // S4.1.2.3 & S5.2.3: ignore leading . @@ -15,7 +16,7 @@ export function canonicalDomain(str: string | null): string | null { // convert to IDN if any non-ASCII characters // eslint-disable-next-line no-control-regex if (/[^\u0001-\u007f]/.test(_str)) { - _str = punycode.toASCII(_str) + _str = toASCII(_str) } return _str.toLowerCase() diff --git a/lib/cookie/cookie.ts b/lib/cookie/cookie.ts index fcc916d6..782234ff 100644 --- a/lib/cookie/cookie.ts +++ b/lib/cookie/cookie.ts @@ -30,7 +30,7 @@ */ // This file was too big before we added max-lines, and it's ongoing work to reduce its size. -/* eslint max-lines: [1, 750] */ +/* eslint max-lines: [1, 800] */ import { getPublicSuffix } from '../getPublicSuffix' import * as validators from '../validators' @@ -115,12 +115,15 @@ type ParseCookieOptions = { loose?: boolean | undefined } -function parse( - str: string, - options?: ParseCookieOptions, -): Cookie | undefined | null { +/** + * Parses a string into a Cookie object. + * @param str the Set-Cookie header or a Cookie string to parse. Note: when parsing a Cookie header it must be split by ';' before each Cookie string can be parsed. + * @param options configures strict or loose mode for cookie parsing + * @returns `Cookie` object when parsing succeeds, `undefined` when parsing fails. + */ +function parse(str: string, options?: ParseCookieOptions): Cookie | undefined { if (validators.isEmptyString(str) || !validators.isString(str)) { - return null + return undefined } str = str.trim() @@ -276,9 +279,9 @@ function parse( return c } -function fromJSON(str: unknown): Cookie | null { +function fromJSON(str: unknown): Cookie | undefined { if (!str || validators.isEmptyString(str)) { - return null + return undefined } let obj: unknown @@ -286,7 +289,7 @@ function fromJSON(str: unknown): Cookie | null { try { obj = JSON.parse(str) } catch (e) { - return null + return undefined } } else { // assume it's an Object @@ -528,7 +531,7 @@ export class Cookie { return obj } - clone(): Cookie | null { + clone(): Cookie | undefined { return fromJSON(this.toJSON()) } @@ -693,15 +696,12 @@ export class Cookie { } // Mostly S5.1.2 and S5.2.3: - canonicalizedDomain(): string | null { - if (this.domain == null) { - return null - } + canonicalizedDomain(): string | undefined { return canonicalDomain(this.domain) } - cdomain(): string | null { - return this.canonicalizedDomain() + cdomain(): string | undefined { + return canonicalDomain(this.domain) } static parse = parse diff --git a/lib/cookie/cookieJar.ts b/lib/cookie/cookieJar.ts index a6aa6a8a..ec881ae9 100644 --- a/lib/cookie/cookieJar.ts +++ b/lib/cookie/cookieJar.ts @@ -9,8 +9,9 @@ import { pathMatch } from '../pathMatch' import { Cookie } from './cookie' import { Callback, - createPromiseCallback, ErrorCallback, + Nullable, + createPromiseCallback, inOperator, safeToString, } from '../utils' @@ -84,13 +85,13 @@ function getCookieContext(url: unknown): URL | urlParse { } type SameSiteLevel = keyof (typeof Cookie)['sameSiteLevel'] -function checkSameSiteContext(value: string): SameSiteLevel | null { +function checkSameSiteContext(value: string): SameSiteLevel | undefined { validators.validate(validators.isNonEmptyString(value), value) const context = String(value).toLowerCase() if (context === 'none' || context === 'lax' || context === 'strict') { return context } else { - return null + return undefined } } @@ -161,7 +162,7 @@ export class CookieJar { readonly prefixSecurity: string constructor( - store?: Store | null | undefined, + store?: Nullable, options?: CreateCookieJarOptions | boolean, ) { if (typeof options === 'boolean') { @@ -267,7 +268,7 @@ export class CookieJar { return promiseCallback.resolve(undefined) } - const host = canonicalDomain(context.hostname) + const host = canonicalDomain(context.hostname) ?? null const loose = options?.loose || this.enableLooseMode let sameSiteContext = null @@ -440,16 +441,16 @@ export class CookieJar { } } - function withCookie( - err: Error | null, - oldCookie: Cookie | undefined | null, + const withCookie: Callback = function withCookie( + err, + oldCookie, ): void { if (err) { cb(err) return } - const next = function (err: Error | null): void { + const next: ErrorCallback = function (err) { if (err) { cb(err) } else if (typeof cookie === 'string') { @@ -491,6 +492,7 @@ export class CookieJar { } } + // TODO: Refactor to avoid using a callback store.findCookie(cookie.domain, cookie.path, cookie.key, withCookie) return promiseCallback.promise } @@ -748,18 +750,13 @@ export class CookieJar { callback, ) - const next: Callback = function ( - err: Error | null, - cookies: Cookie[] | undefined, - ) { + const next: Callback = function (err, cookies) { if (err) { promiseCallback.callback(err) - } else if (cookies === undefined) { - promiseCallback.callback(null, undefined) } else { promiseCallback.callback( null, - cookies.map((c) => { + cookies?.map((c) => { return c.toString() }), ) @@ -879,7 +876,7 @@ export class CookieJar { cookies = cookies.slice() // do not modify the original - const putNext = (err: Error | null): void => { + const putNext: ErrorCallback = (err) => { if (err) { return callback(err, undefined) } @@ -896,7 +893,7 @@ export class CookieJar { return callback(e instanceof Error ? e : new Error(), undefined) } - if (cookie === null) { + if (cookie === undefined) { return putNext(null) // skip this cookie } @@ -995,7 +992,8 @@ export class CookieJar { let completedCount = 0 const removeErrors: Error[] = [] - function removeCookieCb(removeErr: Error | null): void { + // TODO: Refactor to avoid using callback + const removeCookieCb: ErrorCallback = function removeCookieCb(removeErr) { if (removeErr) { removeErrors.push(removeErr) } diff --git a/lib/cookie/defaultPath.ts b/lib/cookie/defaultPath.ts index 1436d6e0..726ea88e 100644 --- a/lib/cookie/defaultPath.ts +++ b/lib/cookie/defaultPath.ts @@ -1,12 +1,14 @@ // RFC6265 S5.1.4 Paths and Path-Match +import type { Nullable } from '../utils' + /* * "The user agent MUST use an algorithm equivalent to the following algorithm * to compute the default-path of a cookie:" * * Assumption: the path (and not query part or absolute uri) is passed in. */ -export function defaultPath(path?: string | null): string { +export function defaultPath(path?: Nullable): string { // "2. If the uri-path is empty or if the first character of the uri-path is not // a %x2F ("/") character, output %x2F ("/") and skip the remaining steps. if (!path || path.slice(0, 1) !== '/') { diff --git a/lib/cookie/domainMatch.ts b/lib/cookie/domainMatch.ts index 073f59b7..caaea90a 100644 --- a/lib/cookie/domainMatch.ts +++ b/lib/cookie/domainMatch.ts @@ -1,3 +1,4 @@ +import type { Nullable } from '../utils' import { canonicalDomain } from './canonicalDomain' // Dumped from ip-regex@4.0.0, with the following changes: @@ -9,16 +10,16 @@ const IP_REGEX_LOWERCASE = // S5.1.3 Domain Matching export function domainMatch( - str?: string | null, - domStr?: string | null, + str?: Nullable, + domStr?: Nullable, canonicalize?: boolean, -): boolean | null { +): boolean | undefined { if (str == null || domStr == null) { - return null + return undefined } - let _str: string | null - let _domStr: string | null + let _str: Nullable + let _domStr: Nullable if (canonicalize !== false) { _str = canonicalDomain(str) @@ -29,7 +30,7 @@ export function domainMatch( } if (_str == null || _domStr == null) { - return null + return undefined } /* diff --git a/lib/cookie/parseDate.ts b/lib/cookie/parseDate.ts index f71ff307..b9208489 100644 --- a/lib/cookie/parseDate.ts +++ b/lib/cookie/parseDate.ts @@ -1,4 +1,7 @@ // date-time parsing constants (RFC6265 S5.1.1) + +import type { Nullable } from '../utils' + // eslint-disable-next-line no-control-regex const DATE_DELIM = /[\x09\x20-\x2F\x3B-\x40\x5B-\x60\x7B-\x7E]/ @@ -32,7 +35,7 @@ function parseDigits( minDigits: number, maxDigits: number, trailingOK: boolean, -): number | null { +): number | undefined { let count = 0 while (count < token.length) { const c = token.charCodeAt(count) @@ -45,17 +48,17 @@ function parseDigits( // constrain to a minimum and maximum number of digits. if (count < minDigits || count > maxDigits) { - return null + return } if (!trailingOK && count != token.length) { - return null + return } return parseInt(token.slice(0, count), 10) } -function parseTime(token: string): number[] | null { +function parseTime(token: string): number[] | undefined { const parts = token.split(':') const result = [0, 0, 0] @@ -66,7 +69,7 @@ function parseTime(token: string): number[] | null { */ if (parts.length !== 3) { - return null + return } for (let i = 0; i < 3; i++) { @@ -75,12 +78,12 @@ function parseTime(token: string): number[] | null { // have a trailer const trailingOK = i == 2 const numPart = parts[i] - if (numPart == null) { - return null + if (numPart === undefined) { + return } const num = parseDigits(numPart, 1, 2, trailingOK) - if (num === null) { - return null + if (num === undefined) { + return } result[i] = num } @@ -88,7 +91,7 @@ function parseTime(token: string): number[] | null { return result } -function parseMonth(token: string): number | null { +function parseMonth(token: string): number | undefined { token = String(token).slice(0, 3).toLowerCase() switch (token) { case 'jan': @@ -116,16 +119,16 @@ function parseMonth(token: string): number | null { case 'dec': return MONTH_TO_NUM.dec default: - return null + return } } /* * RFC6265 S5.1.1 date parser (see RFC for full grammar) */ -export function parseDate(str: string | undefined | null): Date | undefined { +export function parseDate(str: Nullable): Date | undefined { if (!str) { - return undefined + return } /* RFC6265 S5.1.1: @@ -134,15 +137,15 @@ export function parseDate(str: string | undefined | null): Date | undefined { */ const tokens = str.split(DATE_DELIM) if (!tokens) { - return undefined + return } - let hour = null - let minute = null - let second = null - let dayOfMonth = null - let month = null - let year = null + let hour: number | undefined + let minute: number | undefined + let second: number | undefined + let dayOfMonth: number | undefined + let month: number | undefined + let year: number | undefined for (let i = 0; i < tokens.length; i++) { const token = (tokens[i] ?? '').trim() @@ -150,16 +153,14 @@ export function parseDate(str: string | undefined | null): Date | undefined { continue } - let result - /* 2.1. If the found-time flag is not set and the token matches the time * production, set the found-time flag and set the hour- value, * minute-value, and second-value to the numbers denoted by the digits in * the date-token, respectively. Skip the remaining sub-steps and continue * to the next date-token. */ - if (second === null) { - result = parseTime(token) + if (second === undefined) { + const result = parseTime(token) if (result) { hour = result[0] minute = result[1] @@ -173,10 +174,10 @@ export function parseDate(str: string | undefined | null): Date | undefined { * the day-of-month-value to the number denoted by the date-token. Skip * the remaining sub-steps and continue to the next date-token. */ - if (dayOfMonth === null) { + if (dayOfMonth === undefined) { // "day-of-month = 1*2DIGIT ( non-digit *OCTET )" - result = parseDigits(token, 1, 2, true) - if (result !== null) { + const result = parseDigits(token, 1, 2, true) + if (result !== undefined) { dayOfMonth = result continue } @@ -187,9 +188,9 @@ export function parseDate(str: string | undefined | null): Date | undefined { * the month denoted by the date-token. Skip the remaining sub-steps and * continue to the next date-token. */ - if (month === null) { - result = parseMonth(token) - if (result !== null) { + if (month === undefined) { + const result = parseMonth(token) + if (result !== undefined) { month = result continue } @@ -200,10 +201,10 @@ export function parseDate(str: string | undefined | null): Date | undefined { * number denoted by the date-token. Skip the remaining sub-steps and * continue to the next date-token. */ - if (year === null) { + if (year === undefined) { // "year = 2*4DIGIT ( non-digit *OCTET )" - result = parseDigits(token, 2, 4, true) - if (result !== null) { + const result = parseDigits(token, 2, 4, true) + if (result !== undefined) { year = result /* From S5.1.1: * 3. If the year-value is greater than or equal to 70 and less @@ -234,12 +235,12 @@ export function parseDate(str: string | undefined | null): Date | undefined { * So, in order as above: */ if ( - dayOfMonth === null || - month == null || - year == null || - hour == null || - minute == null || - second == null || + dayOfMonth === undefined || + month === undefined || + year === undefined || + hour === undefined || + minute === undefined || + second === undefined || dayOfMonth < 1 || dayOfMonth > 31 || year < 1601 || @@ -247,7 +248,7 @@ export function parseDate(str: string | undefined | null): Date | undefined { minute > 59 || second > 59 ) { - return undefined + return } return new Date(Date.UTC(year, month, dayOfMonth, hour, minute, second)) diff --git a/lib/getPublicSuffix.ts b/lib/getPublicSuffix.ts index 7e7967cf..63ca2c88 100644 --- a/lib/getPublicSuffix.ts +++ b/lib/getPublicSuffix.ts @@ -49,7 +49,7 @@ const defaultGetPublicSuffixOptions: GetPublicSuffixOptions = { export function getPublicSuffix( domain: string, options: GetPublicSuffixOptions = {}, -): string | null { +): string | undefined { options = { ...defaultGetPublicSuffixOptions, ...options } const domainParts = domain.split('.') const topLevelDomain = domainParts[domainParts.length - 1] @@ -84,8 +84,9 @@ export function getPublicSuffix( ) } - return getDomain(domain, { + const publicSuffix = getDomain(domain, { allowIcannDomains: true, allowPrivateDomains: true, }) + if (publicSuffix) return publicSuffix } diff --git a/lib/memstore.ts b/lib/memstore.ts index c1e1c2f7..e66a683f 100644 --- a/lib/memstore.ts +++ b/lib/memstore.ts @@ -33,7 +33,12 @@ import type { Cookie } from './cookie/cookie' import { pathMatch } from './pathMatch' import { permuteDomain } from './permuteDomain' import { Store } from './store' -import { Callback, createPromiseCallback, ErrorCallback } from './utils' +import { + Callback, + createPromiseCallback, + ErrorCallback, + Nullable, +} from './utils' export type MemoryCookieStoreIndex = { [domain: string]: { @@ -54,20 +59,20 @@ export class MemoryCookieStore extends Store { } override findCookie( - domain: string | null, - path: string | null, - key: string | undefined, + domain: Nullable, + path: Nullable, + key: Nullable, ): Promise override findCookie( - domain: string | null, - path: string | null, - key: string | undefined, + domain: Nullable, + path: Nullable, + key: Nullable, callback: Callback, ): void override findCookie( - domain: string | null, - path: string | null, - key: string | undefined, + domain: Nullable, + path: Nullable, + key: Nullable, callback?: Callback, ): unknown { const promiseCallback = createPromiseCallback(callback) diff --git a/lib/permuteDomain.ts b/lib/permuteDomain.ts index 195d7267..26d3780c 100644 --- a/lib/permuteDomain.ts +++ b/lib/permuteDomain.ts @@ -37,13 +37,13 @@ import { getPublicSuffix } from './getPublicSuffix' export function permuteDomain( domain: string, allowSpecialUseDomain?: boolean, -): string[] | null { +): string[] | undefined { const pubSuf = getPublicSuffix(domain, { allowSpecialUseDomain: allowSpecialUseDomain, }) if (!pubSuf) { - return null + return undefined } if (pubSuf == domain) { return [domain] diff --git a/lib/store.ts b/lib/store.ts index 545c4723..f23eb462 100644 --- a/lib/store.ts +++ b/lib/store.ts @@ -36,7 +36,7 @@ 'use strict' import type { Cookie } from './cookie/cookie' -import type { Callback, ErrorCallback } from './utils' +import type { Callback, ErrorCallback, Nullable } from './utils' export class Store { synchronous: boolean @@ -46,39 +46,39 @@ export class Store { } findCookie( - domain: string | null, - path: string | null, - key: string | undefined, - ): Promise + domain: Nullable, + path: Nullable, + key: Nullable, + ): Promise findCookie( - domain: string | null, - path: string | null, - key: string | undefined, - callback: Callback, + domain: Nullable, + path: Nullable, + key: Nullable, + callback: Callback, ): void findCookie( - _domain: string | null, - _path: string | null, - _key: string | undefined, - _callback?: Callback, + _domain: Nullable, + _path: Nullable, + _key: Nullable, + _callback?: Callback, ): unknown { throw new Error('findCookie is not implemented') } findCookies( - domain: string | null, - path: string | null, + domain: Nullable, + path: Nullable, allowSpecialUseDomain?: boolean, ): Promise findCookies( - domain: string | null, - path: string | null, + domain: Nullable, + path: Nullable, allowSpecialUseDomain?: boolean, callback?: Callback, ): void findCookies( - _domain: string | null, - _path: string | null, + _domain: Nullable, + _path: Nullable, _allowSpecialUseDomain: boolean | Callback = false, _callback?: Callback, ): unknown { @@ -108,34 +108,34 @@ export class Store { } removeCookie( - domain: string | null | undefined, - path: string | null | undefined, - key: string | null | undefined, + domain: Nullable, + path: Nullable, + key: Nullable, ): Promise removeCookie( - domain: string | null | undefined, - path: string | null | undefined, - key: string | null | undefined, + domain: Nullable, + path: Nullable, + key: Nullable, callback: ErrorCallback, ): void removeCookie( - _domain: string | null | undefined, - _path: string | null | undefined, - _key: string | null | undefined, + _domain: Nullable, + _path: Nullable, + _key: Nullable, _callback?: ErrorCallback, ): unknown { throw new Error('removeCookie is not implemented') } - removeCookies(domain: string, path: string | null): Promise + removeCookies(domain: string, path: Nullable): Promise removeCookies( domain: string, - path: string | null, + path: Nullable, callback: ErrorCallback, ): void removeCookies( _domain: string, - _path: string | null, + _path: Nullable, _callback?: ErrorCallback, ): unknown { throw new Error('removeCookies is not implemented') diff --git a/lib/utils.ts b/lib/utils.ts index d97a5ece..c3068b7d 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -9,6 +9,9 @@ export interface ErrorCallback { (error: Error | null): void } +/** The inverse of NonNullable. */ +export type Nullable = T | null | undefined + /** Wrapped `Object.prototype.toString`, so that you don't need to remember to use `.call()`. */ export const objectToString = (obj: unknown): string => Object.prototype.toString.call(obj) diff --git a/test/cookie_to_json_test.js b/test/cookie_to_json_test.js index a7b4ca7f..152e55d1 100644 --- a/test/cookie_to_json_test.js +++ b/test/cookie_to_json_test.js @@ -79,7 +79,8 @@ vows }, "null deserialization": { topic: function() { - return Cookie.fromJSON(null); + // vows breaks if you try to return `undefined` :( + return Cookie.fromJSON(null) ?? null; }, "is null": function(cookie) { assert.equal(cookie, null); diff --git a/test/parsing_test.js b/test/parsing_test.js index 0ed27dd1..34a6eb54 100644 --- a/test/parsing_test.js +++ b/test/parsing_test.js @@ -32,18 +32,22 @@ "use strict"; const vows = require("vows"); const assert = require("assert"); -const tough = require("../dist/cookie"); -const Cookie = tough.Cookie; +const {Cookie} = require("../dist/cookie"); const LOTS_OF_SEMICOLONS = ";".repeat(65535); const LOTS_OF_SPACES = " ".repeat(65535); +/** + * Hacky workaround for the fact that vows complains "callback not fired" if a topic returns `undefined`. + */ +const SHOULD_BE_UNDEFINED = Symbol("vows breaks if you try to return `undefined` from a topic.") + vows .describe("Parsing") .addBatch({ simple: { topic: function() { - return Cookie.parse("a=bcd") || null; + return Cookie.parse("a=bcd"); }, parsed: function(c) { assert.ok(c); @@ -66,9 +70,7 @@ vows }, "with expiry": { topic: function() { - return ( - Cookie.parse("a=bcd; Expires=Tue, 18 Oct 2011 07:05:03 GMT") || null - ); + return Cookie.parse("a=bcd; Expires=Tue, 18 Oct 2011 07:05:03 GMT"); }, parsed: function(c) { assert.ok(c); @@ -89,11 +91,7 @@ vows }, "with expiry and path": { topic: function() { - return ( - Cookie.parse( - 'abc="xyzzy!"; Expires=Tue, 18 Oct 2011 07:05:03 GMT; Path=/aBc' - ) || null - ); + return Cookie.parse('abc="xyzzy!"; Expires=Tue, 18 Oct 2011 07:05:03 GMT; Path=/aBc'); }, parsed: function(c) { assert.ok(c); @@ -121,10 +119,8 @@ vows }, "with most things": { topic: function() { - return ( - Cookie.parse( - 'abc="xyzzy!"; Expires=Tue, 18 Oct 2011 07:05:03 GMT; Path=/aBc; Domain=example.com; Secure; HTTPOnly; Max-Age=1234; Foo=Bar; Baz' - ) || null + return Cookie.parse( + 'abc="xyzzy!"; Expires=Tue, 18 Oct 2011 07:05:03 GMT; Path=/aBc; Domain=example.com; Secure; HTTPOnly; Max-Age=1234; Foo=Bar; Baz' ); }, parsed: function(c) { @@ -199,7 +195,7 @@ vows }, "trailing dot in domain": { topic: function() { - return Cookie.parse("a=b; Domain=example.com.", true) || null; + return Cookie.parse("a=b; Domain=example.com.", true); }, "has the domain": function(c) { assert.equal(c.domain, "example.com."); @@ -242,15 +238,15 @@ vows }, garbage: { topic: function() { - return Cookie.parse("\x08", true) || null; + return Cookie.parse("\x08", true) ?? SHOULD_BE_UNDEFINED; }, "doesn't parse": function(c) { - assert.equal(c, null); + assert.equal(c, SHOULD_BE_UNDEFINED); } }, "public suffix domain": { topic: function() { - return Cookie.parse("a=b; domain=kyoto.jp", true) || null; + return Cookie.parse("a=b; domain=kyoto.jp", true); }, "parses fine": function(c) { assert.ok(c); @@ -264,7 +260,7 @@ vows "public suffix foonet.net": { "top level": { topic: function() { - return Cookie.parse("a=b; domain=foonet.net") || null; + return Cookie.parse("a=b; domain=foonet.net"); }, "parses and is valid": function(c) { assert.ok(c); @@ -274,7 +270,7 @@ vows }, www: { topic: function() { - return Cookie.parse("a=b; domain=www.foonet.net") || null; + return Cookie.parse("a=b; domain=www.foonet.net"); }, "parses and is valid": function(c) { assert.ok(c); @@ -284,7 +280,7 @@ vows }, "with a dot": { topic: function() { - return Cookie.parse("a=b; domain=.foonet.net") || null; + return Cookie.parse("a=b; domain=.foonet.net"); }, "parses and is valid": function(c) { assert.ok(c); @@ -354,7 +350,7 @@ vows }, "spaces in value": { topic: function() { - return Cookie.parse("a=one two three", false) || null; + return Cookie.parse("a=one two three", false); }, parsed: function(c) { assert.ok(c); @@ -377,7 +373,7 @@ vows }, "quoted spaces in value": { topic: function() { - return Cookie.parse('a="one two three"', false) || null; + return Cookie.parse('a="one two three"', false); }, parsed: function(c) { assert.ok(c); @@ -400,7 +396,7 @@ vows }, "non-ASCII in value": { topic: function() { - return Cookie.parse("farbe=weiß", false) || null; + return Cookie.parse("farbe=weiß", false); }, parsed: function(c) { assert.ok(c); @@ -423,7 +419,7 @@ vows }, "empty key": { topic: function() { - return Cookie.parse("=abc", { loose: true }) || null; + return Cookie.parse("=abc", { loose: true }); }, parsed: function(c) { assert.ok(c); @@ -446,7 +442,7 @@ vows }, "non-existent key": { topic: function() { - return Cookie.parse("abc", { loose: true }) || null; + return Cookie.parse("abc", { loose: true }); }, parsed: function(c) { assert.ok(c); @@ -469,7 +465,7 @@ vows }, "weird format": { topic: function() { - return Cookie.parse("=foo=bar", { loose: true }) || null; + return Cookie.parse("=foo=bar", { loose: true }); }, parsed: function(c) { assert.ok(c); @@ -494,7 +490,7 @@ vows topic: function() { // takes abnormally long due to semi-catastrophic regexp backtracking const str = `foo=bar${LOTS_OF_SEMICOLONS} domain=example.com`; - return Cookie.parse(str) || null; + return Cookie.parse(str); }, parsed: function(c) { assert.ok(c); @@ -521,9 +517,9 @@ vows const str1 = `x${LOTS_OF_SPACES}x`; const str2 = "x x"; const t0 = Date.now(); - const cookie1 = Cookie.parse(str1) || null; + const cookie1 = Cookie.parse(str1); const t1 = Date.now(); - const cookie2 = Cookie.parse(str2) || null; + const cookie2 = Cookie.parse(str2); const t2 = Date.now(); return { cookie1: cookie1, @@ -533,10 +529,10 @@ vows }; }, "large one doesn't parse": function(c) { - assert.equal(c.cookie1, null); + assert.strictEqual(c.cookie1, undefined); }, "small one doesn't parse": function(c) { - assert.equal(c.cookie2, null); + assert.strictEqual(c.cookie2, undefined); }, "takes about the same time for each": function(c) { const long1 = c.dt1 + 1; // avoid 0ms @@ -551,9 +547,9 @@ vows const str1 = `x${LOTS_OF_SPACES}=x`; const str2 = "x =x"; const t0 = Date.now(); - const cookie1 = Cookie.parse(str1) || null; + const cookie1 = Cookie.parse(str1); const t1 = Date.now(); - const cookie2 = Cookie.parse(str2) || null; + const cookie2 = Cookie.parse(str2); const t2 = Date.now(); return { cookie1: cookie1, @@ -585,9 +581,9 @@ vows const str1 = `x${LOTS_OF_SPACES}x`; const str2 = "x x"; const t0 = Date.now(); - const cookie1 = Cookie.parse(str1, { loose: true }) || null; + const cookie1 = Cookie.parse(str1, { loose: true }); const t1 = Date.now(); - const cookie2 = Cookie.parse(str2, { loose: true }) || null; + const cookie2 = Cookie.parse(str2, { loose: true }); const t2 = Date.now(); return { cookie1: cookie1, @@ -619,9 +615,9 @@ vows const str1 = `x${LOTS_OF_SPACES}=x`; const str2 = "x =x"; const t0 = Date.now(); - const cookie1 = Cookie.parse(str1, { loose: true }) || null; + const cookie1 = Cookie.parse(str1, { loose: true }); const t1 = Date.now(); - const cookie2 = Cookie.parse(str2, { loose: true }) || null; + const cookie2 = Cookie.parse(str2, { loose: true }); const t2 = Date.now(); return { cookie1: cookie1, @@ -651,7 +647,7 @@ vows "same-site": { lax: { topic: function() { - return Cookie.parse("abc=xyzzy; SameSite=Lax") || null; + return Cookie.parse("abc=xyzzy; SameSite=Lax"); }, parsed: function(c) { assert.ok(c); @@ -665,7 +661,7 @@ vows }, strict: { topic: function() { - return Cookie.parse("abc=xyzzy; SameSite=StRiCt") || null; + return Cookie.parse("abc=xyzzy; SameSite=StRiCt"); }, parsed: function(c) { assert.ok(c); @@ -679,7 +675,7 @@ vows }, none: { topic: function() { - return Cookie.parse("abc=xyz; SameSite=NoNe") || null; + return Cookie.parse("abc=xyz; SameSite=NoNe"); }, parsed: function(c) { assert.ok(c); @@ -693,7 +689,7 @@ vows }, bad: { topic: function() { - return Cookie.parse("abc=xyzzy; SameSite=example.com") || null; + return Cookie.parse("abc=xyzzy; SameSite=example.com"); }, parsed: function(c) { assert.ok(c); @@ -707,7 +703,7 @@ vows }, absent: { topic: function() { - return Cookie.parse("abc=xyzzy;") || null; + return Cookie.parse("abc=xyzzy;"); }, parsed: function(c) { assert.ok(c); @@ -722,34 +718,34 @@ vows }, "empty string": { topic: function() { - return Cookie.parse(""); + return Cookie.parse("") ?? SHOULD_BE_UNDEFINED; }, "is empty": function(c) { - assert.isNull(c); + assert.equal(c, SHOULD_BE_UNDEFINED) } }, "missing string": { topic: function() { - return Cookie.parse(); + return Cookie.parse() ?? SHOULD_BE_UNDEFINED; }, "is empty": function(c) { - assert.isNull(c); + assert.equal(c, SHOULD_BE_UNDEFINED); } }, "some string object": { topic: function() { - return Cookie.parse(new String("")); + return Cookie.parse(new String("")) ?? SHOULD_BE_UNDEFINED; }, "is empty": function(c) { - assert.isNull(c, null); + assert.equal(c, SHOULD_BE_UNDEFINED); } }, "some empty string object": { topic: function() { - return Cookie.parse(new String()); + return Cookie.parse(new String()) ?? SHOULD_BE_UNDEFINED; }, "is empty": function(c) { - assert.isNull(c, null); + assert.equal(c, SHOULD_BE_UNDEFINED); } } })