fix: sanitize non-UTF-8-encodable strings before JSON response serialization by mykola-mokhnach · Pull Request #1236 · appium/WebDriverAgent

mykola-mokhnach · 2026-08-28T06:30:35Z

FBResponseJSONPayload asserted before its UTF-8 fallback could run when NSJSONSerialization returned nil outright, and the fallback itself (fb_utf8SafeStringWithReplacement:) mishandled unpaired UTF-16 surrogates since canBeConvertedToEncoding:/dataUsingEncoding:allowLossyConversion: misreport for that case.

Fixes appium/appium#22673.

…ization FBResponseJSONPayload asserted before its UTF-8 fallback could run when NSJSONSerialization returned nil outright, and the fallback itself (fb_utf8SafeStringWithReplacement:) mishandled unpaired UTF-16 surrogates since canBeConvertedToEncoding:/dataUsingEncoding:allowLossyConversion: misreport for that case. Fixes appium/appium#22673. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Copilot

Pull request overview

This PR addresses JSON response serialization failures caused by strings containing unpaired UTF-16 surrogate code units (non-UTF-8-encodable), ensuring FBResponseJSONPayload can apply a sanitization fallback even when NSJSONSerialization returns nil outright.

Changes:

Update FBResponseJSONPayload to fall back to a UTF-8-safe dictionary when initial JSON serialization fails or produces non-UTF-8 data.
Rework fb_utf8SafeStringWithReplacement: to manually validate surrogate code units, replacing unpaired surrogates with U+FFFD.
Add/extend unit tests covering unpaired-surrogate sanitization and end-to-end response dispatch serialization, and wire the new test into the Xcode project.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
WebDriverAgentTests/UnitTests/NSDictionaryFBUtf8SafeTests.m	Adds coverage for unpaired surrogate sanitization and JSON serializability of sanitized output.
WebDriverAgentTests/UnitTests/FBResponseJSONPayloadTests.m	New tests verifying `FBResponseJSONPayload` dispatch sanitizes invalid strings (regression coverage for appium/appium#22673).
WebDriverAgentLib/Routing/FBResponseJSONPayload.m	Triggers sanitization fallback when JSON serialization returns `nil` or yields non-UTF-8 data.
WebDriverAgentLib/Categories/NSDictionary+FBUtf8SafeDictionary.m	Implements manual surrogate validation/replacement to correctly handle unpaired surrogates.
WebDriverAgent.xcodeproj/project.pbxproj	Adds the new unit test file to the test target sources.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

mykola-mokhnach · 2026-08-28T09:10:43Z

+  NSUInteger length = self.length;
+  NSMutableString *result = [NSMutableString stringWithCapacity:length];
  NSString *replacementStr = [NSString stringWithCharacters:&replacement length:1];


Fixed in d557b0a — the function is now single-pass and lazily allocates the mutable string/copy only when an unpaired surrogate is actually found, so surrogate-free strings (the common case) return self untouched.

mykola-mokhnach · 2026-08-28T09:10:47Z

+  if (nil == jsonData || nil == [[NSString alloc] initWithData:jsonData encoding:NSUTF8StringEncoding]) {
    [FBLogger log:@"The incoming data cannot be encoded to UTF-8 JSON. Applying lossy conversion as a workaround."];
    jsonData = [NSJSONSerialization dataWithJSONObject:[self.dictionary fb_utf8SafeDictionary]


Fixed in d557b0a — reworded to "JSON serialization failed or produced non-UTF-8 data. Applying lossy conversion as a workaround."

mykola-mokhnach · 2026-08-28T09:10:51Z

+  NSError *error;
+  NSDictionary *parsed = [NSJSONSerialization JSONObjectWithData:response.responseData
+                                                           options:0
+                                                             error:&error];
+  XCTAssertNil(error);


Fixed in d557b0a — initialized to nil.

mykola-mokhnach · 2026-08-28T09:10:52Z

+  NSError *error;
+  NSData *jsonData = [NSJSONSerialization dataWithJSONObject:safe
+                                                     options:0
+                                                       error:&error];
+  XCTAssertNotNil(jsonData, @"Sanitized dictionary must be serializable to JSON, error of %@", error);


Fixed in d557b0a — reworded to "JSON serialization of the sanitized dictionary unexpectedly failed: %@".

KazuCocoa · 2026-08-28T07:23:24Z

[P2] Invalid UTF-16 dictionary keys remain unsanitized

fb_utf8SafeDictionary rewrites only values and preserves every dictionary key unchanged:

WebDriverAgent/WebDriverAgentLib/Categories/NSDictionary+FBUtf8SafeDictionary.m

Lines 73 to 86 in 4552d2d

    
           - (instancetype)fb_utf8SafeDictionary 
        
           { 
        
             NSMutableDictionary *result = [self mutableCopy]; 
        
             for (id key in self) { 
        
               id value = result[key]; 
        
               if ([value isKindOfClass:NSString.class]) { 
        
                 result[key] = [(NSString *)value fb_utf8SafeStringWithReplacement:REPLACER]; 
        
               } else if ([value isKindOfClass:NSArray.class]) { 
        
                 result[key] = [(NSArray *)value fb_utf8SafeArray]; 
        
               } else if ([value isKindOfClass:NSDictionary.class]) { 
        
                 result[key] = [(NSDictionary *)value fb_utf8SafeDictionary]; 
        
               } 
        
             } 
        
             return result.copy;

If a response dictionary key contains an unpaired UTF-16 surrogate, both the initial serialization and the fallback serialization in FBResponseJSONPayload fail with NSCocoaErrorDomain code 3852, so execution still reaches the assertion. I reproduced this with a key built from {'k', 0xD800, 'y'}.

Please either sanitize NSString keys while rebuilding the dictionary (and define collision behavior when two keys sanitize to the same value), or explicitly validate/reject them. A response-dispatch regression test with an invalid key would cover this path.

KazuCocoa

lgtm entirely

…rings Addresses PR review feedback on #1236: fb_utf8SafeDictionary only sanitized values, leaving invalid keys to still crash serialization; fb_utf8SafeStringWithReplacement: now avoids allocating/copying for the common case of strings without surrogate code units; clarified the FBResponseJSONPayload log message and fixed uninitialized NSError locals in tests. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

mykola-mokhnach · 2026-08-28T09:10:39Z

Good catch — fixed in d557b0a. fb_utf8SafeDictionary now sanitizes NSString keys as well as values, rebuilding the dictionary rather than mutating a copy in place (since a sanitized key is a different dictionary slot than the original). Collision behavior is documented in a code comment: if two distinct keys sanitize to the same string, the later one wins, same as any other duplicate-key literal.

Added regression coverage for exactly the case you described — a key built from {'k', 0xD800, 'y'} — both at the fb_utf8SafeDictionary level and end-to-end through FBResponseJSONPayload dispatchWithResponse:.

## [16.9.2](v16.9.1...v16.9.2) (2026-08-28) ### Bug Fixes * cache the testmanagerd protocol version fallback on timeout ([#1228](#1228)) ([f3d8e0c](f3d8e0c)) * sanitize non-UTF-8-encodable strings before JSON response serialization ([#1236](#1236)) ([fa6a250](fa6a250)) ### Miscellaneous Chores * **deps:** bump @appium/strongbox from 1.1.3 to 2.0.0 ([#1237](#1237)) ([03db844](03db844))

github-actions · 2026-08-28T22:01:39Z

🎉 This PR is included in version 16.9.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

mykola-mokhnach requested review from KazuCocoa and eglitise August 28, 2026 06:30

KazuCocoa requested a lite review from Copilot August 28, 2026 06:46

Copilot started reviewing on behalf of KazuCocoa August 28, 2026 06:46 View session

Copilot AI reviewed Aug 28, 2026

View reviewed changes

KazuCocoa approved these changes Aug 28, 2026

View reviewed changes

mykola-mokhnach merged commit fa6a250 into master Aug 28, 2026
58 of 60 checks passed

mykola-mokhnach deleted the utf branch August 28, 2026 13:14

github-actions Bot added the released label Aug 28, 2026

Navigation Menu

Uh oh!

fix: sanitize non-UTF-8-encodable strings before JSON response serialization - #1236

fix: sanitize non-UTF-8-encodable strings before JSON response serialization#1236
mykola-mokhnach merged 2 commits into
masterfrom
utf

mykola-mokhnach commented Aug 28, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

mykola-mokhnach Aug 28, 2026

Uh oh!

mykola-mokhnach Aug 28, 2026

Uh oh!

mykola-mokhnach Aug 28, 2026

Uh oh!

mykola-mokhnach Aug 28, 2026

Uh oh!

KazuCocoa commented Aug 28, 2026

Uh oh!

KazuCocoa left a comment

Uh oh!

mykola-mokhnach commented Aug 28, 2026

Uh oh!

Uh oh!

github-actions Bot commented Aug 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

mykola-mokhnach commented Aug 28, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

mykola-mokhnach Aug 28, 2026

Choose a reason for hiding this comment

Uh oh!

mykola-mokhnach Aug 28, 2026

Choose a reason for hiding this comment

Uh oh!

mykola-mokhnach Aug 28, 2026

Choose a reason for hiding this comment

Uh oh!

mykola-mokhnach Aug 28, 2026

Choose a reason for hiding this comment

Uh oh!

KazuCocoa commented Aug 28, 2026

Uh oh!

KazuCocoa left a comment

Choose a reason for hiding this comment

Uh oh!

mykola-mokhnach commented Aug 28, 2026

Uh oh!

Uh oh!

github-actions Bot commented Aug 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants