From aa1ee67d43db8ee6025a47272e7107b78d5fe2bf Mon Sep 17 00:00:00 2001 From: Will Harney <62956339+wjhsf@users.noreply.github.com> Date: Fri, 2 Feb 2024 15:36:52 -0500 Subject: [PATCH] Remove workarounds for `util` in non-node environments (#359) * Always use util.inspect.custom, rather than conditionally. * Always use custom inspect. * remove pointless custom inspect from memstore * only use inspect symbol in cookie * Add test for cookie custom inspect --- lib/__tests__/cookieUtils.spec.ts | 17 +++ lib/__tests__/nodeUtilFallback.spec.ts | 119 ----------------- lib/cookie/cookie.ts | 14 +- lib/memstore.ts | 108 +-------------- lib/utilHelper.ts | 70 ---------- test/node_util_fallback_test.js | 174 ------------------------- 6 files changed, 19 insertions(+), 483 deletions(-) create mode 100644 lib/__tests__/cookieUtils.spec.ts delete mode 100644 lib/__tests__/nodeUtilFallback.spec.ts delete mode 100644 lib/utilHelper.ts delete mode 100644 test/node_util_fallback_test.js diff --git a/lib/__tests__/cookieUtils.spec.ts b/lib/__tests__/cookieUtils.spec.ts new file mode 100644 index 00000000..e1458d26 --- /dev/null +++ b/lib/__tests__/cookieUtils.spec.ts @@ -0,0 +1,17 @@ +import { inspect } from 'node:util' +import { Cookie } from '../cookie/cookie' + +describe('Cookie utils', () => { + describe('custom inspect', () => { + it('should be a readable string', () => { + const cookie = new Cookie({ + key: 'test', + value: 'b', + maxAge: 60, + }) + expect(inspect(cookie)).toBe( + 'Cookie="test=b; Max-Age=60; hostOnly=?; aAge=?; cAge=0ms"', + ) + }) + }) +}) diff --git a/lib/__tests__/nodeUtilFallback.spec.ts b/lib/__tests__/nodeUtilFallback.spec.ts deleted file mode 100644 index 9d3c536c..00000000 --- a/lib/__tests__/nodeUtilFallback.spec.ts +++ /dev/null @@ -1,119 +0,0 @@ -import util from 'util' -import { Cookie } from '../cookie/cookie' -import { CookieJar } from '../cookie/cookieJar' -import { inspectFallback, MemoryCookieStore } from '../memstore' -import { getCustomInspectSymbol, getUtilInspect } from '../utilHelper' - -describe('Node util module fallback for non-node environments', () => { - describe('getCustomInspectSymbol', () => { - it('should not be null in a node environment', () => { - expect(getCustomInspectSymbol()).toEqual( - Symbol.for('nodejs.util.inspect.custom') || util.inspect.custom, - ) - }) - - it('should not be null in a non-node environment since we create the symbol if it does not exist', () => { - expect( - getCustomInspectSymbol({ - requireUtil: () => undefined, - }), - ).toEqual(Symbol.for('nodejs.util.inspect.custom') || util.inspect.custom) - }) - }) - - describe('getUtilInspect', () => { - it('should use util.inspect in a node environment', () => { - const inspect = getUtilInspect(() => 'fallback') - expect(inspect('util.inspect')).toEqual(util.inspect('util.inspect')) - }) - - it('should use fallback inspect function in a non-node environment', () => { - const inspect = getUtilInspect(() => 'fallback', { - requireUtil: () => undefined, - }) - expect(inspect('util.inspect')).toEqual('fallback') - }) - }) - - describe('util usage in Cookie', () => { - it('custom inspect for Cookie still works', () => { - const cookie = Cookie.parse('a=1; Domain=example.com; Path=/') - if (!cookie) { - throw new Error('This should not be undefined') - } - expect(cookie.inspect()).toEqual(util.inspect(cookie)) - }) - }) - - describe('util usage in MemoryCookie', () => { - let memoryStore: MemoryCookieStore - - beforeEach(() => { - memoryStore = new MemoryCookieStore() - }) - - describe('when store is empty', () => { - it('custom inspect for MemoryCookie still works', () => { - expect(memoryStore.inspect()).toEqual(util.inspect(memoryStore)) - }) - - it('fallback produces equivalent output to custom inspect', () => { - expect(util.inspect(memoryStore.idx)).toEqual( - inspectFallback(memoryStore.idx), - ) - }) - }) - - describe('when store has a single cookie', () => { - beforeEach(async () => { - const cookieJar = new CookieJar(memoryStore) - await cookieJar.setCookie( - 'a=1; Domain=example.com; Path=/', - 'http://example.com/index.html', - ) - }) - - it('custom inspect for MemoryCookie still works', () => { - expect(memoryStore.inspect()).toEqual(util.inspect(memoryStore)) - }) - - it('fallback produces equivalent output to custom inspect', () => { - expect(util.inspect(memoryStore.idx)).toEqual( - inspectFallback(memoryStore.idx), - ) - }) - }) - - describe('when store has multiple cookies', () => { - beforeEach(async () => { - const cookieJar = new CookieJar(memoryStore) - const url = 'http://example.com/index.html' - await cookieJar.setCookie('a=0; Domain=example.com; Path=/', url) - await cookieJar.setCookie('b=1; Domain=example.com; Path=/', url) - await cookieJar.setCookie('c=2; Domain=example.com; Path=/', url) - await cookieJar.setCookie( - 'd=3; Domain=example.com; Path=/some-path/', - url, - ) - await cookieJar.setCookie( - 'e=4; Domain=example.com; Path=/some-path/', - url, - ) - await cookieJar.setCookie( - 'f=5; Domain=another.com; Path=/', - 'http://another.com/index.html', - ) - }) - - it('custom inspect for MemoryCookie still works', () => { - expect(memoryStore.inspect()).toEqual(util.inspect(memoryStore)) - }) - - it('fallback produces equivalent output to custom inspect', () => { - expect(util.inspect(memoryStore.idx)).toEqual( - inspectFallback(memoryStore.idx), - ) - }) - }) - }) -}) diff --git a/lib/cookie/cookie.ts b/lib/cookie/cookie.ts index 3929ec58..75aac272 100644 --- a/lib/cookie/cookie.ts +++ b/lib/cookie/cookie.ts @@ -34,7 +34,6 @@ import { getPublicSuffix } from '../getPublicSuffix' import * as validators from '../validators' -import { getCustomInspectSymbol } from '../utilHelper' import { inOperator } from '../utils' import { formatDate } from './formatDate' @@ -421,17 +420,6 @@ export class Cookie { sameSite: string | undefined constructor(options: CreateCookieOptions = {}) { - // supports inspect if that feature is available in the environment - const customInspectSymbol = getCustomInspectSymbol() - if (customInspectSymbol) { - Object.defineProperty(this, customInspectSymbol, { - value: this.inspect.bind(this), - enumerable: false, - writable: false, - configurable: false, - }) - } - Object.assign(this, cookieDefaults, options) this.creation = options.creation ?? cookieDefaults.creation ?? new Date() @@ -444,7 +432,7 @@ export class Cookie { }) } - inspect() { + [Symbol.for('nodejs.util.inspect.custom')]() { const now = Date.now() const hostOnly = this.hostOnly != null ? this.hostOnly.toString() : '?' const createAge = diff --git a/lib/memstore.ts b/lib/memstore.ts index c14be565..b169da7b 100644 --- a/lib/memstore.ts +++ b/lib/memstore.ts @@ -33,13 +33,7 @@ import type { Cookie } from './cookie/cookie' import { pathMatch } from './pathMatch' import { permuteDomain } from './permuteDomain' import { Store } from './store' -import { getCustomInspectSymbol, getUtilInspect } from './utilHelper' -import { - Callback, - createPromiseCallback, - inOperator, - ErrorCallback, -} from './utils' +import { Callback, createPromiseCallback, ErrorCallback } from './utils' export type MemoryCookieStoreIndex = { [domain: string]: { @@ -57,20 +51,6 @@ export class MemoryCookieStore extends Store { super() this.synchronous = true this.idx = Object.create(null) as MemoryCookieStoreIndex - const customInspectSymbol = getCustomInspectSymbol() - if (customInspectSymbol) { - Object.defineProperty(this, customInspectSymbol, { - value: this.inspect.bind(this), - enumerable: false, - writable: false, - configurable: false, - }) - } - } - - inspect() { - const util = { inspect: getUtilInspect(inspectFallback) } - return `{ idx: ${util.inspect(this.idx, false, 2)} }` } override findCookie( @@ -310,89 +290,3 @@ export class MemoryCookieStore extends Store { return promiseCallback.resolve(cookies) } } - -export function inspectFallback(val: unknown): string { - if (typeof val === 'string') { - return `'${val}'` - } - - if (val && typeof val === 'object') { - const domains = Object.keys(val) - if (domains.length === 0) { - return '[Object: null prototype] {}' - } - let result = '[Object: null prototype] {\n' - Object.keys(val).forEach((domain, i) => { - if (inOperator(domain, val)) { - result += formatDomain(domain, val[domain]) - if (i < domains.length - 1) { - result += ',' - } - result += '\n' - } - }) - result += '}' - return result - } - - return String(val) -} - -function formatDomain(domainName: string, domainValue: unknown) { - if (typeof domainValue === 'string') { - return `'${domainValue}'` - } - - if (domainValue && typeof domainValue === 'object') { - const indent = ' ' - let result = `${indent}'${domainName}': [Object: null prototype] {\n` - Object.keys(domainValue).forEach((path, i, paths) => { - if (inOperator(path, domainValue)) { - result += formatPath(path, domainValue[path]) - if (i < paths.length - 1) { - result += ',' - } - result += '\n' - } - }) - result += `${indent}}` - return result - } - - return String(domainValue) -} - -function formatPath(pathName: string, pathValue: unknown) { - if (typeof pathValue === 'string') { - return `'${pathValue}'` - } - - if (pathValue && typeof pathValue === 'object') { - const indent = ' ' - let result = `${indent}'${pathName}': [Object: null prototype] {\n` - Object.keys(pathValue).forEach((cookieName, i, cookieNames) => { - if (inOperator(cookieName, pathValue)) { - const cookie = pathValue[cookieName] - if ( - cookie != null && - typeof cookie === 'object' && - inOperator('inspect', cookie) && - typeof cookie.inspect === 'function' - ) { - const inspectedValue: unknown = cookie.inspect() - if (typeof inspectedValue === 'string') { - result += ` ${cookieName}: ${inspectedValue}` - if (i < cookieNames.length - 1) { - result += ',' - } - result += '\n' - } - } - } - }) - result += `${indent}}` - return result - } - - return String(pathValue) -} diff --git a/lib/utilHelper.ts b/lib/utilHelper.ts deleted file mode 100644 index ead2d745..00000000 --- a/lib/utilHelper.ts +++ /dev/null @@ -1,70 +0,0 @@ -/** - * It would nice to drop this entirely but, for whatever reason, we expose an - * `inspect` method on `Cookie` and `MemoryCookieStore` that just delegates to - * Node's `util.inspect` functionality. Since that functionality isn't supported - * in non-Node environments (e.g.; React Native) this fallback is here to provide - * equivalent behavior when it is not present. - */ - -import type util from 'node:util' - -type RequireUtil = () => typeof util | undefined -type InspectCompatFunction = ( - object: unknown, - showHidden?: boolean, - depth?: number | null, - color?: boolean, -) => string - -function requireUtil(): typeof util | undefined { - try { - // eslint-disable-next-line @typescript-eslint/no-var-requires - return require('util') as typeof util - } catch (e) { - return undefined - } -} - -// for v10.12.0+ -function lookupCustomInspectSymbol(): symbol { - return Symbol.for('nodejs.util.inspect.custom') -} - -// for older node environments -function tryReadingCustomSymbolFromUtilInspect(options: { - requireUtil?: RequireUtil -}): typeof util.inspect.custom | undefined { - const _requireUtil = options.requireUtil || requireUtil - const nodeUtil = _requireUtil() - return nodeUtil ? nodeUtil.inspect.custom : undefined -} - -export function getUtilInspect( - fallback: (value: unknown) => string, - options: { requireUtil?: RequireUtil } = {}, -): InspectCompatFunction { - const _requireUtil = options.requireUtil || requireUtil - const nodeUtil = _requireUtil() - return function inspect( - object: unknown, - showHidden?: boolean, - depth?: number | null, - color?: boolean, - ): string { - return nodeUtil - ? nodeUtil.inspect(object, showHidden, depth, color) - : fallback(object) - } -} - -export function getCustomInspectSymbol( - options: { - requireUtil?: RequireUtil - } = {}, -): symbol { - // get custom inspect symbol for node environments - return ( - tryReadingCustomSymbolFromUtilInspect(options) || - lookupCustomInspectSymbol() - ) -} diff --git a/test/node_util_fallback_test.js b/test/node_util_fallback_test.js deleted file mode 100644 index 7f2a8a57..00000000 --- a/test/node_util_fallback_test.js +++ /dev/null @@ -1,174 +0,0 @@ -/*! - * Copyright (c) 2022, Salesforce.com, Inc. - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are met: - * - * 1. Redistributions of source code must retain the above copyright notice, - * this list of conditions and the following disclaimer. - * - * 2. Redistributions in binary form must reproduce the above copyright notice, - * this list of conditions and the following disclaimer in the documentation - * and/or other materials provided with the distribution. - * - * 3. Neither the name of Salesforce.com nor the names of its contributors may - * be used to endorse or promote products derived from this software without - * specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" - * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE - * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE - * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE - * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR - * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF - * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS - * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN - * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) - * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE - * POSSIBILITY OF SUCH DAMAGE. - */ - -"use strict"; -const vows = require("vows"); -const assert = require("assert"); -const tough = require("../dist/cookie"); -const util = require("util"); -const inspectFallback = require("../dist/memstore").inspectFallback; -const { - getCustomInspectSymbol, - getUtilInspect -} = require("../dist/utilHelper"); -const Cookie = tough.Cookie; -const CookieJar = tough.CookieJar; - -function resetAgeFields(str) { - return str.replace(/\d+ms/g, "0ms"); -} - -vows - .describe("Node util module fallback for non-node environments") - .addBatch({ - getCustomInspectSymbol: { - "should not be null in a node environment": function() { - assert.equal( - getCustomInspectSymbol(), - Symbol.for("nodejs.util.inspect.custom") || util.inspect.custom - ); - }, - "should not be null in a node environment when custom inspect symbol cannot be retrieved (< node v10.12.0)": function() { - assert.equal( - getCustomInspectSymbol({ - lookupCustomInspectSymbol: () => undefined - }), - Symbol.for("nodejs.util.inspect.custom") || util.inspect.custom - ); - } - }, - getUtilInspect: { - "should use util.inspect in a node environment": function() { - const inspect = getUtilInspect(() => "fallback"); - assert.equal(inspect("util.inspect"), util.inspect("util.inspect")); - }, - "should use fallback inspect function in a non-node environment": function() { - const inspect = getUtilInspect(() => "fallback", { - requireUtil: () => undefined - }); - assert.equal(inspect("util.inspect"), "fallback"); - } - }, - "util usage in Cookie": { - "custom inspect for Cookie still works": function() { - const cookie = Cookie.parse("a=1; Domain=example.com; Path=/"); - // The custom inspect uses Date.now(), so the two invocations cannot be directly compared, - // as "cAge" will not necessarily be the same value (sometimes 0ms, sometimes 1ms). - // assert.equal(cookie.inspect(), util.inspect(cookie)); - const expected = /^Cookie="a=1; Domain=example\.com; Path=\/; hostOnly=\?; aAge=\?; cAge=\dms"$/ - assert.match(cookie.inspect(), expected) - assert.match(util.inspect(cookie), expected) - } - }, - "util usage in MemoryCookie": { - "when store is empty": { - topic: function() { - const cookieJar = new CookieJar(); - return cookieJar.store; - }, - "custom inspect for MemoryCookie still works": function(memoryStore) { - assert.equal( - resetAgeFields(util.inspect(memoryStore)), - resetAgeFields(memoryStore.inspect()) - ); - }, - "fallback produces equivalent output to custom inspect": function( - memoryStore - ) { - assert.equal( - resetAgeFields(util.inspect(memoryStore.idx)), - resetAgeFields(inspectFallback(memoryStore.idx)) - ); - } - }, - "when store has a single cookie": { - topic: function() { - const cookieJar = new CookieJar(); - cookieJar.setCookieSync( - "a=1; Domain=example.com; Path=/", - "http://example.com/index.html" - ); - return cookieJar.store; - }, - "custom inspect for MemoryCookie still works": function(memoryStore) { - assert.equal( - resetAgeFields(util.inspect(memoryStore)), - resetAgeFields(memoryStore.inspect()) - ); - }, - "fallback produces equivalent output to custom inspect": function( - memoryStore - ) { - assert.equal( - resetAgeFields(util.inspect(memoryStore.idx)), - resetAgeFields(inspectFallback(memoryStore.idx)) - ); - } - }, - "when store has a multiple cookies": { - topic: function() { - const cookieJar = new CookieJar(); - ["a", "b", "c"].forEach((cookieName, i) => { - cookieJar.setCookieSync( - `${cookieName}=${i}; Domain=example.com; Path=/`, - "http://example.com/index.html" - ); - }); - ["d", "e"].forEach((cookieName, i) => { - cookieJar.setCookieSync( - `${cookieName}=${i}; Domain=example.com; Path=/some-path/`, - "http://example.com/index.html" - ); - }); - cookieJar.setCookieSync( - `f=0; Domain=another.com; Path=/`, - "http://another.com/index.html" - ); - return cookieJar.store; - }, - "custom inspect for MemoryCookie still works": function(memoryStore) { - assert.equal( - resetAgeFields(util.inspect(memoryStore)), - resetAgeFields(memoryStore.inspect()) - ); - }, - "fallback produces equivalent output to custom inspect": function( - memoryStore - ) { - assert.equal( - resetAgeFields(util.inspect(memoryStore.idx)), - resetAgeFields(inspectFallback(memoryStore.idx)) - ); - } - } - } - }) - .export(module);