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

misc: Disallow direct usage of globals #3999

Merged
merged 1 commit into from Sep 21, 2021
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Expand Up @@ -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)
Expand Down
6 changes: 4 additions & 2 deletions packages/angular/src/tracing.ts
Expand Up @@ -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';

Expand All @@ -12,6 +12,8 @@ let instrumentationInitialized: boolean;
let stashedStartTransaction: (context: TransactionContext) => Transaction | undefined;
let stashedStartTransactionOnLocationChange: boolean;

const global = getGlobalObject<Window>();

/**
* Creates routing instrumentation for Angular Router.
*/
Expand All @@ -26,7 +28,7 @@ export function routingInstrumentation(

if (startTransactionOnPageLoad) {
customStartTransaction({
name: window.location.pathname,
name: global.location.pathname,
op: 'pageload',
});
}
Expand Down
12 changes: 9 additions & 3 deletions 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<Window>();
let ignoreOnError: number = 0;

/**
Expand Down Expand Up @@ -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);

Expand All @@ -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);
Expand Down
6 changes: 3 additions & 3 deletions packages/browser/src/transports/base.ts
Expand Up @@ -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();
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
}

/**
Expand Down
24 changes: 24 additions & 0 deletions packages/eslint-config-sdk/src/index.js
Expand Up @@ -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',

Expand Down
2 changes: 1 addition & 1 deletion packages/tracing/src/browser/browsertracing.ts
Expand Up @@ -256,7 +256,7 @@ export function getHeaderContext(): Partial<TransactionContext> | 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<Window>().document.querySelector(`meta[name=${metaName}]`);
return el ? el.getAttribute('content') : null;
}

Expand Down
2 changes: 1 addition & 1 deletion packages/tracing/src/browser/metrics.ts
Expand Up @@ -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) {
Expand Down
Expand Up @@ -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<Window>().document.visibilityState === 'hidden' ? 0 : Infinity;
};

const trackChanges = (): void => {
Expand Down
4 changes: 3 additions & 1 deletion packages/tracing/src/browser/web-vitals/lib/onHidden.ts
Expand Up @@ -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<Window>().document.visibilityState === 'hidden') {
cb(event);
if (once) {
removeEventListener('visibilitychange', onHiddenOrPageHide, true);
Expand Down
7 changes: 4 additions & 3 deletions packages/utils/src/misc.ts
Expand Up @@ -31,8 +31,8 @@ const fallbackGlobalObject = {};
export function getGlobalObject<T>(): 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;
Expand Down Expand Up @@ -227,8 +227,9 @@ export function addExceptionMechanism(
* A safe form of location.href
*/
export function getLocationHref(): string {
const global = getGlobalObject<Window>();
try {
return document.location.href;
return global.document.location.href;
} catch (oO) {
return '';
}
Expand Down
5 changes: 5 additions & 0 deletions packages/utils/src/object.ts
Expand Up @@ -224,10 +224,15 @@ function normalizeValue<T>(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]';
}
Expand Down