Skip to content

Commit 768f25c

Browse files
cy-moicy-moi
committed
Extract shouldMaskAttriubte to share for SR and action names; Improve code according to comments
1 parent 06574d5 commit 768f25c

File tree

5 files changed

+107
-81
lines changed

5 files changed

+107
-81
lines changed

‎packages/rum-core/src/domain/action/getActionNameFromElement.spec.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -645,7 +645,7 @@ describe('getActionNameFromElement', () => {
645645
},
646646
{
647647
html: `
648-
<div data-dd-privacy="mask-unless-allowlisted" aria-label="bar" target>
648+
<div data-dd-privacy="mask-unless-allowlisted" alt="bar" target>
649649
<span>bar</span>
650650
</div>
651651
`,
@@ -655,7 +655,7 @@ describe('getActionNameFromElement', () => {
655655
},
656656
{
657657
html: `
658-
<div data-dd-privacy="mask-unless-allowlisted" aria-label="foo" target>
658+
<div data-dd-privacy="mask-unless-allowlisted" alt="foo" target>
659659
<span>bar</span>
660660
</div>
661661
`,

‎packages/rum-core/src/domain/action/getActionNameFromElement.ts‎

Lines changed: 57 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { ExperimentalFeature, isExperimentalFeatureEnabled, safeTruncate } from '@datadog/browser-core'
22
import { getPrivacySelector, NodePrivacyLevel } from '../privacyConstants'
3-
import { getNodePrivacyLevel, maskDisallowedTextContent, shouldMaskNode } from '../privacy'
3+
import { getNodePrivacyLevel, maskDisallowedTextContent, shouldMaskNode, shouldMaskAttribute } from '../privacy'
44
import type { NodePrivacyLevelCache } from '../privacy'
55
import type { RumConfiguration } from '../configuration'
66
import { isElementNode } from '../../browser/htmlDomUtils'
@@ -16,6 +16,8 @@ export function getActionNameFromElement(
1616
rumConfiguration: RumConfiguration,
1717
nodePrivacyLevel: NodePrivacyLevel = NodePrivacyLevel.ALLOW
1818
): ActionName {
19+
const nodePrivacyLevelCache: NodePrivacyLevelCache = new Map()
20+
1921
const { actionNameAttribute: userProgrammaticAttribute } = rumConfiguration
2022

2123
// Proceed to get the action name in two steps:
@@ -36,8 +38,8 @@ export function getActionNameFromElement(
3638
}
3739

3840
return (
39-
getActionNameFromElementForStrategies(element, priorityStrategies, rumConfiguration) ||
40-
getActionNameFromElementForStrategies(element, fallbackStrategies, rumConfiguration) || {
41+
getActionNameFromElementForStrategies(element, priorityStrategies, rumConfiguration, nodePrivacyLevelCache) ||
42+
getActionNameFromElementForStrategies(element, fallbackStrategies, rumConfiguration, nodePrivacyLevelCache) || {
4143
name: '',
4244
nameSource: ActionNameSource.BLANK,
4345
}
@@ -58,14 +60,15 @@ function getActionNameFromElementProgrammatically(targetElement: Element, progra
5860

5961
type NameStrategy = (
6062
element: Element | HTMLElement | HTMLInputElement | HTMLSelectElement,
61-
rumConfiguration: RumConfiguration
63+
rumConfiguration: RumConfiguration,
64+
nodePrivacyLevelCache: NodePrivacyLevelCache
6265
) => ActionName | undefined | null
6366

6467
const priorityStrategies: NameStrategy[] = [
6568
// associated LABEL text
66-
(element, rumConfiguration) => {
69+
(element, rumConfiguration, nodePrivacyLevelCache) => {
6770
if ('labels' in element && element.labels && element.labels.length > 0) {
68-
return getActionNameFromTextualContent(element.labels[0], rumConfiguration)
71+
return getActionNameFromTextualContent(element.labels[0], rumConfiguration, nodePrivacyLevelCache)
6972
}
7073
},
7174
// INPUT button (and associated) value
@@ -79,41 +82,47 @@ const priorityStrategies: NameStrategy[] = [
7982
}
8083
},
8184
// BUTTON, LABEL or button-like element text
82-
(element, rumConfiguration) => {
85+
(element, rumConfiguration, nodePrivacyLevelCache) => {
8386
if (element.nodeName === 'BUTTON' || element.nodeName === 'LABEL' || element.getAttribute('role') === 'button') {
84-
return getActionNameFromTextualContent(element, rumConfiguration)
87+
return getActionNameFromTextualContent(element, rumConfiguration, nodePrivacyLevelCache)
8588
}
8689
},
87-
(element, rumConfiguration) => getActionNameFromStandardAttribute(element, 'aria-label', rumConfiguration),
90+
(element, rumConfiguration, nodePrivacyLevelCache) =>
91+
getActionNameFromStandardAttribute(element, 'aria-label', rumConfiguration, nodePrivacyLevelCache),
8892
// associated element text designated by the aria-labelledby attribute
89-
(element, rumConfiguration) => {
93+
(element, rumConfiguration, nodePrivacyLevelCache) => {
9094
const labelledByAttribute = element.getAttribute('aria-labelledby')
9195
if (labelledByAttribute) {
9296
return {
9397
name: labelledByAttribute
9498
.split(/\s+/)
9599
.map((id) => getElementById(element, id))
96100
.filter((label): label is HTMLElement => Boolean(label))
97-
.map((element) => getTextualContent(element, rumConfiguration))
101+
.map((element) => getTextualContent(element, rumConfiguration, nodePrivacyLevelCache))
98102
.join(' '),
99103
nameSource: ActionNameSource.TEXT_CONTENT,
100104
}
101105
}
102106
},
103-
(element, rumConfiguration) => getActionNameFromStandardAttribute(element, 'alt', rumConfiguration),
104-
(element, rumConfiguration) => getActionNameFromStandardAttribute(element, 'name', rumConfiguration),
105-
(element, rumConfiguration) => getActionNameFromStandardAttribute(element, 'title', rumConfiguration),
106-
(element, rumConfiguration) => getActionNameFromStandardAttribute(element, 'placeholder', rumConfiguration),
107+
(element, rumConfiguration, nodePrivacyLevelCache) =>
108+
getActionNameFromStandardAttribute(element, 'alt', rumConfiguration, nodePrivacyLevelCache),
109+
(element, rumConfiguration, nodePrivacyLevelCache) =>
110+
getActionNameFromStandardAttribute(element, 'name', rumConfiguration, nodePrivacyLevelCache),
111+
(element, rumConfiguration, nodePrivacyLevelCache) =>
112+
getActionNameFromStandardAttribute(element, 'title', rumConfiguration, nodePrivacyLevelCache),
113+
(element, rumConfiguration, nodePrivacyLevelCache) =>
114+
getActionNameFromStandardAttribute(element, 'placeholder', rumConfiguration, nodePrivacyLevelCache),
107115
// SELECT first OPTION text
108-
(element, rumConfiguration) => {
116+
(element, rumConfiguration, nodePrivacyLevelCache) => {
109117
if ('options' in element && element.options.length > 0) {
110-
return getActionNameFromTextualContent(element.options[0], rumConfiguration)
118+
return getActionNameFromTextualContent(element.options[0], rumConfiguration, nodePrivacyLevelCache)
111119
}
112120
},
113121
]
114122

115123
const fallbackStrategies: NameStrategy[] = [
116-
(element, rumConfiguration) => getActionNameFromTextualContent(element, rumConfiguration),
124+
(element, rumConfiguration, nodePrivacyLevelCache) =>
125+
getActionNameFromTextualContent(element, rumConfiguration, nodePrivacyLevelCache),
117126
]
118127

119128
/**
@@ -124,7 +133,8 @@ const MAX_PARENTS_TO_CONSIDER = 10
124133
function getActionNameFromElementForStrategies(
125134
targetElement: Element,
126135
strategies: NameStrategy[],
127-
rumConfiguration: RumConfiguration
136+
rumConfiguration: RumConfiguration,
137+
nodePrivacyLevelCache: NodePrivacyLevelCache
128138
) {
129139
let element: Element | null = targetElement
130140
let recursionCounter = 0
@@ -136,7 +146,7 @@ function getActionNameFromElementForStrategies(
136146
element.nodeName !== 'HEAD'
137147
) {
138148
for (const strategy of strategies) {
139-
const actionName = strategy(element, rumConfiguration)
149+
const actionName = strategy(element, rumConfiguration, nodePrivacyLevelCache)
140150
if (actionName) {
141151
const { name, nameSource } = actionName
142152
const trimmedName = name && name.trim()
@@ -172,31 +182,44 @@ function getElementById(refElement: Element, id: string) {
172182
function getActionNameFromStandardAttribute(
173183
element: Element | HTMLElement,
174184
attribute: string,
175-
rumConfiguration: RumConfiguration
185+
rumConfiguration: RumConfiguration,
186+
nodePrivacyLevelCache: NodePrivacyLevelCache
176187
): ActionName {
177188
const { enablePrivacyForActionName, defaultPrivacyLevel } = rumConfiguration
178-
const nodeSelfPrivacyLevel = getNodePrivacyLevel(element, defaultPrivacyLevel)
179-
const attributeValue = element.getAttribute(attribute)
189+
let attributeValue = element.getAttribute(attribute)
190+
if (attributeValue && enablePrivacyForActionName) {
191+
const nodePrivacyLevel = getNodePrivacyLevel(element, defaultPrivacyLevel, nodePrivacyLevelCache)
192+
if (
193+
shouldMaskAttribute(element.tagName, attribute, nodePrivacyLevel, rumConfiguration, attributeValue)
194+
) {
195+
attributeValue = maskDisallowedTextContent(attributeValue, ACTION_NAME_PLACEHOLDER)
196+
}
197+
} else if (!attributeValue) {
198+
attributeValue = ''
199+
}
200+
180201
return {
181-
name:
182-
enablePrivacyForActionName && nodeSelfPrivacyLevel && shouldMaskNode(element, nodeSelfPrivacyLevel, false)
183-
? maskDisallowedTextContent(attributeValue || '', ACTION_NAME_PLACEHOLDER)
184-
: attributeValue || '',
202+
name: attributeValue,
185203
nameSource: ActionNameSource.STANDARD_ATTRIBUTE,
186204
}
187205
}
188206

189207
function getActionNameFromTextualContent(
190208
element: Element | HTMLElement,
191-
rumConfiguration: RumConfiguration
209+
rumConfiguration: RumConfiguration,
210+
nodePrivacyLevelCache: NodePrivacyLevelCache
192211
): ActionName {
193212
return {
194-
name: getTextualContent(element, rumConfiguration) || '',
213+
name: getTextualContent(element, rumConfiguration, nodePrivacyLevelCache) || '',
195214
nameSource: ActionNameSource.TEXT_CONTENT,
196215
}
197216
}
198217

199-
function getTextualContent(element: Element, rumConfiguration: RumConfiguration) {
218+
function getTextualContent(
219+
element: Element,
220+
rumConfiguration: RumConfiguration,
221+
nodePrivacyLevelCache: NodePrivacyLevelCache
222+
) {
200223
if ((element as HTMLElement).isContentEditable) {
201224
return
202225
}
@@ -212,7 +235,8 @@ function getTextualContent(element: Element, rumConfiguration: RumConfiguration)
212235
element,
213236
userProgrammaticAttribute,
214237
enablePrivacyForActionName,
215-
defaultPrivacyLevel
238+
defaultPrivacyLevel,
239+
nodePrivacyLevelCache
216240
)
217241
}
218242

@@ -256,10 +280,9 @@ function getTextualContentWithTreeWalker(
256280
element: Element,
257281
userProgrammaticAttribute: string | undefined,
258282
privacyEnabledActionName: boolean,
259-
defaultPrivacyLevel: NodePrivacyLevel
283+
defaultPrivacyLevel: NodePrivacyLevel,
284+
nodePrivacyLevelCache: NodePrivacyLevelCache
260285
) {
261-
const nodePrivacyLevelCache: NodePrivacyLevelCache = new Map()
262-
263286
const walker = document.createTreeWalker(
264287
element,
265288
// eslint-disable-next-line no-bitwise

‎packages/rum-core/src/domain/privacy.ts‎

Lines changed: 41 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,10 @@ import {
55
CENSORED_STRING_MARK,
66
getPrivacySelector,
77
TEXT_MASKING_CHAR,
8+
PRIVACY_ATTR_NAME,
89
} from './privacyConstants'
10+
import { STABLE_ATTRIBUTES } from './getSelectorFromElement'
11+
import type { RumConfiguration } from './configuration'
912

1013
export type NodePrivacyLevelCache = Map<Node, NodePrivacyLevel>
1114

@@ -128,16 +131,13 @@ export function getNodeSelfPrivacyLevel(node: Node): NodePrivacyLevel | undefine
128131
* Other `shouldMaskNode` cases are edge cases that should not matter too much (ex: should we mask a
129132
* node if it is ignored or hidden? it doesn't matter since it won't be serialized).
130133
*/
131-
export function shouldMaskNode(node: Node, privacyLevel: NodePrivacyLevel, isTextContent: boolean = true) {
134+
export function shouldMaskNode(node: Node, privacyLevel: NodePrivacyLevel) {
132135
switch (privacyLevel) {
133136
case NodePrivacyLevel.MASK:
134137
case NodePrivacyLevel.HIDDEN:
135138
case NodePrivacyLevel.IGNORE:
136139
return true
137140
case NodePrivacyLevel.MASK_UNLESS_ALLOWLISTED:
138-
if (!isTextContent) {
139-
return true
140-
}
141141
if (isTextNode(node)) {
142142
// Always return true if our parent is a form element, like MASK_USER_INPUT.
143143
// Otherwise, decide whether to mask based on the allowlist.
@@ -153,6 +153,43 @@ export function shouldMaskNode(node: Node, privacyLevel: NodePrivacyLevel, isTex
153153
}
154154
}
155155

156+
export function shouldMaskAttribute(
157+
tagName: string,
158+
attributeName: string,
159+
nodePrivacyLevel: NodePrivacyLevel,
160+
configuration: RumConfiguration,
161+
attributeValue?: string | null
162+
) {
163+
if (nodePrivacyLevel === NodePrivacyLevel.MASK || nodePrivacyLevel === NodePrivacyLevel.MASK_UNLESS_ALLOWLISTED) {
164+
if (
165+
attributeName !== PRIVACY_ATTR_NAME &&
166+
!STABLE_ATTRIBUTES.includes(attributeName) &&
167+
attributeName !== configuration.actionNameAttribute
168+
) {
169+
switch (attributeName) {
170+
case 'title':
171+
case 'alt':
172+
case 'placeholder':
173+
return true
174+
}
175+
if (tagName === 'A' && attributeName === 'href') {
176+
return true
177+
}
178+
if (tagName === 'IFRAME' && attributeName === 'srcdoc') {
179+
return true
180+
}
181+
if (attributeValue && attributeName.startsWith('data-')) {
182+
return true
183+
}
184+
if ((tagName === 'IMG' || tagName === 'SOURCE' ) && (attributeName === 'src' || attributeName === 'srcset')) {
185+
return true
186+
}
187+
}
188+
}
189+
190+
return false
191+
}
192+
156193
function isFormElement(node: Node | null): boolean {
157194
if (!node || node.nodeType !== node.ELEMENT_NODE) {
158195
return false

‎packages/rum/src/domain/record/serialization/htmlAst.specHelper.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -854,7 +854,7 @@ export const AST_MASK_UNLESS_ALLOWLISTED = {
854854
type: 2,
855855
tagName: 'a',
856856
attributes: {
857-
href: 'https://private.com/path/nested?query=param#hash',
857+
href: '***',
858858
},
859859
childNodes: [
860860
{
@@ -871,7 +871,7 @@ export const AST_MASK_UNLESS_ALLOWLISTED = {
871871
type: 2,
872872
tagName: 'img',
873873
attributes: {
874-
src: 'https://private.com/path/nested?query=param#hash',
874+
src: 'data:image/gif;base64,R0lGODlhAQABAIAAAMLCwgAAACH5BAAAAAAALAAAAAABAAEAAAICRAEAOw==',
875875
},
876876
childNodes: [],
877877
},

‎packages/rum/src/domain/record/serialization/serializeAttribute.ts‎

Lines changed: 5 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
11
import {
22
NodePrivacyLevel,
3-
PRIVACY_ATTR_NAME,
43
CENSORED_STRING_MARK,
54
CENSORED_IMG_MARK,
6-
STABLE_ATTRIBUTES,
75
sanitizeIfLongDataUrl,
6+
shouldMaskAttribute,
87
} from '@datadog/browser-rum-core'
98
import type { RumConfiguration } from '@datadog/browser-rum-core'
109
import { censoredImageForSize } from './serializationUtils'
@@ -24,24 +23,10 @@ export function serializeAttribute(
2423
return null
2524
}
2625
const attributeValue = element.getAttribute(attributeName)
27-
if (
28-
nodePrivacyLevel === NodePrivacyLevel.MASK &&
29-
attributeName !== PRIVACY_ATTR_NAME &&
30-
!STABLE_ATTRIBUTES.includes(attributeName) &&
31-
attributeName !== configuration.actionNameAttribute
32-
) {
33-
const tagName = element.tagName
34-
35-
switch (attributeName) {
36-
// Mask Attribute text content
37-
case 'title':
38-
case 'alt':
39-
case 'placeholder':
40-
return CENSORED_STRING_MARK
41-
}
42-
26+
const tagName = element.tagName
27+
if (shouldMaskAttribute(tagName, attributeName, nodePrivacyLevel, configuration, attributeValue)) {
4328
// mask image URLs
44-
if (tagName === 'IMG' && (attributeName === 'src' || attributeName === 'srcset')) {
29+
if (tagName === 'IMG') {
4530
// generate image with similar dimension than the original to have the same rendering behaviour
4631
const image = element as HTMLImageElement
4732
if (image.naturalWidth > 0) {
@@ -55,26 +40,7 @@ export function serializeAttribute(
5540
return CENSORED_IMG_MARK
5641
}
5742

58-
// mask source URLs
59-
if (tagName === 'SOURCE' && (attributeName === 'src' || attributeName === 'srcset')) {
60-
return CENSORED_IMG_MARK
61-
}
62-
63-
// mask <a> URLs
64-
if (tagName === 'A' && attributeName === 'href') {
65-
return CENSORED_STRING_MARK
66-
}
67-
68-
// mask data-* attributes
69-
if (attributeValue && attributeName.startsWith('data-')) {
70-
// Exception: it's safe to reveal the `${PRIVACY_ATTR_NAME}` attr
71-
return CENSORED_STRING_MARK
72-
}
73-
74-
// mask iframe srcdoc
75-
if (tagName === 'IFRAME' && attributeName === 'srcdoc') {
76-
return CENSORED_STRING_MARK
77-
}
43+
return CENSORED_STRING_MARK
7844
}
7945

8046
if (!attributeValue || typeof attributeValue !== 'string') {

0 commit comments

Comments
 (0)