Skip to content

Commit

Permalink
standardize helpers on returning undefined
Browse files Browse the repository at this point in the history
  • Loading branch information
wjhsf committed Mar 19, 2024
1 parent 4d0b46e commit 4a97015
Show file tree
Hide file tree
Showing 7 changed files with 25 additions and 22 deletions.
8 changes: 4 additions & 4 deletions lib/__tests__/domainMatch.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions lib/cookie/canonicalDomain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ import { IP_V6_REGEX_OBJECT } from './constants'
import type { Nullable } from '../utils'

// S5.1.2 Canonicalized Host Names
export function canonicalDomain(str: Nullable<string>): string | null {
export function canonicalDomain(str: Nullable<string>): string | undefined {
if (str == null) {
return null
return undefined
}
let _str = str.trim().replace(/^\./, '') // S4.1.2.3 & S5.2.3: ignore leading .

Expand Down
14 changes: 8 additions & 6 deletions lib/cookie/cookie.ts
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,10 @@ function parse(str: string, options?: ParseCookieOptions): Cookie | undefined {
return c
}

// TBD: `null` is valid JSON, but `undefined` is not. Options:
// 1. Change this to return `undefined` - weird because it's not JSON
// 2. Keep this as `null` - weird because it's a violation of our new convention
// 3. Change *everything* to return `null` - a lot of work, maybe other edge cases that we can't change?
function fromJSON(str: unknown): Cookie | null {
if (!str || validators.isEmptyString(str)) {
return null
Expand Down Expand Up @@ -532,6 +536,7 @@ export class Cookie {
return obj
}

// TBD: This is a wrapper for `fromJSON`, return type depends on decision there
clone(): Cookie | null {
return fromJSON(this.toJSON())
}
Expand Down Expand Up @@ -697,15 +702,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
Expand Down
6 changes: 3 additions & 3 deletions lib/cookie/cookieJar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,13 +85,13 @@ function getCookieContext(url: unknown): URL | urlParse<string> {
}

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
}
}

Expand Down Expand Up @@ -268,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
Expand Down
6 changes: 3 additions & 3 deletions lib/cookie/domainMatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ export function domainMatch(
str?: Nullable<string>,
domStr?: Nullable<string>,
canonicalize?: boolean,
): boolean | null {
): boolean | undefined {
if (str == null || domStr == null) {
return null
return undefined
}

let _str: Nullable<string>
Expand All @@ -30,7 +30,7 @@ export function domainMatch(
}

if (_str == null || _domStr == null) {
return null
return undefined
}

/*
Expand Down
5 changes: 3 additions & 2 deletions lib/getPublicSuffix.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -84,8 +84,9 @@ export function getPublicSuffix(
)
}

return getDomain(domain, {
const publicSuffix = getDomain(domain, {
allowIcannDomains: true,
allowPrivateDomains: true,
})
if (publicSuffix) return publicSuffix
}
4 changes: 2 additions & 2 deletions lib/permuteDomain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down

0 comments on commit 4a97015

Please sign in to comment.