diff --git a/packages/core/src/integrations/functiontostring.ts b/packages/core/src/integrations/functiontostring.ts index 2f3a8ce295d3..f206051512ac 100644 --- a/packages/core/src/integrations/functiontostring.ts +++ b/packages/core/src/integrations/functiontostring.ts @@ -19,22 +19,29 @@ const _functionToStringIntegration = (() => { const callOriginalFunctionToString = Function.prototype.call.bind(originalFunctionToString) as ( thisArg: unknown, ) => string; + // eslint-disable-next-line @typescript-eslint/unbound-method + const hasOwnProperty = Function.prototype.call.bind(Object.prototype.hasOwnProperty) as ( + obj: unknown, + key: PropertyKey, + ) => boolean; // intrinsics (like Function.prototype) might be immutable in some environments // e.g. Node with --frozen-intrinsics, XS (an embedded JavaScript engine) or SES (a JavaScript proposal) try { Function.prototype.toString = function (this: WrappedFunction): string { - const originalFunction = getOriginalFunction(this); let unwrappedFunction: WrappedFunction | undefined; try { - if (SETUP_CLIENTS.has(getClient() as Client) && originalFunction !== undefined) { - unwrappedFunction = originalFunction; + // `markFunctionWrapped` sets the original as an own property. An inherited lookup could walk a + // prototype chain that loops through a Proxy and overflow the stack, which native `toString` never does. + if (SETUP_CLIENTS.has(getClient() as Client) && hasOwnProperty(this, '__sentry_original__')) { + unwrappedFunction = getOriginalFunction(this); } } catch { // Reading the Sentry carrier off `getClient()` can throw a `SecurityError` when `this` (or the global - // object) is a `WindowProxy` whose browsing context was navigated cross-origin. The native - // `toString` never throws here, so fall back to it to avoid turning harmless introspection into noise. + // object) is a `WindowProxy` whose browsing context was navigated cross-origin, and Proxy traps on + // `this` can throw too. The native `toString` never throws here, so fall back to it to avoid turning + // harmless introspection into noise. } return callOriginalFunctionToString(unwrappedFunction ?? this); diff --git a/packages/core/test/lib/integrations/functiontostring.test.ts b/packages/core/test/lib/integrations/functiontostring.test.ts index e42c83a77611..d47103b9132f 100644 --- a/packages/core/test/lib/integrations/functiontostring.test.ts +++ b/packages/core/test/lib/integrations/functiontostring.test.ts @@ -120,6 +120,51 @@ describe('FunctionToString', () => { expect(Reflect.apply(Function.prototype.toString, inspectedFunction, new Array(100_000))).toBe(expected); }); + it('does not overflow the stack when the prototype chain loops back through a Proxy', () => { + function inspectedFunction(): void {} + const expected = inspectedFunction.toString(); + Object.setPrototypeOf(inspectedFunction, Object.create(new Proxy(inspectedFunction, {}))); + const foo = { + bar(): void {}, + }; + const originalBar = foo.bar.toString(); + fill(foo, 'bar', () => function wrapped(): void {}); + + const fts = functionToStringIntegration(); + getClient()?.addIntegration(fts); + + expect(Function.prototype.toString.call(inspectedFunction)).toBe(expected); + expect(foo.bar.toString()).toBe(originalBar); + }); + + it('does not unwrap a function that only inherits the original from a wrapped function', () => { + const foo = { + bar(): void {}, + }; + fill(foo, 'bar', () => function wrapped(): void {}); + function inheriting(): void {} + const expected = inheriting.toString(); + Object.setPrototypeOf(inheriting, foo.bar); + + const fts = functionToStringIntegration(); + getClient()?.addIntegration(fts); + + expect(Function.prototype.toString.call(inheriting)).toBe(expected); + }); + + it('falls back to native toString when the original function lookup throws', () => { + function inspectedFunction(): void {} + const throwingTrap = (): never => { + throw new Error('trap'); + }; + const proxy = new Proxy(inspectedFunction, { get: throwingTrap, getOwnPropertyDescriptor: throwingTrap }); + + const fts = functionToStringIntegration(); + getClient()?.addIntegration(fts); + + expect(Function.prototype.toString.call(proxy)).toBe('function () { [native code] }'); + }); + it('falls back to native toString and does not throw when the carrier read throws', () => { const foo = { bar(wat: boolean): boolean { diff --git a/packages/server-utils/src/integrations/hono/patchAppRequest.ts b/packages/server-utils/src/integrations/hono/patchAppRequest.ts index 15bb20cc21fd..d77dc4ef1d8a 100644 --- a/packages/server-utils/src/integrations/hono/patchAppRequest.ts +++ b/packages/server-utils/src/integrations/hono/patchAppRequest.ts @@ -4,6 +4,7 @@ import { debug, getActiveSpan, getOriginalFunction, startSpan, type WrappedFunct import { INTERNAL_REQUEST_ORIGIN } from './constants'; import type { Env, Hono } from './honoTypes'; import { DEBUG_BUILD } from '../../debug-build'; +import { sentryOriginalTraps } from './sentryOriginalTraps'; const INTERNAL_REQUEST_OP = HTTP_SERVER; @@ -113,12 +114,7 @@ export function patchAppRequest(app: Hono): void { }, ); }, - get(target, prop, receiver) { - if (prop === '__sentry_original__') { - return originalRequest; - } - return Reflect.get(target, prop, receiver); - }, + ...sentryOriginalTraps(originalRequest), }); DEBUG_BUILD && debug.log('[hono] Patched app.request for internal dispatch tracing.'); diff --git a/packages/server-utils/src/integrations/hono/patchRoute.ts b/packages/server-utils/src/integrations/hono/patchRoute.ts index 398df72fb924..b63add8f582a 100644 --- a/packages/server-utils/src/integrations/hono/patchRoute.ts +++ b/packages/server-utils/src/integrations/hono/patchRoute.ts @@ -5,6 +5,7 @@ import type { Hono, HonoRoute } from './honoTypes'; import { getMiddlewareFlags } from './isMiddleware'; import { patchAppRequest } from './patchAppRequest'; import { wrapMiddlewareWithSpan } from './wrapMiddlewareSpan'; +import { sentryOriginalTraps } from './sentryOriginalTraps'; export type { HonoRoute }; @@ -88,12 +89,7 @@ export function installRouteHookOnPrototype(honoBaseProto: HonoBaseProto): Route return Reflect.apply(_target, thisArg, args); }, - get(target, prop, receiver) { - if (prop === '__sentry_original__') { - return originalRoute; - } - return Reflect.get(target, prop, receiver); - }, + ...sentryOriginalTraps(originalRoute), }); honoBaseProto.__sentryRouteHook__ = handle; diff --git a/packages/server-utils/src/integrations/hono/sentryOriginalTraps.ts b/packages/server-utils/src/integrations/hono/sentryOriginalTraps.ts new file mode 100644 index 000000000000..d623a0aa3819 --- /dev/null +++ b/packages/server-utils/src/integrations/hono/sentryOriginalTraps.ts @@ -0,0 +1,26 @@ +// oxlint-disable-next-line typescript/no-unsafe-function-type +type AnyFunction = Function; + +/** + * Proxy traps that expose `original` as `__sentry_original__` on a wrapping Proxy without defining it on the target, + * which is the original itself. `functionToStringIntegration` only unwraps own properties, so the property has to + * look like an own one too. + */ +export function sentryOriginalTraps( + original: AnyFunction, +): Pick, 'get' | 'getOwnPropertyDescriptor'> { + return { + get(target, prop, receiver) { + if (prop === '__sentry_original__') { + return original; + } + return Reflect.get(target, prop, receiver); + }, + getOwnPropertyDescriptor(target, prop) { + if (prop === '__sentry_original__') { + return { value: original, writable: true, enumerable: false, configurable: true }; + } + return Reflect.getOwnPropertyDescriptor(target, prop); + }, + }; +} diff --git a/packages/server-utils/src/integrations/hono/wrapMiddlewareSpan.ts b/packages/server-utils/src/integrations/hono/wrapMiddlewareSpan.ts index 86f96e05039c..10ccdb727ac6 100644 --- a/packages/server-utils/src/integrations/hono/wrapMiddlewareSpan.ts +++ b/packages/server-utils/src/integrations/hono/wrapMiddlewareSpan.ts @@ -11,6 +11,7 @@ import { import { type MiddlewareHandler } from './honoTypes'; import { SENTRY_HONO_MIDDLEWARE } from './createHonoMiddleware'; import { defaultShouldHandleError } from './defaultShouldHandleError'; +import { sentryOriginalTraps } from './sentryOriginalTraps'; const MIDDLEWARE_ORIGIN = 'auto.middleware.hono'; @@ -59,11 +60,6 @@ export function wrapMiddlewareWithSpan(handler: MiddlewareHandler): MiddlewareHa span.end(); } }, - get(target, prop, receiver) { - if (prop === '__sentry_original__') { - return handler; - } - return Reflect.get(target, prop, receiver); - }, + ...sentryOriginalTraps(handler), }); } diff --git a/packages/server-utils/test/integrations/hono/sentryOriginalTraps.test.ts b/packages/server-utils/test/integrations/hono/sentryOriginalTraps.test.ts new file mode 100644 index 000000000000..183a7820e133 --- /dev/null +++ b/packages/server-utils/test/integrations/hono/sentryOriginalTraps.test.ts @@ -0,0 +1,24 @@ +import { functionToStringIntegration, getCurrentScope, setCurrentClient } from '@sentry/core'; +import { afterAll, describe, expect, it } from 'vitest'; +import { wrapMiddlewareWithSpan } from '../../../src/integrations/hono/wrapMiddlewareSpan'; +import { getDefaultTestClientOptions, TestClient } from '../../mocks/client'; + +describe('sentryOriginalTraps', () => { + afterAll(() => { + getCurrentScope().setClient(undefined); + }); + + it('lets functionToStringIntegration unwrap a wrapped Hono middleware', async () => { + const client = new TestClient(getDefaultTestClientOptions()); + setCurrentClient(client); + client.addIntegration(functionToStringIntegration()); + + const middleware = async (_c: unknown, next: () => Promise): Promise => { + await next(); + }; + const wrapped = wrapMiddlewareWithSpan(middleware); + + expect(wrapped).not.toBe(middleware); + expect(wrapped.toString()).toBe(middleware.toString()); + }); +});