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
@ThomsonTan
Copy link
Contributor

Is modules set the the latest commit in submodule? The new commit hash b48f76d8284b5785572569134db181bccf856d75 seems not correct.

@nhpaine nhpaine force-pushed the user/npaine/AddWarningAtProblemIndex branch from eafbb90 to 5f75378 Compare January 28, 2026 00:15
Updates lib/modules submodule pointer to reference the merged commit
b34f1c412 from PR #317 on master branch.

The core C++ changes have been reviewed and merged into the submodule.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@nhpaine nhpaine force-pushed the user/npaine/AddWarningAtProblemIndex branch from 5f75378 to ef9550f Compare January 28, 2026 00:18
@ThomsonTan ThomsonTan merged commit 3f160f9 into main Jan 28, 2026
17 of 27 checks passed
@ThomsonTan ThomsonTan deleted the user/npaine/AddWarningAtProblemIndex branch January 28, 2026 17:51
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.

4 participants