review fixes (maskAttribue -> fn and try/catch

Author: billyvg

packages/replay/src/integration.ts

@@ -13,6 +13,7 @@ import { ReplayContainer } from './replay';
 import type { RecordingOptions, ReplayConfiguration, ReplayPluginOptions, SendBufferedReplayOptions } from './types';
 import { getPrivacyOptions } from './util/getPrivacyOptions';
 import { isBrowser } from './util/isBrowser';
+import { maskAttribute } from './util/maskAttribute';
 
 const MEDIA_SELECTORS =
   'img,image,svg,video,object,picture,embed,map,audio,link[rel="icon"],link[rel="apple-touch-icon"]';
@@ -124,28 +125,15 @@ export class Replay implements Integration {
       maskInputOptions: { ...(maskInputOptions || {}), password: true },
       maskTextFn: maskFn,
       maskInputFn: maskFn,
-      maskAttributeFn: (key: string, value: string, el: HTMLElement): string => {
-        // We only mask attributes if `maskAllText` is true
-        if (!maskAllText) {
-          return value;
-        }
-
-        // unmaskTextSelector takes precendence
-        if (privacyOptions.unmaskTextSelector && el.matches(privacyOptions.unmaskTextSelector)) {
-          return value;
-        }
-
-        if (
-          maskAttributes.includes(key) ||
-          // Need to mask `value` attribute for `<input>` if it's a button-like
-          // type
-          (key === 'value' && el.tagName === 'INPUT' && ['submit', 'button'].includes(el.getAttribute('type') || ''))
-        ) {
-          return value.replace(/[\S]/g, '*');
-        }
-
-        return value;
-      },
+      maskAttributeFn: (key: string, value: string, el: HTMLElement): string =>
+        maskAttribute({
+          maskAttributes,
+          maskAllText,
+          privacyOptions,
+          key,
+          value,
+          el,
+        }),
 
       ...privacyOptions,
 
@@ -158,8 +146,13 @@ export class Replay implements Integration {
       // origin for playback
       collectFonts: true,
       errorHandler: (err: Error) => {
-        // @ts-ignore Set this so that replay SDK can ignore errors originating from rrweb
-        err.__rrweb__ = true;
+        try {
+          // @ts-ignore Set this so that replay SDK can ignore errors originating from rrweb
+          err.__rrweb__ = true;
+        } catch {
+          // avoid any potential hazards here
+        }
+        // return true to suppress throwing the error inside of rrweb
         return true;
       },
     };

packages/replay/src/util/maskAttribute.ts

@@ -0,0 +1,43 @@
+import type { getPrivacyOptions } from './getPrivacyOptions';
+
+interface MaskAttributeParams {
+  maskAttributes: string[];
+  maskAllText: boolean;
+  privacyOptions: ReturnType<typeof getPrivacyOptions>;
+  key: string;
+  value: string;
+  el: HTMLElement;
+}
+
+/**
+ * Masks an attribute if necessary, otherwise return attribute value as-is.
+ */
+export function maskAttribute({
+  el,
+  key,
+  maskAttributes,
+  maskAllText,
+  privacyOptions,
+  value,
+}: MaskAttributeParams): string {
+  // We only mask attributes if `maskAllText` is true
+  if (!maskAllText) {
+    return value;
+  }
+
+  // unmaskTextSelector takes precendence
+  if (privacyOptions.unmaskTextSelector && el.matches(privacyOptions.unmaskTextSelector)) {
+    return value;
+  }
+
+  if (
+    maskAttributes.includes(key) ||
+    // Need to mask `value` attribute for `<input>` if it's a button-like
+    // type
+    (key === 'value' && el.tagName === 'INPUT' && ['submit', 'button'].includes(el.getAttribute('type') || ''))
+  ) {
+    return value.replace(/[\S]/g, '*');
+  }
+
+  return value;
+}

packages/replay/test/unit/util/maskAttribute.test.ts

@@ -0,0 +1,54 @@
+import { maskAttribute } from '../../../src/util/maskAttribute';
+
+describe('maskAttribute', () => {
+  const defaultEl = document.createElement('div');
+  defaultEl.className = 'classy';
+  const privacyOptions = {
+    maskTextSelector: '',
+    unmaskTextSelector: '.unmask',
+    blockSelector: '',
+    unblockSelector: '',
+    ignoreSelector: '',
+  };
+  const defaultArgs = {
+    el: defaultEl,
+    key: 'title',
+    maskAttributes: ['title'],
+    maskAllText: true,
+    privacyOptions,
+    value: 'foo',
+  };
+
+  const inputSubmit = document.createElement('input');
+  const inputButton = document.createElement('input');
+  [inputSubmit, inputButton].forEach(el => {
+    el.type = 'submit';
+  });
+
+  test.each([
+    ['masks if `maskAllText` is true', defaultArgs, '***'],
+    [
+      'does not mask if `maskAllText` is false, despite `maskTextSelector` ',
+      { ...defaultArgs, maskAllText: false, maskTextSelector: 'classy' },
+      'foo',
+    ],
+    ['does not mask if `maskAllText` is false', { ...defaultArgs, maskAllText: false }, 'foo'],
+    [
+      'does not mask if `unmaskTextSelector` matches',
+      { ...defaultArgs, privacyOptions: { ...privacyOptions, unmaskTextSelector: '.classy' } },
+      'foo',
+    ],
+    [
+      'masks `value` attribute on `<input>` with type "submit"',
+      { ...defaultArgs, el: inputSubmit, value: 'input value' },
+      '***** *****',
+    ],
+    [
+      'masks `value` attribute on `<input>` with type "button"',
+      { ...defaultArgs, el: inputButton, value: 'input value' },
+      '***** *****',
+    ],
+  ])('%s', (_: string, input, output) => {
+    expect(maskAttribute(input)).toEqual(output);
+  });
+});