From 8c77bb6a0b1602ffc7ad802938897cc63a26c67a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Og=C3=B3rek?= Date: Tue, 21 Sep 2021 11:45:37 +0200 Subject: [PATCH] misc: Disallow direct usage of globals (#3999) --- CHANGELOG.md | 4 ++++ packages/angular/src/tracing.ts | 6 +++-- packages/browser/src/helpers.ts | 12 +++++++--- packages/browser/src/transports/base.ts | 6 ++--- packages/eslint-config-sdk/src/index.js | 24 +++++++++++++++++++ .../tracing/src/browser/browsertracing.ts | 2 +- packages/tracing/src/browser/metrics.ts | 2 +- .../web-vitals/lib/getVisibilityWatcher.ts | 4 +++- .../src/browser/web-vitals/lib/onHidden.ts | 4 +++- packages/utils/src/misc.ts | 7 +++--- packages/utils/src/object.ts | 5 ++++ 11 files changed, 61 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eaeb2c410e57..97faa2797819 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott +## 6.13.2 + +- fix(browser): Use getGlobalObject for document check (#3996) + ## 6.13.1 - fix(browser): Check for document when sending outcomes (#3993) diff --git a/packages/angular/src/tracing.ts b/packages/angular/src/tracing.ts index 05da76df22e6..8364b09de5f5 100644 --- a/packages/angular/src/tracing.ts +++ b/packages/angular/src/tracing.ts @@ -2,7 +2,7 @@ import { AfterViewInit, Directive, Injectable, Input, NgModule, OnDestroy, OnIni import { Event, NavigationEnd, NavigationStart, Router } from '@angular/router'; import { getCurrentHub } from '@sentry/browser'; import { Span, Transaction, TransactionContext } from '@sentry/types'; -import { logger, stripUrlQueryAndFragment, timestampWithMs } from '@sentry/utils'; +import { getGlobalObject, logger, stripUrlQueryAndFragment, timestampWithMs } from '@sentry/utils'; import { Observable, Subscription } from 'rxjs'; import { filter, tap } from 'rxjs/operators'; @@ -12,6 +12,8 @@ let instrumentationInitialized: boolean; let stashedStartTransaction: (context: TransactionContext) => Transaction | undefined; let stashedStartTransactionOnLocationChange: boolean; +const global = getGlobalObject(); + /** * Creates routing instrumentation for Angular Router. */ @@ -26,7 +28,7 @@ export function routingInstrumentation( if (startTransactionOnPageLoad) { customStartTransaction({ - name: window.location.pathname, + name: global.location.pathname, op: 'pageload', }); } diff --git a/packages/browser/src/helpers.ts b/packages/browser/src/helpers.ts index 53ca6a23d93d..f1f47cddeb65 100644 --- a/packages/browser/src/helpers.ts +++ b/packages/browser/src/helpers.ts @@ -1,7 +1,8 @@ import { API, captureException, withScope } from '@sentry/core'; import { DsnLike, Event as SentryEvent, Mechanism, Scope, WrappedFunction } from '@sentry/types'; -import { addExceptionMechanism, addExceptionTypeValue, logger } from '@sentry/utils'; +import { addExceptionMechanism, addExceptionTypeValue, getGlobalObject, logger } from '@sentry/utils'; +const global = getGlobalObject(); let ignoreOnError: number = 0; /** @@ -193,16 +194,21 @@ export interface ReportDialogOptions { * @hidden */ export function injectReportDialog(options: ReportDialogOptions = {}): void { + if (!global.document) { + return; + } + if (!options.eventId) { logger.error(`Missing eventId option in showReportDialog call`); return; } + if (!options.dsn) { logger.error(`Missing dsn option in showReportDialog call`); return; } - const script = document.createElement('script'); + const script = global.document.createElement('script'); script.async = true; script.src = new API(options.dsn).getReportDialogEndpoint(options); @@ -211,7 +217,7 @@ export function injectReportDialog(options: ReportDialogOptions = {}): void { script.onload = options.onLoad; } - const injectionPoint = document.head || document.body; + const injectionPoint = global.document.head || global.document.body; if (injectionPoint) { injectionPoint.appendChild(script); diff --git a/packages/browser/src/transports/base.ts b/packages/browser/src/transports/base.ts index a2955f7c5969..eaae72a6e97e 100644 --- a/packages/browser/src/transports/base.ts +++ b/packages/browser/src/transports/base.ts @@ -51,7 +51,7 @@ export abstract class BaseTransport implements Transport { // eslint-disable-next-line deprecation/deprecation this.url = this._api.getStoreEndpointWithUrlEncodedAuth(); - if (this.options.sendClientReports && global && global.document) { + if (this.options.sendClientReports && global.document) { global.document.addEventListener('visibilitychange', () => { if (global.document.visibilityState === 'hidden') { this._flushOutcomes(); @@ -99,7 +99,7 @@ export abstract class BaseTransport implements Transport { return; } - if (!navigator || typeof navigator.sendBeacon !== 'function') { + if (!global.navigator || typeof global.navigator.sendBeacon !== 'function') { logger.warn('Beacon API not available, skipping sending outcomes.'); return; } @@ -134,7 +134,7 @@ export abstract class BaseTransport implements Transport { }); const envelope = `${envelopeHeader}\n${itemHeaders}\n${item}`; - navigator.sendBeacon(url, envelope); + global.navigator.sendBeacon(url, envelope); } /** diff --git a/packages/eslint-config-sdk/src/index.js b/packages/eslint-config-sdk/src/index.js index d9526d4dffcb..2ad423603166 100644 --- a/packages/eslint-config-sdk/src/index.js +++ b/packages/eslint-config-sdk/src/index.js @@ -117,6 +117,30 @@ module.exports = { // Configuration for files under src files: ['src/**/*'], rules: { + 'no-restricted-globals': [ + 'error', + { + name: 'window', + message: + 'Some global variables are not available in environments like WebWorker or Node.js. Use getGlobalObject() instead.', + }, + { + name: 'document', + message: + 'Some global variables are not available in environments like WebWorker or Node.js. Use getGlobalObject() instead.', + }, + { + name: 'location', + message: + 'Some global variables are not available in environments like WebWorker or Node.js. Use getGlobalObject() instead.', + }, + { + name: 'navigator', + message: + 'Some global variables are not available in environments like WebWorker or Node.js. Use getGlobalObject() instead.', + }, + ], + // We want to prevent async await usage in our files to prevent uncessary bundle size. '@sentry-internal/sdk/no-async-await': 'error', diff --git a/packages/tracing/src/browser/browsertracing.ts b/packages/tracing/src/browser/browsertracing.ts index 803fa8934457..31aa00fb735d 100644 --- a/packages/tracing/src/browser/browsertracing.ts +++ b/packages/tracing/src/browser/browsertracing.ts @@ -256,7 +256,7 @@ export function getHeaderContext(): Partial | undefined { /** Returns the value of a meta tag */ export function getMetaContent(metaName: string): string | null { - const el = document.querySelector(`meta[name=${metaName}]`); + const el = getGlobalObject().document.querySelector(`meta[name=${metaName}]`); return el ? el.getAttribute('content') : null; } diff --git a/packages/tracing/src/browser/metrics.ts b/packages/tracing/src/browser/metrics.ts index 702dd34077a5..4d1c9d244bce 100644 --- a/packages/tracing/src/browser/metrics.ts +++ b/packages/tracing/src/browser/metrics.ts @@ -122,7 +122,7 @@ export class MetricsInstrumentation { break; } case 'resource': { - const resourceName = (entry.name as string).replace(window.location.origin, ''); + const resourceName = (entry.name as string).replace(global.location.origin, ''); const endTimestamp = addResourceSpans(transaction, entry, resourceName, startTime, duration, timeOrigin); // We remember the entry script end time to calculate the difference to the first init mark if (entryScriptStartTimestamp === undefined && (entryScriptSrc || '').indexOf(resourceName) > -1) { diff --git a/packages/tracing/src/browser/web-vitals/lib/getVisibilityWatcher.ts b/packages/tracing/src/browser/web-vitals/lib/getVisibilityWatcher.ts index 0ef4f22f3f1b..8769b6d80935 100644 --- a/packages/tracing/src/browser/web-vitals/lib/getVisibilityWatcher.ts +++ b/packages/tracing/src/browser/web-vitals/lib/getVisibilityWatcher.ts @@ -14,12 +14,14 @@ * limitations under the License. */ +import { getGlobalObject } from '@sentry/utils'; + import { onHidden } from './onHidden'; let firstHiddenTime = -1; const initHiddenTime = (): number => { - return document.visibilityState === 'hidden' ? 0 : Infinity; + return getGlobalObject().document.visibilityState === 'hidden' ? 0 : Infinity; }; const trackChanges = (): void => { diff --git a/packages/tracing/src/browser/web-vitals/lib/onHidden.ts b/packages/tracing/src/browser/web-vitals/lib/onHidden.ts index 88b1d4993911..c1a0d4df231f 100644 --- a/packages/tracing/src/browser/web-vitals/lib/onHidden.ts +++ b/packages/tracing/src/browser/web-vitals/lib/onHidden.ts @@ -14,13 +14,15 @@ * limitations under the License. */ +import { getGlobalObject } from '@sentry/utils'; + export interface OnHiddenCallback { (event: Event): void; } export const onHidden = (cb: OnHiddenCallback, once?: boolean): void => { const onHiddenOrPageHide = (event: Event): void => { - if (event.type === 'pagehide' || document.visibilityState === 'hidden') { + if (event.type === 'pagehide' || getGlobalObject().document.visibilityState === 'hidden') { cb(event); if (once) { removeEventListener('visibilitychange', onHiddenOrPageHide, true); diff --git a/packages/utils/src/misc.ts b/packages/utils/src/misc.ts index 7c7a568db51d..92145f671639 100644 --- a/packages/utils/src/misc.ts +++ b/packages/utils/src/misc.ts @@ -31,8 +31,8 @@ const fallbackGlobalObject = {}; export function getGlobalObject(): T & SentryGlobal { return (isNodeEnv() ? global - : typeof window !== 'undefined' - ? window + : typeof window !== 'undefined' // eslint-disable-line no-restricted-globals + ? window // eslint-disable-line no-restricted-globals : typeof self !== 'undefined' ? self : fallbackGlobalObject) as T & SentryGlobal; @@ -227,8 +227,9 @@ export function addExceptionMechanism( * A safe form of location.href */ export function getLocationHref(): string { + const global = getGlobalObject(); try { - return document.location.href; + return global.document.location.href; } catch (oO) { return ''; } diff --git a/packages/utils/src/object.ts b/packages/utils/src/object.ts index c06ea5877ddc..fe78d6c2765c 100644 --- a/packages/utils/src/object.ts +++ b/packages/utils/src/object.ts @@ -224,10 +224,15 @@ function normalizeValue(value: T, key?: any): T | string { return '[Global]'; } + // It's safe to use `window` and `document` here in this manner, as we are asserting using `typeof` first + // which won't throw if they are not present. + + // eslint-disable-next-line no-restricted-globals if (typeof (window as any) !== 'undefined' && (value as unknown) === window) { return '[Window]'; } + // eslint-disable-next-line no-restricted-globals if (typeof (document as any) !== 'undefined' && (value as unknown) === document) { return '[Document]'; }