feat(ai): add full JSON \uXXXX decoding with UTF-8 and surrogate pair support #126

Manually merged

jabber.developer merged 2 commits from fix/ai-json-encoding into master 2026-05-19 19:15:15 +00:00

Conversation 1 Commits 2 Files Changed 2 +121 -17

jabber.developer commented 2026-05-19 16:45:24 +00:00

Owner

Copy Link

Summary

Implements proper JSON Unicode escape (\uXXXX) decoding in the AI client, including UTF-8 encoding and UTF-16 surrogate pair handling. Also tightens the /ai start command to require an explicit provider name.

Changes

src/ai/ai_client.c

• Added _hex4_to_int() — converts 4-hex-digit sequences to 16-bit code points
• Added _write_utf8() — encodes a Unicode code point as UTF-8 bytes
• Extended _json_unescape_substring() to handle:
  • Single \uXXXX sequences → UTF-8 encoded characters
  • Surrogate pairs (\uHHHH\uLLLL) → characters outside the BMP (U+10000–U+10FFFF)
  • Invalid/orphan surrogates → Unicode replacement character (U+FFFD)
• Buffer allocation increased to accommodate expanded UTF-8 output (up to 6 bytes per \uXXXX)
• Added g_realloc() to shrink output buffer to exact size

src/command/cmd_funcs.c

• Removed automatic fallback to default provider from preferences for /ai start
• Now requires explicit provider argument; shows error if omitted
• Model resolution order preserved: explicit > provider default > hardcoded fallback

Motivation

Previously, \uXXXX JSON escapes were passed through verbatim, resulting in garbled output for non-ASCII characters (e.g., emojis, CJK, mathematical symbols). This change ensures proper Unicode round-tripping through the AI pipeline.

The provider default removal in /ai start prevents silent use of an unexpected provider when the user's default may have changed or been misconfigured.

## Summary Implements proper JSON Unicode escape (`\uXXXX`) decoding in the AI client, including UTF-8 encoding and UTF-16 surrogate pair handling. Also tightens the `/ai start` command to require an explicit provider name. ## Changes ### `src/ai/ai_client.c` - Added `_hex4_to_int()` — converts 4-hex-digit sequences to 16-bit code points - Added `_write_utf8()` — encodes a Unicode code point as UTF-8 bytes - Extended `_json_unescape_substring()` to handle: - Single `\uXXXX` sequences → UTF-8 encoded characters - Surrogate pairs (`\uHHHH\uLLLL`) → characters outside the BMP (U+10000–U+10FFFF) - Invalid/orphan surrogates → Unicode replacement character (U+FFFD) - Buffer allocation increased to accommodate expanded UTF-8 output (up to 6 bytes per `\uXXXX`) - Added `g_realloc()` to shrink output buffer to exact size ### `src/command/cmd_funcs.c` - Removed automatic fallback to default provider from preferences for `/ai start` - Now requires explicit provider argument; shows error if omitted - Model resolution order preserved: explicit > provider default > hardcoded fallback ## Motivation Previously, `\uXXXX` JSON escapes were passed through verbatim, resulting in garbled output for non-ASCII characters (e.g., emojis, CJK, mathematical symbols). This change ensures proper Unicode round-tripping through the AI pipeline. The provider default removal in `/ai start` prevents silent use of an unexpected provider when the user's default may have changed or been misconfigured.

jabber.developer added 2 commits 2026-05-19 16:45:25 +00:00

fix(ai): properly decode \uXXXX JSON escape sequences as UTF-8 ... 4776c1f1ec

Add helper functions to parse hex digits and encode UTF-8 characters. Update buffer allocation to account for UTF-8 expansion and implement full surrogate pair support for characters outside the BMP. Previously, \uXXXX sequences were passed through verbatim; they are now correctly decoded into proper UTF-8 strings.

fix(ai): resolve UAF and require provider argument instead of defaults ... 53cdf488b6

This enforces explicit provider specification.
Automatic fallback to preferences or "openai" is removed.
Users must now pass the provider name explicitly.

default provider functionality was improperly removed and implemented (some parts still remain intact), leading to UAF.

jabber.developer2 commented 2026-05-19 18:30:33 +00:00

Collaborator

Copy Link

Probably worth addressing in this PR

1. UAF fix may be doing more than fixing the UAF

src/command/cmd_funcs.c:10932-10941. The original UAF looks real to me — auto_gchar gchar* default_provider was scoped to an inner if block, so cleanup ran at block exit while provider_name still held a pointer into it. The PR's resolution is to drop the fallback to PREF_AI_PROVIDER and the "openai" default, which fixes the lifetime issue but also removes a feature. That may have been intentional, but I can't tell from the commit body — it reads as a bugfix while behaving as a breaking change.

There's already an unused auto_gchar gchar* owned_provider_name in scope; assigning into it would extend the lifetime across the function and seemingly fix the UAF without touching the fallback path:

auto_gchar gchar* owned_provider_name = NULL;
if (!provider_name) {
    owned_provider_name = prefs_get_string(PREF_AI_PROVIDER);
    if (owned_provider_name && *owned_provider_name)
        provider_name = owned_provider_name;
}
if (!provider_name) provider_name = "openai";

Maybe author considered this and rejected it for a reason I can't see — worth a quick clarification either way.

2. Help / examples may be inconsistent with new behaviour

src/command/cmd_defs.c:2811 still shows "/ai start [<provider>] [<model>]" with the optional brackets, and src/command/cmd_defs.c:2841 lists "/ai start" as a valid example. With the new mandatory-arg behaviour, that example seems like it would now produce the "Please specify provider name." error. If the removal in #1 stays, these probably want updating; if #1 reverts to keeping the fallback, they're fine as-is.

Probably minor

3. Surrogate-pair else branch may be effectively dead code

src/ai/ai_client.c:325-339. The outer guard reads in[2] != '\\' || in[3] != 'u' — when false, it would mean the hex digits of \uXXXX start with \u, which I think can only happen on malformed input. If I'm reading this right, even when the branch is entered, _hex4_to_int(in + 2) returns −1 (since in[2] == '\\' isn't a hex digit), so the surrogate-pair logic after that point shouldn't actually run. The valid-JSON surrogate-pair path looks like it's handled inside the first branch's nested else (the surrogate-range check on cp).

If that reading is correct, the outer else can probably be dropped and the case simplified. Not urgent — no observable misbehaviour, just confusing on read.

## Probably worth addressing in this PR ### 1. UAF fix may be doing more than fixing the UAF [src/command/cmd_funcs.c:10932-10941](src/command/cmd_funcs.c#L10932-L10941). The original UAF looks real to me — `auto_gchar gchar* default_provider` was scoped to an inner `if` block, so cleanup ran at block exit while `provider_name` still held a pointer into it. The PR's resolution is to drop the fallback to `PREF_AI_PROVIDER` and the `"openai"` default, which fixes the lifetime issue but also removes a feature. That may have been intentional, but I can't tell from the commit body — it reads as a bugfix while behaving as a breaking change. There's already an unused `auto_gchar gchar* owned_provider_name` in scope; assigning into it would extend the lifetime across the function and seemingly fix the UAF without touching the fallback path: ```c auto_gchar gchar* owned_provider_name = NULL; if (!provider_name) { owned_provider_name = prefs_get_string(PREF_AI_PROVIDER); if (owned_provider_name && *owned_provider_name) provider_name = owned_provider_name; } if (!provider_name) provider_name = "openai"; ``` Maybe author considered this and rejected it for a reason I can't see — worth a quick clarification either way. ### 2. Help / examples may be inconsistent with new behaviour [src/command/cmd_defs.c:2811](src/command/cmd_defs.c#L2811) still shows `"/ai start [<provider>] [<model>]"` with the optional brackets, and [src/command/cmd_defs.c:2841](src/command/cmd_defs.c#L2841) lists `"/ai start"` as a valid example. With the new mandatory-arg behaviour, that example seems like it would now produce the `"Please specify provider name."` error. If the removal in #1 stays, these probably want updating; if #1 reverts to keeping the fallback, they're fine as-is. ## Probably minor ### 3. Surrogate-pair `else` branch may be effectively dead code [src/ai/ai_client.c:325-339](src/ai/ai_client.c#L325-L339). The outer guard reads `in[2] != '\\' || in[3] != 'u'` — when false, it would mean the hex digits of `\uXXXX` start with `\u`, which I think can only happen on malformed input. If I'm reading this right, even when the branch is entered, `_hex4_to_int(in + 2)` returns −1 (since `in[2] == '\\'` isn't a hex digit), so the surrogate-pair logic after that point shouldn't actually run. The valid-JSON surrogate-pair path looks like it's handled inside the first branch's nested `else` (the surrogate-range check on `cp`). If that reading is correct, the outer else can probably be dropped and the case simplified. Not urgent — no observable misbehaviour, just confusing on read.

jabber.developer manually merged commit 53cdf488b6 into master 2026-05-19 19:15:15 +00:00

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

Compat/Breaking

Breaking change that won't be backward compatible

Kind/Bug

Something is not working

Kind/Documentation

Documentation changes

Kind/Enhancement

Improve existing functionality

Kind/Feature

New functionality

Kind/Security

This is security issue

Kind/Testing

Issue or pull request related to testing

Priority

Critical

1

The priority is critical

Priority

High

2

The priority is high

Priority

Low

4

The priority is low

Priority

Medium

3

The priority is medium

Reviewed

Confirmed

1

Issue has been confirmed

Reviewed

Duplicate

2

This issue or pull request already exists

Reviewed

Invalid

3

Invalid issue

Reviewed

Won't Fix

3

This issue won't be fixed

Status

Abandoned

3

Somebody has started to work on this but abandoned work

Status

Blocked

1

Something is blocking this issue or pull request

Status

Need More Info

2

Feedback is required to reproduce issue or to continue work

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

admin jabber.developer jabber.developer2

No Assignees

2 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: devs/cproof#126

No description provided.