diff --git a/DatadogRUM/Sources/RUMMonitor/Scopes/RUMFeatureOperationManager.swift b/DatadogRUM/Sources/RUMMonitor/Scopes/RUMFeatureOperationManager.swift index 1178eb3776..5b0b959daa 100644 --- a/DatadogRUM/Sources/RUMMonitor/Scopes/RUMFeatureOperationManager.swift +++ b/DatadogRUM/Sources/RUMMonitor/Scopes/RUMFeatureOperationManager.swift @@ -157,30 +157,62 @@ internal class RUMFeatureOperationManager { /// Validates the `name` and `operationKey` of the command private func validateCommand(_ command: RUMOperationStepVitalCommand) -> Bool { // Validate name (required) - guard validateString(command.name, stepType: command.stepType) else { + guard validateName(command.name, stepType: command.stepType) else { return false } // Validate operationKey if present (optional) if let operationKey = command.operationKey, - !validateString(operationKey, stepType: command.stepType) { + !validateOperationKey(operationKey, stepType: command.stepType) { return false } return true } - /// Validates the string is not empty - // or contains only whitespace/line breaks - private func validateString(_ value: String, stepType: RUMVitalOperationStepEvent.Vital.StepType) -> Bool { + /// ASCII character set accepted by the backend for `vital.name`, matching + /// the server-side regex `[\w.@$-]` (letters, digits, `_`, `.`, `@`, `$`, + /// `-`). Built explicitly from ASCII so that Unicode-aware categories + /// (which `CharacterSet.alphanumerics` would pull in) do not mask + /// non-conforming names — e.g. `ログイン` is all Unicode "Letter, other" + /// and must still trigger the warning. + private static let validOperationNameCharacters = CharacterSet( + charactersIn: "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789_.@$-" + ) + + /// Validates the operation name. + /// + /// Blank / empty names are rejected (the backend rejects them with its + /// own non-empty precondition before evaluating the character-set regex). + /// Names that fail the backend's `[\w.@$-]*` character-set regex trigger + /// a developer warning but the event is still emitted — the backend is + /// the source of truth on character-set policy, so client-side drop + /// would force an SDK bump if that policy is ever relaxed. + private func validateName(_ value: String, stepType: RUMVitalOperationStepEvent.Vital.StepType) -> Bool { let trimmed = value.trimmingCharacters(in: .whitespacesAndNewlines) guard !trimmed.isEmpty else { - DD.logger.error("Operation `name` and `operationKey` cannot be empty or contain only whitespace/line breaks. \(stepType) command will be ignored.") + DD.logger.error("Operation `name` cannot be empty or contain only whitespace/line breaks. \(stepType) command will be ignored.") + return false + } + + if !value.unicodeScalars.allSatisfy(Self.validOperationNameCharacters.contains) { + DD.logger.warn("Operation `name` '\(value)' does not match the backend-accepted pattern [\\w.@$-]* (letters, digits, _ . @ $ -). The \(stepType) command will still be emitted and may be rejected by the backend.") + } + + return true + } + + /// Validates the operation key: non-blank. The schema does not restrict + /// the character set for `operation_key`. + private func validateOperationKey(_ value: String, stepType: RUMVitalOperationStepEvent.Vital.StepType) -> Bool { + let trimmed = value.trimmingCharacters(in: .whitespacesAndNewlines) + + guard !trimmed.isEmpty else { + DD.logger.error("Operation `operationKey`, when provided, cannot be empty or contain only whitespace/line breaks. \(stepType) command will be ignored.") return false } - // Backend takes care of sanitizing user inputs return true } diff --git a/DatadogRUM/Tests/RUMMonitor/RUMFeatureOperationManagerTests.swift b/DatadogRUM/Tests/RUMMonitor/RUMFeatureOperationManagerTests.swift index e34babe29c..542f5c1722 100644 --- a/DatadogRUM/Tests/RUMMonitor/RUMFeatureOperationManagerTests.swift +++ b/DatadogRUM/Tests/RUMMonitor/RUMFeatureOperationManagerTests.swift @@ -128,6 +128,8 @@ class RUMFeatureOperationManagerTests: XCTestCase { // MARK: - Edge Cases Tests + // Blank / empty names are rejected: the backend rejects them with its + // own non-empty precondition, so the SDK drops client-side to match. private let invalidNames = ["", " ", "\n", "\t", " \n\t "] func testProcess_OperationWithInvalidName_DoesNotCreateVitalEvent() { // Given @@ -148,6 +150,81 @@ class RUMFeatureOperationManagerTests: XCTestCase { XCTAssertEqual(vitalEvents.count, 0) } + // Names containing characters outside the schema facet-path set + // (letters / digits / - _ . @ $). All must be rejected at the API boundary. + private let invalidCharacterSetNames = [ + "user login", // space + "api/v1", // slash + "checkout:step", // colon + "a,b", // comma + "a+b", // plus + "login!", // bang + "login\ttwo", // tab + "ログイン", // Unicode + "login🔐", // emoji + ] + + func testProcess_OperationWithNameOutsideSchemaCharacterSet_StillCreatesVitalEvent() { + // Names outside the schema facet-path set are warned about but still + // emitted — the backend is the source of truth on character-set policy, + // so client-side drop would force an SDK bump if the rule were relaxed. + for invalidName in invalidCharacterSetNames { + let command = RUMOperationStepVitalCommand.mockWith(name: invalidName) + + // When + manager.process( + command, + context: mockContext, + writer: mockWriter, + activeView: .mockAny() + ) + } + + // Then — every event is emitted; the name is preserved verbatim. + let vitalEvents = mockWriter.events(ofType: RUMVitalOperationStepEvent.self) + XCTAssertEqual(vitalEvents.count, invalidCharacterSetNames.count) + for (emitted, expected) in zip(vitalEvents, invalidCharacterSetNames) { + XCTAssertEqual(emitted.vital.name, expected) + } + } + + func testProcess_OperationWithNameInSchemaCharacterSet_CreatesVitalEvent() { + // Given — exercises every allowed character class + let validNames = ["login", "step42", "login-v2", "user_login", "login.v2", "login@prod", "login$1", "LoginV2"] + for validName in validNames { + let command = RUMOperationStepVitalCommand.mockWith(name: validName) + + // When + manager.process( + command, + context: mockContext, + writer: mockWriter, + activeView: .mockAny() + ) + } + + // Then + let vitalEvents = mockWriter.events(ofType: RUMVitalOperationStepEvent.self) + XCTAssertEqual(vitalEvents.count, validNames.count) + } + + func testProcess_OperationKeyOutsideNameCharacterSet_CreatesVitalEvent() { + // operation_key has no character-set restriction in the schema. + let command = RUMOperationStepVitalCommand.mockWith(name: "login", operationKey: "user foo / bar") + + // When + manager.process( + command, + context: mockContext, + writer: mockWriter, + activeView: .mockAny() + ) + + // Then + let vitalEvents = mockWriter.events(ofType: RUMVitalOperationStepEvent.self) + XCTAssertEqual(vitalEvents.count, 1) + } + func testProcess_OperationWithInvalidOperationKey_DoesNotCreateVitalEvent() { // Given for invalidOpKey in invalidNames { diff --git a/TestUtilities/Sources/Mocks/DatadogRUM/RUMFeatureMocks.swift b/TestUtilities/Sources/Mocks/DatadogRUM/RUMFeatureMocks.swift index 4fc3504e0f..c88998fa50 100644 --- a/TestUtilities/Sources/Mocks/DatadogRUM/RUMFeatureMocks.swift +++ b/TestUtilities/Sources/Mocks/DatadogRUM/RUMFeatureMocks.swift @@ -931,7 +931,10 @@ extension RUMOperationStepVitalCommand: AnyMockable, RandomMockable { public static func mockRandom() -> RUMOperationStepVitalCommand { return mockWith( vitalId: .mockRandom(), - name: .mockRandom(), + // vital.name must match the schema facet-path character set + // (letters, digits, - _ . @ $). Use alphanumerics to avoid + // generating whitespace and tripping the API-boundary validator. + name: .mockRandom(among: .alphanumerics), operationKey: .mockRandom(), stepType: .mockRandom(), failureReason: .mockRandom(),