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

Clean up cookie creation #381

Merged
merged 4 commits into from
Mar 14, 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
6 changes: 3 additions & 3 deletions lib/__tests__/cookieJar.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@
expect(allHaveRootPath).toBe(true)

const noCookiesWithAnOtherKeyRetrieved = cookies.every(
(cookie) => !/^other/.test(cookie.key as string),
(cookie) => !/^other/.test(cookie.key),
)
expect(noCookiesWithAnOtherKeyRetrieved).toBe(true)
})
Expand All @@ -430,7 +430,7 @@
expect(cookies).toHaveLength(4)

const noCookiesWithAnOtherKeyRetrieved = cookies.every(
(cookie) => !/^other/.test(cookie.key as string),
(cookie) => !/^other/.test(cookie.key),
)
expect(noCookiesWithAnOtherKeyRetrieved).toBe(true)
})
Expand All @@ -442,7 +442,7 @@
expect(cookies).toHaveLength(4)

const noCookiesWithAnOtherKeyRetrieved = cookies.every(
(cookie) => !/^other/.test(cookie.key as string),
(cookie) => !/^other/.test(cookie.key),
)
expect(noCookiesWithAnOtherKeyRetrieved).toBe(true)
})
Expand Down Expand Up @@ -498,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
8 changes: 2 additions & 6 deletions lib/__tests__/cookieSorting.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@ describe('Cookie sorting', () => {
const cookie2 = new Cookie()
expect(typeof cookie1.creationIndex).toBe('number')
expect(typeof cookie2.creationIndex).toBe('number')
expect(cookie1.creationIndex).toBeLessThan(
cookie2.creationIndex as number,
)
expect(cookie1.creationIndex).toBeLessThan(cookie2.creationIndex)
})

it('should set the creation index during construction when creation time is provided', () => {
Expand All @@ -21,9 +19,7 @@ describe('Cookie sorting', () => {
expect(cookie1.creation).toEqual(cookie2.creation)
expect(typeof cookie1.creationIndex).toBe('number')
expect(typeof cookie2.creationIndex).toBe('number')
expect(cookie1.creationIndex).toBeLessThan(
cookie2.creationIndex as number,
)
expect(cookie1.creationIndex).toBeLessThan(cookie2.creationIndex)
})

it('should leave the creation index alone during setCookie', async () => {
Expand Down
77 changes: 43 additions & 34 deletions lib/cookie/cookie.ts
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,17 @@ function fromJSON(str: unknown) {
return c
}

type CreateCookieOptions = Omit<
{
// Assume that all non-method attributes on the class can be configured, except creationIndex.
// eslint-disable-next-line @typescript-eslint/no-explicit-any
[K in keyof Cookie as Cookie[K] extends (...args: any[]) => any
? never
: K]?: Cookie[K]
},
'creationIndex'
>

const cookieDefaults = {
// the order in which the RFC has them:
key: '',
Expand All @@ -382,46 +393,42 @@ const cookieDefaults = {
creation: null,
lastAccessed: null,
sameSite: undefined,
}

type CreateCookieOptions = {
key?: string
value?: string
expires?: Date | 'Infinity' | null
maxAge?: number | 'Infinity' | '-Infinity'
domain?: string | null
path?: string | null
secure?: boolean
httpOnly?: boolean
extensions?: string[] | null
creation?: Date | 'Infinity' | null
creationIndex?: number
hostOnly?: boolean | null
pathIsDefault?: boolean | null
lastAccessed?: Date | 'Infinity' | null
sameSite?: string | undefined
}
} as const satisfies Required<CreateCookieOptions>
wjhsf marked this conversation as resolved.
Show resolved Hide resolved

export class Cookie {
key: string | undefined
value: string | undefined
expires: Date | 'Infinity' | null | undefined
maxAge: number | 'Infinity' | '-Infinity' | undefined
domain: string | null | undefined
path: string | null | undefined
secure: boolean | undefined
httpOnly: boolean | undefined
extensions: string[] | null | undefined
key: string
value: string
expires: Date | 'Infinity' | null
maxAge: number | 'Infinity' | '-Infinity' | null
domain: string | null
path: string | null
secure: boolean
httpOnly: boolean
extensions: string[] | null
creation: Date | 'Infinity' | null
creationIndex: number | undefined
hostOnly: boolean | null | undefined
pathIsDefault: boolean | null | undefined
lastAccessed: Date | 'Infinity' | null | undefined
creationIndex: number
hostOnly: boolean | null
pathIsDefault: boolean | null
lastAccessed: Date | 'Infinity' | null
sameSite: string | undefined
wjhsf marked this conversation as resolved.
Show resolved Hide resolved

constructor(options: CreateCookieOptions = {}) {
Object.assign(this, cookieDefaults, options)
this.creation = options.creation ?? cookieDefaults.creation ?? new Date()
this.key = options.key ?? cookieDefaults.key
this.value = options.value ?? cookieDefaults.value
this.expires = options.expires ?? cookieDefaults.expires
this.maxAge = options.maxAge ?? cookieDefaults.maxAge
this.domain = options.domain ?? cookieDefaults.domain
this.path = options.path ?? cookieDefaults.path
this.secure = options.secure ?? cookieDefaults.secure
this.httpOnly = options.httpOnly ?? cookieDefaults.httpOnly
this.extensions = options.extensions ?? cookieDefaults.extensions
this.creation = options.creation ?? cookieDefaults.creation
this.hostOnly = options.hostOnly ?? cookieDefaults.hostOnly
this.pathIsDefault = options.pathIsDefault ?? cookieDefaults.pathIsDefault
this.lastAccessed = options.lastAccessed ?? cookieDefaults.lastAccessed
this.sameSite = options.sameSite ?? cookieDefaults.sameSite

this.creation = options.creation ?? new Date()

// used to break creation ties in cookieCompare():
Object.defineProperty(this, 'creationIndex', {
Expand All @@ -430,6 +437,8 @@ export class Cookie {
writable: true,
value: ++Cookie.cookiesCreated,
})
// Duplicate operation, but it makes TypeScript happy...
this.creationIndex = Cookie.cookiesCreated
}

[Symbol.for('nodejs.util.inspect.custom')]() {
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,8 @@
"vows": "^0.8.3"
},
"dependencies": {
"tldts": "^6.1.0",
"punycode": "^2.3.1",
"tldts": "^6.1.0",
"url-parse": "^1.5.10"
}
}
Loading