From 3d533c1bd05b2bb16a89609a8961e951a0a2d6a3 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 27 Feb 2023 09:20:55 +0100 Subject: [PATCH 1/2] fix(snapshot): Handle undefined in `defaultMaskFn` --- packages/rrweb-snapshot/src/snapshot.ts | 31 +++++++++++++++++++------ 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/packages/rrweb-snapshot/src/snapshot.ts b/packages/rrweb-snapshot/src/snapshot.ts index e60c96fab6..7955edff83 100644 --- a/packages/rrweb-snapshot/src/snapshot.ts +++ b/packages/rrweb-snapshot/src/snapshot.ts @@ -25,8 +25,8 @@ const tagNameRegex = new RegExp('[^a-z0-9-_:]'); export const IGNORED_NODE = -2; -function defaultMaskFn(str: string) { - return str.replace(/[\S]/g, '*'); +function defaultMaskFn(str: string | undefined) { + return str ? str.replace(/[\S]/g, '*') : ''; } function genId(): number { @@ -260,7 +260,10 @@ export function transformAttribute( return absoluteToStylesheet(value, getHref()); } else if (tagName === 'object' && name === 'data' && value) { return absoluteToDoc(doc, value); - } else if (maskAllText && ['placeholder', 'title', 'aria-label'].indexOf(name) > -1) { + } else if ( + maskAllText && + ['placeholder', 'title', 'aria-label'].indexOf(name) > -1 + ) { return maskTextFn ? maskTextFn(value) : defaultMaskFn(value); } else { return value; @@ -501,7 +504,14 @@ function serializeNode( let attributes: attributes = {}; for (const { name, value } of Array.from((n as HTMLElement).attributes)) { if (!skipAttribute(tagName, name, value)) { - attributes[name] = transformAttribute(doc, tagName, name, value, maskAllText, maskTextFn); + attributes[name] = transformAttribute( + doc, + tagName, + name, + value, + maskAllText, + maskTextFn, + ); } } // remote css @@ -782,7 +792,8 @@ function slimDOMExcluded( (sn.tagName === 'script' || // (module)preload link (sn.tagName === 'link' && - (sn.attributes.rel === 'preload' || sn.attributes.rel === 'modulepreload') && + (sn.attributes.rel === 'preload' || + sn.attributes.rel === 'modulepreload') && sn.attributes.as === 'script') || // prefetch link (sn.tagName === 'link' && @@ -1236,6 +1247,12 @@ export function cleanupSnapshot() { export default snapshot; /** We want to skip `autoplay` attribute, as this has weird results when replaying. */ -function skipAttribute(tagName: string, attributeName: string, value?: unknown) { - return (tagName === 'video' || tagName === 'audio') && attributeName === 'autoplay'; +function skipAttribute( + tagName: string, + attributeName: string, + value?: unknown, +) { + return ( + (tagName === 'video' || tagName === 'audio') && attributeName === 'autoplay' + ); } From f40f0cf619849ebe1b8f79215b9dc192471e14b9 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 27 Feb 2023 10:19:11 +0100 Subject: [PATCH 2/2] PR fix: Add test for null attribute (#66) This adds on top of https://github.com/getsentry/rrweb/pull/65 and adds a test for this, as well as streamlining this check in the `transformAttribute` method. --- packages/rrweb-snapshot/src/rebuild.ts | 2 +- packages/rrweb-snapshot/src/snapshot.ts | 27 +-- packages/rrweb-snapshot/src/types.ts | 2 +- packages/rrweb-snapshot/typings/snapshot.d.ts | 2 +- packages/rrweb-snapshot/typings/types.d.ts | 2 +- .../__snapshots__/integration.test.ts.snap | 199 ++++++++++++++++++ packages/rrweb/test/integration.test.ts | 31 +++ 7 files changed, 249 insertions(+), 16 deletions(-) diff --git a/packages/rrweb-snapshot/src/rebuild.ts b/packages/rrweb-snapshot/src/rebuild.ts index 263ad99181..34c8766ffd 100644 --- a/packages/rrweb-snapshot/src/rebuild.ts +++ b/packages/rrweb-snapshot/src/rebuild.ts @@ -154,7 +154,7 @@ function buildNode( continue; } value = - typeof value === 'boolean' || typeof value === 'number' ? '' : value; + typeof value === 'boolean' || typeof value === 'number' || value === null ? '' : value; // attribute names start with rr_ are internal attributes added by rrweb if (!name.startsWith('rr_')) { const isTextarea = tagName === 'textarea' && name === 'value'; diff --git a/packages/rrweb-snapshot/src/snapshot.ts b/packages/rrweb-snapshot/src/snapshot.ts index 7955edff83..f1092f2c25 100644 --- a/packages/rrweb-snapshot/src/snapshot.ts +++ b/packages/rrweb-snapshot/src/snapshot.ts @@ -238,36 +238,39 @@ export function transformAttribute( doc: Document, tagName: string, name: string, - value: string, + value: string | null, maskAllText: boolean, maskTextFn: MaskTextFn | undefined, -): string { +): string | null { + if (!value) { + return value; + } + // relative path in attribute - if (name === 'src' || (name === 'href' && value)) { + if (name === 'src' || name === 'href') { return absoluteToDoc(doc, value); - } else if (name === 'xlink:href' && value && value[0] !== '#') { + } else if (name === 'xlink:href' && value[0] !== '#') { // xlink:href starts with # is an id pointer return absoluteToDoc(doc, value); } else if ( name === 'background' && - value && (tagName === 'table' || tagName === 'td' || tagName === 'th') ) { return absoluteToDoc(doc, value); - } else if (name === 'srcset' && value) { + } else if (name === 'srcset') { return getAbsoluteSrcsetString(doc, value); - } else if (name === 'style' && value) { + } else if (name === 'style') { return absoluteToStylesheet(value, getHref()); - } else if (tagName === 'object' && name === 'data' && value) { + } else if (tagName === 'object' && name === 'data') { return absoluteToDoc(doc, value); } else if ( maskAllText && ['placeholder', 'title', 'aria-label'].indexOf(name) > -1 ) { return maskTextFn ? maskTextFn(value) : defaultMaskFn(value); - } else { - return value; } + + return value; } export function _isBlockedElement( @@ -770,8 +773,8 @@ function serializeNode( } } -function lowerIfExists(maybeAttr: string | number | boolean): string { - if (maybeAttr === undefined) { +function lowerIfExists(maybeAttr: string | number | boolean | null | undefined): string { + if (maybeAttr === undefined || maybeAttr === null) { return ''; } else { return (maybeAttr as string).toLowerCase(); diff --git a/packages/rrweb-snapshot/src/types.ts b/packages/rrweb-snapshot/src/types.ts index eedd49252f..834d4d2e40 100644 --- a/packages/rrweb-snapshot/src/types.ts +++ b/packages/rrweb-snapshot/src/types.ts @@ -21,7 +21,7 @@ export type documentTypeNode = { }; export type attributes = { - [key: string]: string | number | boolean; + [key: string]: string | number | boolean | null; }; export type elementNode = { type: NodeType.Element; diff --git a/packages/rrweb-snapshot/typings/snapshot.d.ts b/packages/rrweb-snapshot/typings/snapshot.d.ts index 65cad17f4f..0f4ec0b1b1 100644 --- a/packages/rrweb-snapshot/typings/snapshot.d.ts +++ b/packages/rrweb-snapshot/typings/snapshot.d.ts @@ -2,7 +2,7 @@ import { serializedNodeWithId, INode, idNodeMap, MaskInputOptions, SlimDOMOption export declare const IGNORED_NODE = -2; export declare function absoluteToStylesheet(cssText: string | null, href: string): string; export declare function absoluteToDoc(doc: Document, attributeValue: string): string; -export declare function transformAttribute(doc: Document, tagName: string, name: string, value: string, maskAllText: boolean, maskTextFn: MaskTextFn | undefined): string; +export declare function transformAttribute(doc: Document, tagName: string, name: string, value: string | null, maskAllText: boolean, maskTextFn: MaskTextFn | undefined): string | null; export declare function _isBlockedElement(element: HTMLElement, blockClass: string | RegExp, blockSelector: string | null, unblockSelector: string | null): boolean; export declare function needMaskingText(node: Node | null, maskTextClass: string | RegExp, maskTextSelector: string | null, unmaskTextSelector: string | null, maskAllText: boolean): boolean; export declare function serializeNodeWithId(n: Node | INode, options: { diff --git a/packages/rrweb-snapshot/typings/types.d.ts b/packages/rrweb-snapshot/typings/types.d.ts index 4d1eb144e8..6d0fc98602 100644 --- a/packages/rrweb-snapshot/typings/types.d.ts +++ b/packages/rrweb-snapshot/typings/types.d.ts @@ -18,7 +18,7 @@ export type documentTypeNode = { systemId: string; }; export type attributes = { - [key: string]: string | number | boolean; + [key: string]: string | number | boolean | null; }; export type elementNode = { type: NodeType.Element; diff --git a/packages/rrweb/test/__snapshots__/integration.test.ts.snap b/packages/rrweb/test/__snapshots__/integration.test.ts.snap index 86f4466ac9..782918e17f 100644 --- a/packages/rrweb/test/__snapshots__/integration.test.ts.snap +++ b/packages/rrweb/test/__snapshots__/integration.test.ts.snap @@ -3684,6 +3684,205 @@ exports[`record integration tests can use maskInputOptions to configure which ty ]" `; +exports[`record integration tests handles null attribute values 1`] = ` +"[ + { + "type": 0, + "data": {} + }, + { + "type": 1, + "data": {} + }, + { + "type": 4, + "data": { + "href": "about:blank", + "width": 1920, + "height": 1080 + } + }, + { + "type": 2, + "data": { + "node": { + "type": 0, + "childNodes": [ + { + "type": 1, + "name": "html", + "publicId": "", + "systemId": "", + "id": 2 + }, + { + "type": 2, + "tagName": "html", + "attributes": {}, + "childNodes": [ + { + "type": 2, + "tagName": "head", + "attributes": {}, + "childNodes": [], + "id": 4 + }, + { + "type": 2, + "tagName": "body", + "attributes": {}, + "childNodes": [ + { + "type": 3, + "textContent": "\\n ", + "id": 6 + }, + { + "type": 2, + "tagName": "p", + "attributes": {}, + "childNodes": [ + { + "type": 3, + "textContent": "******** ********", + "id": 8 + } + ], + "id": 7 + }, + { + "type": 3, + "textContent": "\\n ", + "id": 9 + }, + { + "type": 2, + "tagName": "ul", + "attributes": {}, + "childNodes": [ + { + "type": 3, + "textContent": "\\n ", + "id": 11 + }, + { + "type": 2, + "tagName": "li", + "attributes": {}, + "childNodes": [], + "id": 12 + }, + { + "type": 3, + "textContent": "\\n ", + "id": 13 + } + ], + "id": 10 + }, + { + "type": 3, + "textContent": "\\n ", + "id": 14 + }, + { + "type": 2, + "tagName": "canvas", + "attributes": {}, + "childNodes": [], + "id": 15 + }, + { + "type": 3, + "textContent": "\\n\\n ", + "id": 16 + }, + { + "type": 2, + "tagName": "script", + "attributes": {}, + "childNodes": [ + { + "type": 3, + "textContent": "SCRIPT_PLACEHOLDER", + "id": 18 + } + ], + "id": 17 + }, + { + "type": 3, + "textContent": "\\n \\n \\n", + "id": 19 + } + ], + "id": 5 + } + ], + "id": 3 + } + ], + "id": 1 + }, + "initialOffset": { + "left": 0, + "top": 0 + } + } + }, + { + "type": 3, + "data": { + "source": 0, + "texts": [], + "attributes": [ + { + "id": 20, + "attributes": { + "aria-label": "*****", + "id": "test-li" + } + } + ], + "removes": [], + "adds": [ + { + "parentId": 10, + "nextId": null, + "node": { + "type": 2, + "tagName": "li", + "attributes": { + "aria-label": "*****", + "id": "test-li" + }, + "childNodes": [], + "id": 20 + } + } + ] + } + }, + { + "type": 3, + "data": { + "source": 0, + "texts": [], + "attributes": [ + { + "id": 20, + "attributes": { + "aria-label": null + } + } + ], + "removes": [], + "adds": [] + } + } +]" +`; + exports[`record integration tests should mask all text (except unmaskTextSelector), using maskAllText 1`] = ` "[ { diff --git a/packages/rrweb/test/integration.test.ts b/packages/rrweb/test/integration.test.ts index 8283fa482f..9c09082a5a 100644 --- a/packages/rrweb/test/integration.test.ts +++ b/packages/rrweb/test/integration.test.ts @@ -130,6 +130,37 @@ describe('record integration tests', function (this: ISuite) { assertSnapshot(snapshots); }); + it('handles null attribute values', async () => { + const page: puppeteer.Page = await browser.newPage(); + await page.goto('about:blank'); + await page.setContent( + getHtml.call(this, 'mutation-observer.html', { + maskAllInputs: true, + maskAllText: true, + }), + ); + + await page.evaluate(() => { + const li = document.createElement('li'); + const ul = document.querySelector('ul') as HTMLUListElement; + ul.appendChild(li); + + li.setAttribute('aria-label', 'label'); + li.setAttribute('id', 'test-li'); + }); + + await new Promise((resolve) => setTimeout(resolve, 100)); + + await page.evaluate(() => { + const li = document.querySelector('#test-li') as HTMLLIElement; + // This triggers the mutation observer with a `null` attribute value + li.removeAttribute('aria-label'); + }); + + const snapshots = await page.evaluate('window.snapshots'); + assertSnapshot(snapshots); + }); + it('can record character data muatations', async () => { const page: puppeteer.Page = await browser.newPage(); await page.goto('about:blank');