fix(node): Guard against invalid maxSpanWaitDuration values

Author: mydea

packages/node/src/sdk/initOtel.ts

@@ -10,6 +10,7 @@ import {
 import { GLOBAL_OBJ, SDK_VERSION, consoleSandbox, logger } from '@sentry/core';
 import { SentryPropagator, SentrySampler, SentrySpanProcessor } from '@sentry/opentelemetry';
 import { createAddHookMessageChannel } from 'import-in-the-middle';
+import { DEBUG_BUILD } from '../debug-build';
 import { getOpenTelemetryInstrumentationToPreload } from '../integrations/tracing';
 import { SentryContextManager } from '../otel/contextManager';
 import type { EsmLoaderHookOptions } from '../types';
@@ -18,6 +19,9 @@ import type { NodeClient } from './client';
 
 declare const __IMPORT_META_URL_REPLACEMENT__: string;
 
+// About 277h - this must fit into new Array(len)!
+const MAX_MAX_SPAN_WAIT_DURATION = 1_000_000;
+
 /**
  * Initialize OpenTelemetry for Node.
  */
@@ -138,7 +142,7 @@ export function setupOtel(client: NodeClient): BasicTracerProvider {
     forceFlushTimeoutMillis: 500,
     spanProcessors: [
       new SentrySpanProcessor({
-        timeout: client.getOptions().maxSpanWaitDuration,
+        timeout: _getSpanProcessorTimeout(client.getOptions().maxSpanWaitDuration),
       }),
     ],
   });
@@ -152,6 +156,26 @@ export function setupOtel(client: NodeClient): BasicTracerProvider {
   return provider;
 }
 
+/** Just exported for tests. */
+export function _getSpanProcessorTimeout(maxSpanWaitDuration: number | undefined): number | undefined {
+  if (maxSpanWaitDuration == null) {
+    return undefined;
+  }
+
+  // We guard for a max. value here, because we create an array with this length
+  // So if this value is too large, this would fail
+  if (maxSpanWaitDuration > MAX_MAX_SPAN_WAIT_DURATION) {
+    DEBUG_BUILD &&
+      logger.warn(`\`maxSpanWaitDuration\` is too high, using the maximum value of ${MAX_MAX_SPAN_WAIT_DURATION}`);
+    return MAX_MAX_SPAN_WAIT_DURATION;
+  } else if (maxSpanWaitDuration <= 0 || Number.isNaN(maxSpanWaitDuration)) {
+    DEBUG_BUILD && logger.warn('`maxSpanWaitDuration` must be a positive number, using default value instead.');
+    return undefined;
+  }
+
+  return maxSpanWaitDuration;
+}
+
 /**
  * Setup the OTEL logger to use our own logger.
  */

packages/node/test/sdk/initOtel.test.ts

@@ -0,0 +1,78 @@
+import { logger } from '@sentry/core';
+import { _getSpanProcessorTimeout } from '../../src/sdk/initOtel';
+
+describe('_getSpanProcessorTimeout', () => {
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it('works with undefined', () => {
+    const loggerWarnSpy = jest.spyOn(logger, 'warn').mockImplementation(() => {});
+    const timeout = _getSpanProcessorTimeout(undefined);
+    expect(timeout).toBe(undefined);
+    expect(loggerWarnSpy).not.toHaveBeenCalled();
+  });
+
+  it('works with positive number', () => {
+    const loggerWarnSpy = jest.spyOn(logger, 'warn').mockImplementation(() => {});
+    const timeout = _getSpanProcessorTimeout(10);
+    expect(timeout).toBe(10);
+    expect(loggerWarnSpy).not.toHaveBeenCalled();
+  });
+
+  it('works with 0', () => {
+    const loggerWarnSpy = jest.spyOn(logger, 'warn').mockImplementation(() => {});
+    const timeout = _getSpanProcessorTimeout(0);
+    expect(timeout).toBe(undefined);
+    expect(loggerWarnSpy).toHaveBeenCalledTimes(1);
+    expect(loggerWarnSpy).toHaveBeenCalledWith(
+      '`maxSpanWaitDuration` must be a positive number, using default value instead.',
+    );
+  });
+
+  it('works with negative number', () => {
+    const loggerWarnSpy = jest.spyOn(logger, 'warn').mockImplementation(() => {});
+    const timeout = _getSpanProcessorTimeout(-10);
+    expect(timeout).toBe(undefined);
+    expect(loggerWarnSpy).toHaveBeenCalledTimes(1);
+    expect(loggerWarnSpy).toHaveBeenCalledWith(
+      '`maxSpanWaitDuration` must be a positive number, using default value instead.',
+    );
+  });
+
+  it('works with -Infinity', () => {
+    const loggerWarnSpy = jest.spyOn(logger, 'warn').mockImplementation(() => {});
+    const timeout = _getSpanProcessorTimeout(-Infinity);
+    expect(timeout).toBe(undefined);
+    expect(loggerWarnSpy).toHaveBeenCalledTimes(1);
+    expect(loggerWarnSpy).toHaveBeenCalledWith(
+      '`maxSpanWaitDuration` must be a positive number, using default value instead.',
+    );
+  });
+
+  it('works with Infinity', () => {
+    const loggerWarnSpy = jest.spyOn(logger, 'warn').mockImplementation(() => {});
+    const timeout = _getSpanProcessorTimeout(Infinity);
+    expect(timeout).toBe(1_000_000);
+    expect(loggerWarnSpy).toHaveBeenCalledTimes(1);
+    expect(loggerWarnSpy).toHaveBeenCalledWith('`maxSpanWaitDuration` is too high, using the maximum value of 1000000');
+  });
+
+  it('works with large number', () => {
+    const loggerWarnSpy = jest.spyOn(logger, 'warn').mockImplementation(() => {});
+    const timeout = _getSpanProcessorTimeout(1_000_000_000);
+    expect(timeout).toBe(1_000_000);
+    expect(loggerWarnSpy).toHaveBeenCalledTimes(1);
+    expect(loggerWarnSpy).toHaveBeenCalledWith('`maxSpanWaitDuration` is too high, using the maximum value of 1000000');
+  });
+
+  it('works with NaN', () => {
+    const loggerWarnSpy = jest.spyOn(logger, 'warn').mockImplementation(() => {});
+    const timeout = _getSpanProcessorTimeout(NaN);
+    expect(timeout).toBe(undefined);
+    expect(loggerWarnSpy).toHaveBeenCalledTimes(1);
+    expect(loggerWarnSpy).toHaveBeenCalledWith(
+      '`maxSpanWaitDuration` must be a positive number, using default value instead.',
+    );
+  });
+});