xdebug: probe-full-json fix single quote escapes #12742

bneradt · 2025-12-09T23:37:00Z

The xdebug plugin was incorrectly escaping single quotes as ' in probe-full-json output, which is not a valid JSON escape sequence and broke JSON parsing for headers like Content-Security-Policy that contain CSP directives with single quotes (e.g., 'self', 'unsafe-inline'). This fix modifies the EscapeCharForJson class to only escape single quotes in legacy probe format (which uses single-quoted strings) while leaving them unescaped in full JSON mode, producing RFC 8259-compliant JSON output that can be piped directly to tools like jq.

The xdebug plugin was incorrectly escaping single quotes as \' in probe-full-json output, which is not a valid JSON escape sequence and broke JSON parsing for headers like Content-Security-Policy that contain CSP directives with single quotes (e.g., 'self', 'unsafe-inline'). This fix modifies the EscapeCharForJson class to only escape single quotes in legacy probe format (which uses single-quoted strings) while leaving them unescaped in full JSON mode, producing RFC 8259-compliant JSON output that can be piped directly to tools like jq.

bneradt · 2025-12-09T23:49:38Z

plugins/xdebug/xdebug_escape.cc

+  switch (c) {
+  case '"':
+    return {"\\\""};
+  case '\\':
+    return {"\\\\"};
+  case '\b':
+    return {"\\b"};
+  case '\f':
+    return {"\\f"};
+  case '\t':
+    return {"\\t"};
+  default:
+    return {&c, 1};
+  }
+}


This got moved to it's own file for unit testability reasons. But the only functional change in this patch is that \' is no longer escaped.

bneradt · 2025-12-09T23:50:15Z

plugins/xdebug/xdebug_headers.cc

-    case '\'':
-      return {"\\\'"};


This is not in the new switch (see the xdebug_escape.cc version).

Copilot

Pull request overview

This PR fixes a JSON compliance issue in the xdebug plugin's probe-full-json output where single quotes were incorrectly being escaped as \', which is not valid JSON according to RFC 8259. The fix ensures that single quotes in HTTP headers (like Content-Security-Policy directives containing 'self', 'unsafe-inline') are left unescaped in full JSON mode, producing valid JSON that can be piped to tools like jq.

Key Changes:

Removed single quote escaping logic from EscapeCharForJson class to produce RFC 8259-compliant JSON output
Refactored escaping code by moving EscapeCharForJson from xdebug_headers.cc to new xdebug_escape.h/cc files
Added comprehensive unit tests and integration tests to validate proper handling of single quotes in both legacy probe and full JSON modes

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
plugins/xdebug/xdebug_escape.h	New header file defining the refactored `EscapeCharForJson` class with proper documentation
plugins/xdebug/xdebug_escape.cc	Implementation of `EscapeCharForJson` without single quote escaping to ensure JSON compliance
plugins/xdebug/xdebug_headers.h	Updated to include new `xdebug_escape.h` header and removed local `FULL_JSON` constant
plugins/xdebug/xdebug_headers.cc	Removed `EscapeCharForJson` class definition (moved to separate files)
plugins/xdebug/unit_tests/test_xdebug_utils.cc	Added comprehensive unit tests for character escaping logic in both JSON and legacy modes
plugins/xdebug/CMakeLists.txt	Updated build configuration to include new `xdebug_escape.cc` source file
tests/gold_tests/pluginTest/xdebug/x_probe_full_json/x_probe_full_json.test.py	Added integration test for CSP headers with single quotes to validate JSON output
tests/gold_tests/pluginTest/xdebug/x_probe_full_json/x_probe_full_json.replay.yaml	Added test case for CSP header with single quotes and updated binary data test expectations
tests/gold_tests/pluginTest/xdebug/x_probe_full_json/gold/jq_hex.gold	Updated expected output for hex-encoded binary data test

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-15T20:10:44Z

tests/gold_tests/pluginTest/xdebug/x_probe_full_json/x_probe_full_json.replay.yaml

      content:
        encoding: plain
-        data: "Binary data with quotes!"
+        data: '"Binary data with quotes!"'


The binary data content has been changed from plain text to a quoted string, which alters the actual binary content being tested. This changes the hex encoding and may not accurately test the original binary data scenario. The quotes should be removed to preserve the original test intent.

Suggested change

data: '"Binary data with quotes!"'

data: Binary data with quotes!

Copilot · 2025-12-15T20:10:44Z

tests/gold_tests/pluginTest/xdebug/x_probe_full_json/x_probe_full_json.replay.yaml

      content:
        # Expect hex encoding: "Binary data with quotes!" -> hex
-        data: '42696e617279206461746120776974682071756f74657321'
+        data: '"2242696e617279206461746120776974682071756f7465732122",'


The expected hex output includes leading and trailing quote characters (22 hex = double quote) that were not in the original test data. The hex string should represent only 'Binary data with quotes!' without the added surrounding quotes, unless the source data at line 120 is corrected to remove its added quotes.

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-15T23:05:52Z

plugins/xdebug/xdebug_escape.cc

+  switch (c) {
+  case '"':
+    return {"\\\""};
+  case '\\':
+    return {"\\\\"};
+  case '\b':
+    return {"\\b"};
+  case '\f':
+    return {"\\f"};
+  case '\t':
+    return {"\\t"};
+  default:
+    return {&c, 1};
+  }


The removal of single quote escaping breaks the legacy probe format. The legacy format uses single-quoted strings (e.g., 'header' : 'value'), so single quotes within values must be escaped to prevent premature string termination. For example, a Content-Security-Policy header with value "default-src 'self'" would produce invalid output: 'content-security-policy' : 'default-src 'self'' where the inner 'self' breaks the string. Single quote escaping should be conditionally applied based on the _full_json flag: escape in legacy mode (!_full_json) but not in full JSON mode.

Copilot · 2025-12-15T23:05:52Z

plugins/xdebug/unit_tests/test_xdebug_utils.cc

+         R"('self')"},
+
+    {"legacy: CSP header with multiple single-quoted directives",
+         !xdebug::FULL_JSON,
+         R"(child-src blob: 'self'; connect-src 'self' 'unsafe-inline')",
+         R"(child-src blob: 'self'; connect-src 'self' 'unsafe-inline')"},


These test cases incorrectly expect single quotes to NOT be escaped in legacy mode. Since the legacy probe format uses single-quoted strings, single quotes within values must be escaped to prevent breaking the string delimiters. These tests should expect the single quotes to be escaped as ' in legacy mode (when full_json is false).

Suggested change

R"('self')"},

{"legacy: CSP header with multiple single-quoted directives",

!xdebug::FULL_JSON,

R"(child-src blob: 'self'; connect-src 'self' 'unsafe-inline')",

R"(child-src blob: 'self'; connect-src 'self' 'unsafe-inline')"},

R"(\'self\')"},

{"legacy: CSP header with multiple single-quoted directives",

!xdebug::FULL_JSON,

R"(child-src blob: 'self'; connect-src 'self' 'unsafe-inline')",

R"(child-src blob: \'self\'; connect-src \'self\' \'unsafe-inline\')"},

Copilot · 2025-12-15T23:05:53Z

plugins/xdebug/unit_tests/test_xdebug_utils.cc

+{
+  // clang-format off
+  static std::vector<EscapeTestCase> const tests = {
+    // Single quotes are NOT escaped in either mode.


This comment is incorrect and misleading. Single quotes should be escaped in legacy mode (which uses single-quoted strings) but not in full JSON mode (which uses double-quoted strings per RFC 8259). The comment should clarify that single quotes are escaped differently based on the format.

Suggested change

// Single quotes are NOT escaped in either mode.

// Single quotes are not escaped in either legacy or full JSON mode in this implementation.

// Note: Per RFC 8259, single quotes do not require escaping in full JSON mode (which uses double quotes).

// In legacy mode (which may use single-quoted strings), escaping may be required for strict compatibility,

// but this implementation does not escape them in either mode.

masaori335

Looks good.

bneradt added this to the 10.2.0 milestone Dec 9, 2025

bneradt self-assigned this Dec 9, 2025

bneradt added Bug xdebug xdebug plugin labels Dec 9, 2025

bneradt force-pushed the fix_x_debug_json_parsing branch from 0732363 to f51fa46 Compare December 9, 2025 23:48

bneradt commented Dec 9, 2025

View reviewed changes

bryancall requested a review from Copilot December 15, 2025 20:09

Copilot AI reviewed Dec 15, 2025

View reviewed changes

bryancall requested a review from Copilot December 15, 2025 22:55

Copilot started reviewing on behalf of bryancall December 15, 2025 22:56 View session

bryancall requested a review from masaori335 December 15, 2025 22:56

Copilot AI reviewed Dec 15, 2025

View reviewed changes

masaori335 approved these changes Dec 16, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

xdebug: probe-full-json fix single quote escapes #12742

xdebug: probe-full-json fix single quote escapes #12742

bneradt commented Dec 9, 2025

Uh oh!

bneradt Dec 9, 2025

Uh oh!

bneradt Dec 9, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 15, 2025

Uh oh!

Copilot AI Dec 15, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 15, 2025

Uh oh!

Copilot AI Dec 15, 2025

Uh oh!

Copilot AI Dec 15, 2025

Uh oh!

masaori335 left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	data: '"Binary data with quotes!"'
	data: Binary data with quotes!

-    // Single quotes are NOT escaped in either mode.
+    // Single quotes are not escaped in either legacy or full JSON mode in this implementation.
+    // Note: Per RFC 8259, single quotes do not require escaping in full JSON mode (which uses double quotes).
+    // In legacy mode (which may use single-quoted strings), escaping may be required for strict compatibility,
+    // but this implementation does not escape them in either mode.

xdebug: probe-full-json fix single quote escapes #12742

Are you sure you want to change the base?

xdebug: probe-full-json fix single quote escapes #12742

Conversation

bneradt commented Dec 9, 2025

Uh oh!

bneradt Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

bneradt Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

masaori335 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants