Skip to content

Conversation

@nhpaine
Copy link
Contributor

@nhpaine nhpaine commented Jan 23, 2026

Add Configurable Warning Message Placement

Summary

This change adds a new configuration option InsertWarningAtProblemLocation that controls where warning messages are placed in sanitized strings. When enabled, warnings are inserted at the problem location (right before the detected pattern) instead of being prepended to the beginning of the string.

Motivation

Previously, all warning messages were prepended to the beginning of strings, which could make it difficult to understand the context of the detected issue. By inserting warnings at the exact location where the problem was detected, users get better context about what triggered the sanitizer.

Example Behavior

Input: "User accessed https://evil.com/secret"
Pattern detected: https://evil.com at index 14

Before (default behavior - InsertWarningAtProblemLocation = false):
"WARNING: aka.ms/ODSPSanitizerURL User accessed https://evil.com/secret"

After (when enabled - InsertWarningAtProblemLocation = true):
"User accessed WARNING: aka.ms/ODSPSanitizerURL https://evil.com/secret"

Implementation Details

Core C++ Changes

  • Added InsertWarningAtProblemLocation boolean field to SanitizerConfiguration (default: false)
  • Updated HandleWarningMessage to accept a matchIndex parameter indicating where the pattern was found
  • Created new overload of CreateWarningMessage(prefix, str, offset) alongside existing CreateWarningMessage(prefix, str)
  • The existing 2-parameter method remains unchanged for backwards compatibility
  • When InsertWarningAtProblemLocation == true and matchIndex > 0, the new 3-parameter overload is called

Cross-Platform Support

Configuration propagates through all platform layers:

  • Java/JNI: Updated SanitizerConfiguration.java, Sanitizer.java, and Sanitizer_jni.cpp
  • Objective-C: Updated ODWSanitizerInitConfig.h and .mm
  • Swift: Updated SanitizerInitConfig.swift wrapper

Edge Case Handling

  • offset == 0: Falls back to prepend behavior
  • offset >= string.length(): Falls back to prepend behavior
  • Empty string or prefix: Returns appropriate value
  • Delimiter searches: Always use index 0, triggering prepend behavior

Testing

Added comprehensive unit tests:

  • SanitizerStringUtilsTests.cpp: 9 parameterized tests covering normal cases and edge cases
  • SanitizerProviderTests.cpp: 6 integration tests verifying configuration flow and backwards compatibility

Backwards Compatibility

✅ Fully backwards compatible - default value is false, maintaining existing prepend behavior
✅ Existing code that doesn't set this configuration field continues to work unchanged
✅ No breaking changes to any public APIs

Technical Notes

  • C++11 compatible (uses std::memcpy, resize(), pointer arithmetic)
  • Zero extra allocations in the new overload
  • Consistent performance with existing implementation
  • Works correctly with multi-byte offsets from trie pattern matching

Files Modified

Core C++ (5 files)

  • lib/modules/sanitizer/SanitizerConfiguration.hpp
  • lib/modules/sanitizer/SanitizerProvider.hpp
  • lib/modules/sanitizer/SanitizerProvider.cpp
  • lib/modules/sanitizer/SanitizerStringUtils.hpp
  • lib/modules/sanitizer/SanitizerStringUtils.cpp

Platform Wrappers (6 files)

  • lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/SanitizerConfiguration.java
  • lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/Sanitizer.java
  • lib/jni/Sanitizer_jni.cpp
  • wrappers/obj-c/ODWSanitizerInitConfig.h
  • wrappers/obj-c/ODWSanitizerInitConfig.mm
  • wrappers/swift/Sources/OneDSSwift/SanitizerInitConfig.swift

Tests (2 files)

  • lib/modules/sanitizer/tests/unittests/SanitizerStringUtilsTests.cpp
  • lib/modules/sanitizer/tests/unittests/SanitizerProviderTests.cpp

@nhpaine nhpaine requested a review from a team as a code owner January 23, 2026 22:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants