Skip to content

Commit 4ca6f8a

Browse files
committed
⚗️ 🐛 validate feature-operation name against schema character set
The backend enforces [\w.@$-]* on vital.name via the vital-common-schema facet-path rule. The Browser SDK previously had no input validation on feature-operation vitals. Added validation in addOperationStepVital: - blank / whitespace-only name → display.warn, drop event - name with characters outside [\w.@$-]* → display.warn, emit event anyway (backend is source of truth on character-set policy) operation_key is not subject to this rule. Still gated behind the FEATURE_OPERATION_VITAL experimental flag.
1 parent 5d73db8 commit 4ca6f8a

3 files changed

Lines changed: 95 additions & 1 deletion

File tree

CHANGELOG.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,14 @@
1313
1414
---
1515

16+
## Unreleased
17+
18+
**Public Changes:**
19+
20+
- ⚗️🐛 fix(rum): validate `startFeatureOperation` / `succeedFeatureOperation` / `failFeatureOperation` `name`. Blank/empty names are dropped with a warning (matches the backend's own non-empty precondition). Names that fail the backend-accepted character-set pattern `[\w.@$-]*` also warn via `display.warn` but are still emitted, so a future backend policy relaxation does not force an SDK bump. Still gated behind the `feature_operation_vital` experimental flag. [RUM]
21+
22+
---
23+
1624
## v6.32.0
1725

1826
**Public Changes:**

packages/rum-core/src/domain/vital/vitalCollection.spec.ts

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type { Duration } from '@datadog/browser-core'
22
import { mockClock, type Clock } from '@datadog/browser-core/test'
3-
import { addExperimentalFeatures, clocksNow, ExperimentalFeature, generateUUID } from '@datadog/browser-core'
3+
import { addExperimentalFeatures, clocksNow, display, ExperimentalFeature, generateUUID } from '@datadog/browser-core'
44
import { collectAndValidateRawRumEvents, mockPageStateHistory } from '../../../test'
55
import type { RawRumEvent, RawRumVitalEvent } from '../../rawRumEvent.types'
66
import { VitalType, RumEventType } from '../../rawRumEvent.types'
@@ -271,6 +271,60 @@ describe('vitalCollection', () => {
271271
})
272272
})
273273

274+
// Mirrors the backend's `[\w.@$-]*` server-side validation regex. Names
275+
// that fail the pattern generate a `display.warn` but the event is
276+
// still emitted — the backend is the single source of truth, so a
277+
// client-side drop would force a customer SDK bump if the policy is
278+
// ever relaxed. Blank/empty names are dropped with a warning instead,
279+
// matching the backend's own non-empty precondition.
280+
describe('operation name character set', () => {
281+
beforeEach(() => {
282+
addExperimentalFeatures([ExperimentalFeature.FEATURE_OPERATION_VITAL])
283+
spyOn(display, 'warn')
284+
})
285+
;['user login', 'api/v1', 'checkout:step', 'a,b', 'login!', 'login\ttwo', 'ログイン', 'login🔐'].forEach(
286+
(invalidName) => {
287+
it(`should warn but still emit on name outside the backend pattern: ${JSON.stringify(invalidName)}`, () => {
288+
vitalCollection.addOperationStepVital(invalidName, 'start')
289+
290+
expect(rawRumEvents.length).toBe(1)
291+
expect((rawRumEvents[0].rawRumEvent as RawRumVitalEvent).vital.name).toBe(invalidName)
292+
expect(display.warn).toHaveBeenCalledTimes(1)
293+
expect((display.warn as jasmine.Spy).calls.mostRecent().args[0]).toContain('does not match')
294+
expect((display.warn as jasmine.Spy).calls.mostRecent().args[0]).toContain('still be sent')
295+
})
296+
}
297+
)
298+
;['', ' ', '\t\n'].forEach((blankName) => {
299+
it(`should reject and warn on blank name: ${JSON.stringify(blankName)}`, () => {
300+
vitalCollection.addOperationStepVital(blankName, 'start')
301+
302+
expect(rawRumEvents.length).toBe(0)
303+
expect(display.warn).toHaveBeenCalledTimes(1)
304+
expect((display.warn as jasmine.Spy).calls.mostRecent().args[0]).toContain('cannot be empty or blank')
305+
})
306+
})
307+
;['login', 'step42', 'login-v2', 'user_login', 'login.v2', 'login@prod', 'login$1', 'LoginV2'].forEach(
308+
(validName) => {
309+
it(`should accept name that matches the backend pattern without warning: ${JSON.stringify(validName)}`, () => {
310+
vitalCollection.addOperationStepVital(validName, 'start')
311+
312+
expect(rawRumEvents.length).toBe(1)
313+
expect((rawRumEvents[0].rawRumEvent as RawRumVitalEvent).vital.name).toBe(validName)
314+
expect(display.warn).not.toHaveBeenCalled()
315+
})
316+
}
317+
)
318+
319+
it('should not restrict operationKey to the same character set', () => {
320+
vitalCollection.addOperationStepVital('foo', 'start', { operationKey: 'session 42 / user foo' })
321+
322+
expect(rawRumEvents.length).toBe(1)
323+
expect((rawRumEvents[0].rawRumEvent as RawRumVitalEvent).vital.operation_key).toBe('session 42 / user foo')
324+
expect(display.warn).not.toHaveBeenCalled()
325+
})
326+
})
327+
274328
it('should create a duration vital from add API', () => {
275329
vitalCollection.addDurationVital({
276330
id: generateUUID(),

packages/rum-core/src/domain/vital/vitalCollection.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type { ClocksState, Duration } from '@datadog/browser-core'
22
import {
33
clocksNow,
44
combine,
5+
display,
56
elapsed,
67
ExperimentalFeature,
78
generateUUID,
@@ -124,6 +125,10 @@ export function startVitalCollection(
124125
return
125126
}
126127

128+
if (!validateOperationName(name)) {
129+
return
130+
}
131+
127132
const { operationKey, context, description, handlingStack } = options || {}
128133

129134
const vital: OperationStepVital = {
@@ -249,3 +254,30 @@ function processVital(vital: DurationVital | OperationStepVital): RawRumEventCol
249254
domainContext: handlingStack ? { handlingStack } : {},
250255
}
251256
}
257+
258+
/**
259+
* Validates a feature-operation `vital.name`.
260+
*
261+
* Blank / empty names are rejected (the backend rejects them with its own
262+
* non-empty precondition before evaluating the character-set regex). Names
263+
* that fail the backend's `[\w.@$-]*` character-set regex trigger a warning
264+
* but the event is still emitted — the backend is the source of truth on
265+
* character-set policy, so client-side drop would force a customer SDK bump
266+
* if the rule is ever relaxed.
267+
*
268+
* Returns `true` when the event should be emitted.
269+
*/
270+
const BACKEND_OPERATION_NAME_REGEX = /^[\w.@$-]*$/
271+
272+
function validateOperationName(name: string): boolean {
273+
if (typeof name !== 'string' || name.trim().length === 0) {
274+
display.warn('Feature operation name cannot be empty or blank. Event will not be sent.')
275+
return false
276+
}
277+
if (!BACKEND_OPERATION_NAME_REGEX.test(name)) {
278+
display.warn(
279+
`Feature operation name '${name}' does not match the backend-accepted pattern [\\w.@$-]* (letters, digits, _ . @ $ -). The event will still be sent and may be rejected by the backend.`
280+
)
281+
}
282+
return true
283+
}

0 commit comments

Comments
 (0)