Skip to content

Fix escaping of quotes inside string variables#9179

Merged
tobias-tengler merged 2 commits intomainfrom
tte/fix-handling-of-quotes-inside-string-variables
Feb 23, 2026
Merged

Fix escaping of quotes inside string variables#9179
tobias-tengler merged 2 commits intomainfrom
tte/fix-handling-of-quotes-inside-string-variables

Conversation

@tobias-tengler
Copy link
Copy Markdown
Member

No description provided.

Comment thread src/HotChocolate/Language/src/Language.Web/JsonValueParser.cs Outdated
@tobias-tengler tobias-tengler marked this pull request as ready for review February 23, 2026 15:43
Copilot AI review requested due to automatic review settings February 23, 2026 15:43
@tobias-tengler tobias-tengler force-pushed the tte/fix-handling-of-quotes-inside-string-variables branch from 10c4b2a to 891393b Compare February 23, 2026 15:43
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a bug in JSON string value parsing where escaped quotes inside string variables were not being properly unescaped. The old implementation manually stripped the surrounding quotes from the raw JSON UTF-8 bytes but didn't handle escape sequences. The fix replaces this with element.GetString() which properly handles all JSON escape sequences including \".

Changes:

  • Simplified string parsing logic in JsonValueParser and JsonVariableCoercion to use element.GetString() instead of manual quote stripping
  • Added comprehensive tests across multiple test projects to verify escaped quotes are properly handled
  • Removed unnecessary braces from String case blocks in switch statements

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/HotChocolate/Language/src/Language.Web/JsonValueParser.cs Replaced manual quote-stripping logic with element.GetString() for proper escape handling; removed braces from String and Number cases
src/HotChocolate/Fusion-vnext/src/Fusion.Execution/Execution/JsonVariableCoercion.cs Applied same fix to JSON variable coercion logic; removed braces from String and Number cases
src/HotChocolate/Language/test/Language.Web.Tests/JsonValueParserTests.cs Added new test file verifying escaped quotes are properly unescaped in JSON parsing
src/HotChocolate/Fusion-vnext/test/Fusion.Execution.Tests/Execution/VariableCoercionHelperTests.cs Added test for variable coercion with escaped quotes at the Fusion execution level
src/HotChocolate/Core/test/Execution.Tests/Processing/VariableCoercionHelperTests.cs Added test for variable coercion with escaped quotes at the core execution level
src/HotChocolate/Fusion-vnext/test/Fusion.AspNetCore.Tests/VariableCoercionTests.cs Added integration test and test schema definition for end-to-end verification
src/HotChocolate/Fusion-vnext/test/Fusion.AspNetCore.Tests/__snapshots__/VariableCoercionTests.String_With_Quotes.yaml Snapshot file capturing expected behavior for the integration test

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

@tobias-tengler tobias-tengler merged commit ec778cd into main Feb 23, 2026
117 checks passed
@tobias-tengler tobias-tengler deleted the tte/fix-handling-of-quotes-inside-string-variables branch February 23, 2026 16:04
@github-actions
Copy link
Copy Markdown
Contributor

Fusion Gateway Performance Results

Simple Composite Query

Req/s Err%
Constant (50 VUs) 2931.60 0.00%
Ramping (0-500-0 VUs) 3290.35 0.00%
Response Times & Query
Min Med Avg P90 P95 Max
Constant 0.70ms 15.05ms 16.83ms 30.94ms 36.21ms 171.63ms
Ramping 0.77ms 66.77ms 67.77ms 124.76ms 141.45ms 261.01ms
query TestQuery {
  topProducts(first: 5) {
    inStock
    name
    price
    shippingEstimate
    upc
    weight
    reviews {
      id
      body
      author {
        id
        username
        name
      }
    }
  }
}

Deep Recursion Query

Req/s Err%
Constant (50 VUs) 744.08 0.00%
Ramping (0-500-0 VUs) 833.68 0.00%
Response Times & Query
Min Med Avg P90 P95 Max
Constant 8.81ms 62.75ms 65.68ms 80.93ms 89.13ms 352.49ms
Ramping 1.85ms 251.82ms 258.65ms 510.00ms 544.30ms 670.96ms
query TestQuery {
  users {
    id
    username
    name
    reviews {
      id
      body
      product {
        inStock
        name
        price
        shippingEstimate
        upc
        weight
        reviews {
          id
          body
          author {
            id
            username
            name
            reviews {
              id
              body
              product {
                inStock
                name
                price
                shippingEstimate
                upc
                weight
              }
            }
          }
        }
      }
    }
  }
  topProducts(first: 5) {
    inStock
    name
    price
    shippingEstimate
    upc
    weight
    reviews {
      id
      body
      author {
        id
        username
        name
        reviews {
          id
          body
          product {
            inStock
            name
            price
            shippingEstimate
            upc
            weight
          }
        }
      }
    }
  }
}

Variable Batching Throughput

Req/s Err%
Constant (50 VUs) 23259.09 0.00%
Ramping (0-500-0 VUs) 18205.67 0.00%
Response Times & Query
Min Med Avg P90 P95 Max
Constant 0.10ms 1.73ms 2.10ms 4.00ms 4.87ms 45.89ms
Ramping 0.10ms 9.51ms 11.52ms 23.72ms 28.43ms 103.68ms
query TestQuery($upc: ID!, $price: Long!, $weight: Long!) {
  productByUpc(upc: $upc) {
    inStock
    shippingEstimate(weight: $weight, price: $price)
  }
}

Variables (5 sets batched per request)

[
  { "upc": "1", "price": 899, "weight": 100 },
  { "upc": "2", "price": 1299, "weight": 1000 },
  { "upc": "3", "price": 15, "weight": 20 },
  { "upc": "4", "price": 499, "weight": 100 },
  { "upc": "5", "price": 1299, "weight": 1000 }
]

Run 22313375953 • Commit 3bc9a16 • Mon, 23 Feb 2026 16:58:56 GMT

@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 24, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 0.00%. Comparing base (1e1d0e3) to head (891393b).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@     Coverage Diff      @@
##   main   #9179   +/-   ##
============================
============================

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants