fix: Fix some input masking & align with upstream (#85)

This brings a few further fixes to input masking (esp. for radio buttons), which we also fixed/are in the process of fixing upstream. Also this aligns some things with how they have been fixed upstream, to make a future merging easier.
getsentry · Apr 28, 2023 · b4261e7 · b4261e7
1 parent 78dcb80
commit b4261e7
Show file tree

Hide file tree

Showing 6 changed files with 89 additions and 55 deletions.
diff --git a/packages/rrweb-snapshot/src/snapshot.ts b/packages/rrweb-snapshot/src/snapshot.ts
@@ -14,10 +14,12 @@ import {
   ICanvas,
 } from './types';
 import {
+  getInputValue,
   is2DCanvasBlank,
   isElement,
   isShadowRoot,
   maskInputValue,
+  getInputType,
 } from './utils';
 
 let _id = 1;
@@ -593,17 +595,18 @@ function serializeNode(
           | HTMLTextAreaElement
           | HTMLSelectElement
           | HTMLOptionElement;
-        const value = getInputValue(tagName, el, attributes);
+        const type = getInputType(el);
+        const value = getInputValue(el, tagName.toUpperCase() as unknown as Uppercase<typeof tagName>, type);
         const checked = (n as HTMLInputElement).checked;
 
         if (
-          attributes.type !== 'submit' &&
-          attributes.type !== 'button' &&
+          type !== 'submit' &&
+          type !== 'button' &&
           value
         ) {
           attributes.value = maskInputValue({
             input: el,
-            type: attributes.type,
+            type,
             tagName,
             value,
             maskInputSelector,
@@ -1309,23 +1312,3 @@ function skipAttribute(
   );
 }
 
-function getInputValue(
-  tagName: string,
-  el:
-    | HTMLInputElement
-    | HTMLTextAreaElement
-    | HTMLSelectElement
-    | HTMLOptionElement,
-  attributes: attributes,
-): string {
-  if (
-    tagName === 'input' &&
-    (attributes.type === 'radio' || attributes.type === 'checkbox')
-  ) {
-    // checkboxes & radio buttons return `on` as their el.value when no value is specified
-    // we only want to get the value if it is specified as `value='xxx'`
-    return el.getAttribute('value') || '';
-  }
-
-  return el.value;
-}
diff --git a/packages/rrweb-snapshot/src/utils.ts b/packages/rrweb-snapshot/src/utils.ts
@@ -1,4 +1,4 @@
-import { INode, MaskInputFn, MaskInputOptions } from './types';
+import { attributes, INode, MaskInputFn, MaskInputOptions } from './types';
 
 export function isElement(n: Node | INode): n is Element {
   return n.nodeType === n.ELEMENT_NODE;
@@ -82,7 +82,7 @@ export function maskInputValue({
     return text;
   }
 
-  if (input.hasAttribute('rr_is_password')) {
+  if (input.hasAttribute('data-rr-is-password')) {
     type = 'password';
   }
 
@@ -136,3 +136,38 @@ export function is2DCanvasBlank(canvas: HTMLCanvasElement): boolean {
   }
   return true;
 }
+
+/**
+ * Get the type of an input element.
+ * This takes care of the case where a password input is changed to a text input.
+ * In this case, we continue to consider this of type password, in order to avoid leaking sensitive data
+ * where passwords should be masked.
+ */
+export function getInputType(element: HTMLElement): Lowercase<string> | null {
+  const type = (element as HTMLInputElement).type;
+
+  return element.hasAttribute('data-rr-is-password')
+    ? 'password'
+    : type
+    ? (type.toLowerCase() as Lowercase<string>)
+    : null;
+}
+
+export function getInputValue(
+  el:
+    | HTMLInputElement
+    | HTMLTextAreaElement
+    | HTMLSelectElement
+    | HTMLOptionElement,
+  tagName: Uppercase<string>,
+  type: attributes[string],
+): string {
+  const normalizedType = typeof type === 'string' ? type.toLowerCase() : '';
+  if (tagName === 'INPUT' && (type === 'radio' || type === 'checkbox')) {
+    // checkboxes & radio buttons return `on` as their el.value when no value is specified
+    // we only want to get the value if it is specified as `value='xxx'`
+    return el.getAttribute('value') || '';
+  }
+
+  return el.value;
+}
diff --git a/packages/rrweb-snapshot/typings/utils.d.ts b/packages/rrweb-snapshot/typings/utils.d.ts
@@ -1,4 +1,4 @@
-import { INode, MaskInputFn, MaskInputOptions } from './types';
+import { attributes, INode, MaskInputFn, MaskInputOptions } from './types';
 export declare function isElement(n: Node | INode): n is Element;
 export declare function isShadowRoot(n: Node): n is ShadowRoot;
 interface IsInputTypeMasked {
@@ -18,4 +18,6 @@ interface MaskInputValue extends HasInputMaskOptions {
 }
 export declare function maskInputValue({ input, maskInputSelector, unmaskInputSelector, maskInputOptions, tagName, type, value, maskInputFn, }: MaskInputValue): string;
 export declare function is2DCanvasBlank(canvas: HTMLCanvasElement): boolean;
+export declare function getInputType(element: HTMLElement): Lowercase<string> | null;
+export declare function getInputValue(el: HTMLInputElement | HTMLTextAreaElement | HTMLSelectElement | HTMLOptionElement, tagName: Uppercase<string>, type: attributes[string]): string;
 export {};
diff --git a/packages/rrweb/src/record/mutation.ts b/packages/rrweb/src/record/mutation.ts
@@ -507,10 +507,10 @@ export default class MutationBuffer {
         // This is used to ensure we do not unmask value when using e.g. a "Show password" type button
         if (
           m.attributeName === 'type' &&
-          (m.target as HTMLElement).tagName === 'INPUT' &&
+          target.tagName === 'INPUT' &&
           (m.oldValue || '').toLowerCase() === 'password'
         ) {
-          (m.target as HTMLElement).setAttribute('rr_is_password', 'true');
+          target.setAttribute('data-rr-is-password', 'true');
         }
 
         if (m.attributeName === 'style') {

diff --git a/packages/rrweb/src/record/observer.ts b/packages/rrweb/src/record/observer.ts
@@ -2,6 +2,8 @@ import {
   INode,
   hasInputMaskOptions,
   maskInputValue,
+  getInputType,
+  getInputValue,
 } from '@sentry-internal/rrweb-snapshot';
 import { FontFaceSet } from 'css-font-loading-module';
 import {
@@ -361,7 +363,8 @@ function initInputObserver({
 }: observerParam): listenerHandler {
   function eventHandler(event: Event) {
     let target = getEventTarget(event);
-    const tagName = target && (target as Element).tagName;
+    const tagName =
+      target && (((target as Element).tagName as unknown) as Uppercase<string>);
 
     const userTriggered = event.isTrusted;
     /**
@@ -377,24 +380,25 @@ function initInputObserver({
     ) {
       return;
     }
-    let type: string | undefined = (target as HTMLInputElement).type;
+    const el = target as
+      | HTMLInputElement
+      | HTMLSelectElement
+      | HTMLTextAreaElement;
+    const type = getInputType(el);
     if (
-      (target as HTMLElement).classList.contains(ignoreClass) ||
-      (ignoreSelector && (target as HTMLElement).matches(ignoreSelector))
+      el.classList.contains(ignoreClass) ||
+      (ignoreSelector && el.matches(ignoreSelector))
     ) {
       return;
     }
 
-    let text = (target as HTMLInputElement).value;
+    let text = getInputValue(el, tagName, type);
     let isChecked = false;
 
-    if ((target as HTMLElement).hasAttribute('rr_is_password')) {
-      type = 'password';
-    }
-
     if (type === 'radio' || type === 'checkbox') {
       isChecked = (target as HTMLInputElement).checked;
-    } else if (
+    }
+    if (
       hasInputMaskOptions({
         maskInputOptions,
         maskInputSelector,
@@ -403,7 +407,7 @@ function initInputObserver({
       })
     ) {
       text = maskInputValue({
-        input: target as HTMLElement,
+        input: el,
         maskInputOptions,
         maskInputSelector,
         unmaskInputSelector,
@@ -428,11 +432,21 @@ function initInputObserver({
         .querySelectorAll(`input[type="radio"][name="${name}"]`)
         .forEach((el) => {
           if (el !== target) {
+            const text = maskInputValue({
+              input: el as HTMLInputElement,
+              maskInputOptions,
+              maskInputSelector,
+              unmaskInputSelector,
+              tagName,
+              type,
+              value: getInputValue(el as HTMLInputElement, tagName, type),
+              maskInputFn,
+            });
             cbWithDedup(
               el,
               callbackWrapper(wrapEventWithUserTriggeredFlag)(
                 {
-                  text: (el as HTMLInputElement).value,
+                  text,
                   isChecked: !isChecked,
                   userTriggered: false,
                 },

diff --git a/packages/rrweb/test/__snapshots__/integration.test.ts.snap b/packages/rrweb/test/__snapshots__/integration.test.ts.snap
@@ -1805,7 +1805,7 @@ exports[`record integration tests can record form interactions 1`] = `
     "type": 3,
     "data": {
       "source": 5,
-      "text": "on",
+      "text": "",
       "isChecked": true,
       "id": 32
     }
@@ -1872,7 +1872,7 @@ exports[`record integration tests can record form interactions 1`] = `
     "type": 3,
     "data": {
       "source": 5,
-      "text": "on",
+      "text": "",
       "isChecked": false,
       "id": 47
     }
@@ -3792,7 +3792,7 @@ exports[`record integration tests can use maskInputOptions to configure which ty
     "type": 3,
     "data": {
       "source": 5,
-      "text": "on",
+      "text": "",
       "isChecked": true,
       "id": 32
     }
@@ -3859,7 +3859,7 @@ exports[`record integration tests can use maskInputOptions to configure which ty
     "type": 3,
     "data": {
       "source": 5,
-      "text": "on",
+      "text": "",
       "isChecked": false,
       "id": 47
     }
@@ -4999,7 +4999,7 @@ exports[`record integration tests should always mask value attribute of password
         {
           "id": 18,
           "attributes": {
-            "rr_is_password": "true"
+            "data-rr-is-password": "true"
           }
         }
       ],
@@ -10223,7 +10223,7 @@ exports[`record integration tests should not record input values if maskAllInput
     "type": 3,
     "data": {
       "source": 5,
-      "text": "on",
+      "text": "",
       "isChecked": true,
       "id": 32
     }
@@ -10232,7 +10232,7 @@ exports[`record integration tests should not record input values if maskAllInput
     "type": 3,
     "data": {
       "source": 5,
-      "text": "radio-on",
+      "text": "********",
       "isChecked": false,
       "id": 37
     }
@@ -10241,7 +10241,7 @@ exports[`record integration tests should not record input values if maskAllInput
     "type": 3,
     "data": {
       "source": 5,
-      "text": "radio-off",
+      "text": "*********",
       "isChecked": false,
       "id": 42
     }
@@ -10290,7 +10290,7 @@ exports[`record integration tests should not record input values if maskAllInput
     "type": 3,
     "data": {
       "source": 5,
-      "text": "on",
+      "text": "",
       "isChecked": false,
       "id": 47
     }
@@ -11180,7 +11180,7 @@ exports[`record integration tests should not record input values on selectively
     "type": 3,
     "data": {
       "source": 5,
-      "text": "on",
+      "text": "**",
       "isChecked": true,
       "id": 27
     }
@@ -11189,7 +11189,7 @@ exports[`record integration tests should not record input values on selectively
     "type": 3,
     "data": {
       "source": 5,
-      "text": "off",
+      "text": "***",
       "isChecked": false,
       "id": 32
     }
@@ -11238,7 +11238,7 @@ exports[`record integration tests should not record input values on selectively
     "type": 3,
     "data": {
       "source": 5,
-      "text": "on",
+      "text": "",
       "isChecked": true,
       "id": 37
     }
@@ -14469,7 +14469,7 @@ exports[`record integration tests should record input userTriggered values if us
     "type": 3,
     "data": {
       "source": 5,
-      "text": "on",
+      "text": "",
       "isChecked": true,
       "userTriggered": true,
       "id": 32
@@ -14539,7 +14539,7 @@ exports[`record integration tests should record input userTriggered values if us
     "type": 3,
     "data": {
       "source": 5,
-      "text": "on",
+      "text": "",
       "isChecked": false,
       "userTriggered": true,
       "id": 47