Skip to content

Commit

Permalink
fix: Fix input.type check (#1184)
Browse files Browse the repository at this point in the history
* fix: Fix input.type check

Actually I noticed that `el.type` returns `text` when type is not explicitly set, so this is slightly incorrect.

* fix linting

* Apply formatting changes
  • Loading branch information
mydea authored Apr 7, 2023
1 parent d0fdc0f commit aa79db7
Show file tree
Hide file tree
Showing 9 changed files with 298 additions and 23 deletions.
5 changes: 5 additions & 0 deletions .changeset/nice-pugs-reply.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'rrweb': patch
---

fix: Ensure getting the type of inputs works
7 changes: 2 additions & 5 deletions packages/rrweb-snapshot/src/snapshot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
maskInputValue,
isNativeShadowDom,
getCssRulesString,
getInputType,
} from './utils';

let _id = 1;
Expand Down Expand Up @@ -682,11 +683,7 @@ function serializeElementNode(
attributes.type !== 'button' &&
value
) {
const type: string | null = n.hasAttribute('data-rr-is-password')
? 'password'
: typeof attributes.type === 'string'
? attributes.type.toLowerCase()
: null;
const type = getInputType(n);
attributes.value = maskInputValue({
type,
tagName,
Expand Down
17 changes: 17 additions & 0 deletions packages/rrweb-snapshot/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -248,3 +248,20 @@ export function isNodeMetaEqual(a: serializedNode, b: serializedNode): boolean {
);
return false;
}

/**
* 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
? // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
(type.toLowerCase() as Lowercase<string>)
: null;
}
2 changes: 1 addition & 1 deletion packages/rrweb/src/record/mutation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
maskInputValue,
Mirror,
isNativeShadowDom,
getInputType,
} from 'rrweb-snapshot';
import type { observerParam, MutationBufferParam } from '../types';
import type {
Expand All @@ -29,7 +30,6 @@ import {
isSerializedStylesheet,
inDom,
getShadowHost,
getInputType,
} from '../utils';

type DoubleLinkedListNode = {
Expand Down
8 changes: 6 additions & 2 deletions packages/rrweb/src/record/observer.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
import { MaskInputOptions, maskInputValue, Mirror } from 'rrweb-snapshot';
import {
MaskInputOptions,
maskInputValue,
Mirror,
getInputType,
} from 'rrweb-snapshot';
import type { FontFaceSet } from 'css-font-loading-module';
import {
throttle,
on,
hookSetter,
getInputType,
getWindowScroll,
getWindowHeight,
getWindowWidth,
Expand Down
15 changes: 0 additions & 15 deletions packages/rrweb/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -563,18 +563,3 @@ export function inDom(n: Node): boolean {
if (!doc) return false;
return doc.contains(n) || shadowHostInDom(n);
}

/**
* 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 {
return element.hasAttribute('data-rr-is-password')
? 'password'
: element.hasAttribute('type')
? // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion, @typescript-eslint/no-non-null-assertion
(element.getAttribute('type')!.toLowerCase() as Lowercase<string>)
: null;
}
232 changes: 232 additions & 0 deletions packages/rrweb/test/__snapshots__/integration.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -7473,6 +7473,238 @@ exports[`record integration tests should not record input events on ignored elem
]"
`;

exports[`record integration tests should not record input values if dynamically added and maskAllInputs is true 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\\": {
\\"lang\\": \\"en\\"
},
\\"childNodes\\": [
{
\\"type\\": 2,
\\"tagName\\": \\"head\\",
\\"attributes\\": {},
\\"childNodes\\": [
{
\\"type\\": 3,
\\"textContent\\": \\"\\\\n \\",
\\"id\\": 5
},
{
\\"type\\": 2,
\\"tagName\\": \\"meta\\",
\\"attributes\\": {
\\"charset\\": \\"UTF-8\\"
},
\\"childNodes\\": [],
\\"id\\": 6
},
{
\\"type\\": 3,
\\"textContent\\": \\"\\\\n \\",
\\"id\\": 7
},
{
\\"type\\": 2,
\\"tagName\\": \\"meta\\",
\\"attributes\\": {
\\"name\\": \\"viewport\\",
\\"content\\": \\"width=device-width, initial-scale=1.0\\"
},
\\"childNodes\\": [],
\\"id\\": 8
},
{
\\"type\\": 3,
\\"textContent\\": \\"\\\\n \\",
\\"id\\": 9
},
{
\\"type\\": 2,
\\"tagName\\": \\"title\\",
\\"attributes\\": {},
\\"childNodes\\": [
{
\\"type\\": 3,
\\"textContent\\": \\"Empty\\",
\\"id\\": 11
}
],
\\"id\\": 10
},
{
\\"type\\": 3,
\\"textContent\\": \\"\\\\n \\",
\\"id\\": 12
}
],
\\"id\\": 4
},
{
\\"type\\": 3,
\\"textContent\\": \\"\\\\n \\",
\\"id\\": 13
},
{
\\"type\\": 2,
\\"tagName\\": \\"body\\",
\\"attributes\\": {},
\\"childNodes\\": [
{
\\"type\\": 3,
\\"textContent\\": \\"\\\\n \\",
\\"id\\": 15
},
{
\\"type\\": 2,
\\"tagName\\": \\"div\\",
\\"attributes\\": {
\\"id\\": \\"one\\"
},
\\"childNodes\\": [],
\\"id\\": 16
},
{
\\"type\\": 3,
\\"textContent\\": \\"\\\\n \\\\n \\",
\\"id\\": 17
},
{
\\"type\\": 2,
\\"tagName\\": \\"script\\",
\\"attributes\\": {},
\\"childNodes\\": [
{
\\"type\\": 3,
\\"textContent\\": \\"SCRIPT_PLACEHOLDER\\",
\\"id\\": 19
}
],
\\"id\\": 18
},
{
\\"type\\": 3,
\\"textContent\\": \\"\\\\n \\\\n \\\\n\\\\n\\",
\\"id\\": 20
}
],
\\"id\\": 14
}
],
\\"id\\": 3
}
],
\\"id\\": 1
},
\\"initialOffset\\": {
\\"left\\": 0,
\\"top\\": 0
}
}
},
{
\\"type\\": 3,
\\"data\\": {
\\"source\\": 0,
\\"texts\\": [],
\\"attributes\\": [],
\\"removes\\": [],
\\"adds\\": [
{
\\"parentId\\": 14,
\\"nextId\\": 16,
\\"node\\": {
\\"type\\": 2,
\\"tagName\\": \\"input\\",
\\"attributes\\": {
\\"id\\": \\"input\\",
\\"value\\": \\"**********************\\"
},
\\"childNodes\\": [],
\\"id\\": 21
}
}
]
}
},
{
\\"type\\": 3,
\\"data\\": {
\\"source\\": 5,
\\"text\\": \\"**********************\\",
\\"isChecked\\": false,
\\"id\\": 21
}
},
{
\\"type\\": 3,
\\"data\\": {
\\"source\\": 2,
\\"type\\": 5,
\\"id\\": 21
}
},
{
\\"type\\": 3,
\\"data\\": {
\\"source\\": 5,
\\"text\\": \\"***********************\\",
\\"isChecked\\": false,
\\"id\\": 21
}
},
{
\\"type\\": 3,
\\"data\\": {
\\"source\\": 5,
\\"text\\": \\"************************\\",
\\"isChecked\\": false,
\\"id\\": 21
}
},
{
\\"type\\": 3,
\\"data\\": {
\\"source\\": 5,
\\"text\\": \\"*************************\\",
\\"isChecked\\": false,
\\"id\\": 21
}
}
]"
`;

exports[`record integration tests should not record input values if maskAllInputs is enabled 1`] = `
"[
{
Expand Down
11 changes: 11 additions & 0 deletions packages/rrweb/test/html/empty.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>Empty</title>
</head>
<body>
<div id="one"></div>
</body>
</html>
24 changes: 24 additions & 0 deletions packages/rrweb/test/integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,30 @@ describe('record integration tests', function (this: ISuite) {
assertSnapshot(snapshots);
});

it('should not record input values if dynamically added and maskAllInputs is true', async () => {
const page: puppeteer.Page = await browser.newPage();
await page.goto('about:blank');
await page.setContent(
getHtml.call(this, 'empty.html', { maskAllInputs: true }),
);

await page.evaluate(() => {
const el = document.createElement('input');
el.id = 'input';
el.value = 'input should be masked';

const nextElement = document.querySelector('#one')!;
nextElement.parentNode!.insertBefore(el, nextElement);
});

await page.type('#input', 'moo');

const snapshots = (await page.evaluate(
'window.snapshots',
)) as eventWithTime[];
assertSnapshot(snapshots);
});

it('should record webgl canvas mutations', async () => {
const page: puppeteer.Page = await browser.newPage();
await page.goto('about:blank');
Expand Down

0 comments on commit aa79db7

Please sign in to comment.