From 35372a34ee7a84c156563bca96f2720158e9e54e Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 20 Mar 2023 16:18:14 +0100 Subject: [PATCH 1/2] ref: Rename `rr_is_password` to `data-rr-is-password` --- packages/rrweb-snapshot/src/snapshot.ts | 2 ++ packages/rrweb-snapshot/src/utils.ts | 18 +++++++++++++++++- packages/rrweb-snapshot/typings/utils.d.ts | 1 + packages/rrweb/src/record/mutation.ts | 4 ++-- packages/rrweb/src/record/observer.ts | 15 ++++++++------- .../__snapshots__/integration.test.ts.snap | 2 +- 6 files changed, 31 insertions(+), 11 deletions(-) diff --git a/packages/rrweb-snapshot/src/snapshot.ts b/packages/rrweb-snapshot/src/snapshot.ts index 3b8cda963f..03233e8432 100644 --- a/packages/rrweb-snapshot/src/snapshot.ts +++ b/packages/rrweb-snapshot/src/snapshot.ts @@ -18,6 +18,7 @@ import { isElement, isShadowRoot, maskInputValue, + getInputType, } from './utils'; let _id = 1; @@ -593,6 +594,7 @@ function serializeNode( | HTMLTextAreaElement | HTMLSelectElement | HTMLOptionElement; + const type = getInputType(el); const value = getInputValue(tagName, el, attributes); const checked = (n as HTMLInputElement).checked; diff --git a/packages/rrweb-snapshot/src/utils.ts b/packages/rrweb-snapshot/src/utils.ts index 6e858e4e18..762d9b3043 100644 --- a/packages/rrweb-snapshot/src/utils.ts +++ b/packages/rrweb-snapshot/src/utils.ts @@ -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,19 @@ 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 | null { + const type = (element as HTMLInputElement).type; + + return element.hasAttribute('data-rr-is-password') + ? 'password' + : type + ? (type.toLowerCase() as Lowercase) + : null; +} diff --git a/packages/rrweb-snapshot/typings/utils.d.ts b/packages/rrweb-snapshot/typings/utils.d.ts index 53b974b31a..fdf2efeb8a 100644 --- a/packages/rrweb-snapshot/typings/utils.d.ts +++ b/packages/rrweb-snapshot/typings/utils.d.ts @@ -18,4 +18,5 @@ 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 | null; export {}; diff --git a/packages/rrweb/src/record/mutation.ts b/packages/rrweb/src/record/mutation.ts index 849fca54a8..463fefe06a 100644 --- 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 index e5f0efb5c1..f1666891c6 100644 --- a/packages/rrweb/src/record/observer.ts +++ b/packages/rrweb/src/record/observer.ts @@ -2,6 +2,7 @@ import { INode, hasInputMaskOptions, maskInputValue, + getInputType, } from '@sentry-internal/rrweb-snapshot'; import { FontFaceSet } from 'css-font-loading-module'; import { @@ -377,10 +378,14 @@ 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; } @@ -388,10 +393,6 @@ function initInputObserver({ let text = (target as HTMLInputElement).value; 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 ( diff --git a/packages/rrweb/test/__snapshots__/integration.test.ts.snap b/packages/rrweb/test/__snapshots__/integration.test.ts.snap index b86ccbd08f..fee207babf 100644 --- a/packages/rrweb/test/__snapshots__/integration.test.ts.snap +++ b/packages/rrweb/test/__snapshots__/integration.test.ts.snap @@ -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" } } ], From c10737323bf1080c06b569895b57443f25eae312 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 20 Mar 2023 17:15:07 +0100 Subject: [PATCH 2/2] ref: Align `getInputValue` code with upstream --- packages/rrweb-snapshot/src/snapshot.ts | 29 ++++--------------- packages/rrweb-snapshot/src/utils.ts | 21 +++++++++++++- packages/rrweb-snapshot/typings/utils.d.ts | 3 +- packages/rrweb/src/record/observer.ts | 23 +++++++++++---- .../__snapshots__/integration.test.ts.snap | 26 ++++++++--------- 5 files changed, 58 insertions(+), 44 deletions(-) diff --git a/packages/rrweb-snapshot/src/snapshot.ts b/packages/rrweb-snapshot/src/snapshot.ts index 03233e8432..27bd957ffe 100644 --- a/packages/rrweb-snapshot/src/snapshot.ts +++ b/packages/rrweb-snapshot/src/snapshot.ts @@ -14,6 +14,7 @@ import { ICanvas, } from './types'; import { + getInputValue, is2DCanvasBlank, isElement, isShadowRoot, @@ -595,17 +596,17 @@ function serializeNode( | HTMLSelectElement | HTMLOptionElement; const type = getInputType(el); - const value = getInputValue(tagName, el, attributes); + const value = getInputValue(el, tagName.toUpperCase() as unknown as Uppercase, 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, @@ -1311,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 index 762d9b3043..f4b722e636 100644 --- 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; @@ -152,3 +152,22 @@ export function getInputType(element: HTMLElement): Lowercase | null { ? (type.toLowerCase() as Lowercase) : null; } + +export function getInputValue( + el: + | HTMLInputElement + | HTMLTextAreaElement + | HTMLSelectElement + | HTMLOptionElement, + tagName: Uppercase, + 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 index fdf2efeb8a..10cbd9f358 100644 --- 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 { @@ -19,4 +19,5 @@ 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 | null; +export declare function getInputValue(el: HTMLInputElement | HTMLTextAreaElement | HTMLSelectElement | HTMLOptionElement, tagName: Uppercase, type: attributes[string]): string; export {}; diff --git a/packages/rrweb/src/record/observer.ts b/packages/rrweb/src/record/observer.ts index f1666891c6..8684a1873e 100644 --- a/packages/rrweb/src/record/observer.ts +++ b/packages/rrweb/src/record/observer.ts @@ -3,6 +3,7 @@ import { hasInputMaskOptions, maskInputValue, getInputType, + getInputValue, } from '@sentry-internal/rrweb-snapshot'; import { FontFaceSet } from 'css-font-loading-module'; import { @@ -362,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); const userTriggered = event.isTrusted; /** @@ -390,12 +392,13 @@ function initInputObserver({ return; } - let text = (target as HTMLInputElement).value; + let text = getInputValue(el, tagName, type); let isChecked = false; if (type === 'radio' || type === 'checkbox') { isChecked = (target as HTMLInputElement).checked; - } else if ( + } + if ( hasInputMaskOptions({ maskInputOptions, maskInputSelector, @@ -404,7 +407,7 @@ function initInputObserver({ }) ) { text = maskInputValue({ - input: target as HTMLElement, + input: el, maskInputOptions, maskInputSelector, unmaskInputSelector, @@ -429,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 index fee207babf..8009a0605a 100644 --- 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 } @@ -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