ADBM-3574: Fix JSON support for pgbackrest recovery filters by VoidZeroNull0 · Pull Request #113 · arenadata/pgbackrest

VoidZeroNull0 · 2026-10-28T12:36:25Z

What happened?
Restore failed when a --filter value contained a "\uXXXX" escape
outside the ASCII range - e.g. a surrogate pair like "\uD83E\uDD52".
The parser only supported "\u00XX" and threw on anything else.

How do we fix this?
Decode \uXXXX escapes into UTF-8, including surrogate pairs: combine
high/low surrogates into one code point and encode it as 1–4 UTF-8
bytes. Extracted the logic into jsonDecodeUnicode() function.

Tests?
One success test for the surrogate-pair path was added. One test per
error branch was also added.

Ticket: ADBM-3574

Try the first solution.

This test now passes, it is not needed.

Was wrong this->json++.

Get back this->json++, add this->json--.

Make a separate function, add tests for error branches.

VoidZeroNull0 · 2026-10-29T14:21:47Z

Tests seem to fail because of missing docker image. Nothing related to the patch.

RekGRpth · 2026-10-30T03:02:25Z

+    else if (codepoint <= 0x7FF)
+    {
+        strCatChr(result, (char)(0xC0 | (codepoint >> 6)));
+        strCatChr(result, (char)(0x80 | (codepoint & 0x3F)));
+    }
+    else if (codepoint <= 0xFFFF)
+    {
+        strCatChr(result, (char)(0xE0 | (codepoint >> 12)));
+        strCatChr(result, (char)(0x80 | ((codepoint >> 6) & 0x3F)));
+        strCatChr(result, (char)(0x80 | (codepoint & 0x3F)));
+    }
+    else
+    {
+        strCatChr(result, (char)(0xF0 | (codepoint >> 18)));
+        strCatChr(result, (char)(0x80 | ((codepoint >> 12) & 0x3F)));
+        strCatChr(result, (char)(0x80 | ((codepoint >> 6) & 0x3F)));
+        strCatChr(result, (char)(0x80 | (codepoint & 0x3F)));
+    }


these lines are not covered by tests

This is precisely why the CI is red: it checks the coverage of the ENTIRE codebase.

RekGRpth · 2026-10-30T03:04:05Z

+// Unicode escape
+FN_EXTERN void jsonDecodeUnicode(JsonRead *this, String *result);
+
 // Convert JSON to Variant


Why did you make this function public?

RekGRpth · 2026-10-30T03:05:36Z

+FN_EXTERN void
+jsonDecodeUnicode(JsonRead *const this, String *const result)
+{
+    this->json++; // get past /u


add FUNCTION_TEST_BEGIN/PARAM/RETURN and ASSERT which CODING.md requires

RekGRpth · 2026-10-30T03:07:05Z

-                    strCatChr(result, (char)cvtZSubNToUIntBase(this->json, 0, 2, 16));
-                    this->json++;
-
+                    jsonDecodeUnicode(this, result);


Now jsonReadStr does not validate the input data, and jsonDecodeUnicode does not check it either.

RekGRpth · 2026-10-30T03:23:02Z

Restore failed when a --filter value contained a "\uXXXX" escape
outside the ASCII range - e.g. a surrogate pair like "\uD83E\uDD52".

There is no test for the original bug: \uD83E\uDD52 via jsonReadSkip/jsonValidate, and in partialRestoreTest.c.

RekGRpth · 2026-10-30T03:24:04Z

+/**********************************************************************************************************************************/
+FN_EXTERN void
+jsonDecodeUnicode(JsonRead *const this, String *const result)
+{
+    this->json++; // get past /u
+    const unsigned int high = cvtZSubNToUIntBase(this->json, 0, 4, 16);
+    unsigned int codepoint = high;
+    this->json += 4;
+
+    // Check for supplementary planes
+    if (high >= 0xD800 && high <= 0xDBFF)
+    {
+        if (*this->json != '\\' || *(this->json + 1) != 'u')
+            THROW_FMT(JsonFormatError, "missing low surrogate for high surrogate at: %s", this->json - 6);
+
+        this->json += 2;
+        const unsigned int low = cvtZSubNToUIntBase(this->json, 0, 4, 16);
+        this->json += 4;
+
+        if (low < 0xDC00 || low > 0xDFFF)
+            THROW_FMT(JsonFormatError, "invalid low surrogate in unicode escape at: %s", this->json - 6);
+
+        codepoint = 0x10000 + ((high - 0xD800) << 10) + (low - 0xDC00);
+    }
+    else if (high >= 0xDC00 && high <= 0xDFFF)
+        THROW_FMT(JsonFormatError, "unexpected low surrogate without high surrogate at: %s", this->json - 4);
+
+    // Append 1-4 bytes to the result
+    if (codepoint <= 0x7F)
+    {
+        strCatChr(result, (char)codepoint);
+    }
+    else if (codepoint <= 0x7FF)
+    {
+        strCatChr(result, (char)(0xC0 | (codepoint >> 6)));
+        strCatChr(result, (char)(0x80 | (codepoint & 0x3F)));
+    }
+    else if (codepoint <= 0xFFFF)
+    {
+        strCatChr(result, (char)(0xE0 | (codepoint >> 12)));
+        strCatChr(result, (char)(0x80 | ((codepoint >> 6) & 0x3F)));
+        strCatChr(result, (char)(0x80 | (codepoint & 0x3F)));
+    }
+    else
+    {
+        strCatChr(result, (char)(0xF0 | (codepoint >> 18)));
+        strCatChr(result, (char)(0x80 | ((codepoint >> 12) & 0x3F)));
+        strCatChr(result, (char)(0x80 | ((codepoint >> 6) & 0x3F)));
+        strCatChr(result, (char)(0x80 | (codepoint & 0x3F)));
+    }
+
+    this->json--;
+}
+


jsonDecodeUnicode() lacks validation for 4 hex digits (it reads past the \0 terminator, throws FormatError instead of JsonFormatError, and strtoull accepts extraneous input).

RekGRpth · 2026-10-30T03:25:19Z

                    for (; digitIdx < 4; digitIdx++)
                    {
-                        if (!isdigit(this->json[digitIdx]))
+                        if (!isxdigit(this->json[digitIdx]))


A \ before \0 in jsonReadSkipStr causes the pointer to move past the end of the string (fixed upstream in pgbackrest/pgbackrest@b623071a2).

RekGRpth · 2026-10-30T03:26:05Z

\u0000 results in a NUL byte in the string (while upstream now rejects this).

RekGRpth · 2026-10-30T03:29:48Z

+        codepoint = 0x10000 + ((high - 0xD800) << 10) + (low - 0xDC00);
+    }
+    else if (high >= 0xDC00 && high <= 0xDFFF)
+        THROW_FMT(JsonFormatError, "unexpected low surrogate without high surrogate at: %s", this->json - 4);


Suggested change

THROW_FMT(JsonFormatError, "unexpected low surrogate without high surrogate at: %s", this->json - 4);

THROW_FMT(JsonFormatError, "unexpected low surrogate without high surrogate at: %s", this->json - 6);

to add \u in error message like above

KnightMurloc · 2026-10-30T07:59:14Z

Do we really need to actually decode UTF-16 strings? During WAL filtering, we do not use text table names.

VoidZeroNull0 added 5 commits October 28, 2026 15:35

Initial patch

de82c8a

Try the first solution.

Remove test for unknown encoding

83aafa3

This test now passes, it is not needed.

Fix increment bug

1fbd4bb

Was wrong this->json++.

Fix increment bug again

c8b48a4

Get back this->json++, add this->json--.

Refactor and add tests

5f32798

Make a separate function, add tests for error branches.

VoidZeroNull0 marked this pull request as ready for review October 29, 2026 14:19

RekGRpth reviewed Oct 30, 2026

View reviewed changes

Navigation Menu

Repository navigation

ADBM-3574: Fix JSON support for pgbackrest recovery filters - #113

ADBM-3574: Fix JSON support for pgbackrest recovery filters#113
VoidZeroNull0 wants to merge 5 commits into
2.54-cifrom
ADBM-3574

VoidZeroNull0 commented Oct 28, 2026 •

edited

Loading

Uh oh!

VoidZeroNull0 commented Oct 29, 2026

Uh oh!

RekGRpth Oct 30, 2026

Uh oh!

RekGRpth Oct 30, 2026

Uh oh!

RekGRpth Oct 30, 2026

Uh oh!

RekGRpth Oct 30, 2026

Uh oh!

RekGRpth Oct 30, 2026

Uh oh!

RekGRpth commented Oct 30, 2026

Uh oh!

RekGRpth Oct 30, 2026

Uh oh!

RekGRpth Oct 30, 2026

Uh oh!

RekGRpth commented Oct 30, 2026

Uh oh!

RekGRpth Oct 30, 2026

Uh oh!

KnightMurloc commented Oct 30, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	THROW_FMT(JsonFormatError, "unexpected low surrogate without high surrogate at: %s", this->json - 4);
	THROW_FMT(JsonFormatError, "unexpected low surrogate without high surrogate at: %s", this->json - 6);

Conversation

VoidZeroNull0 commented Oct 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

VoidZeroNull0 commented Oct 29, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RekGRpth commented Oct 30, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RekGRpth commented Oct 30, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KnightMurloc commented Oct 30, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

VoidZeroNull0 commented Oct 28, 2026 •

edited

Loading