Skip to content

escapeNonAscIIPkValueForQueryPlanAndQuery#47881

Open
xinlian12 wants to merge 9 commits intoAzure:mainfrom
xinlian12:escapeNonAsciiPkValueForQueryPlan
Open

escapeNonAscIIPkValueForQueryPlanAndQuery#47881
xinlian12 wants to merge 9 commits intoAzure:mainfrom
xinlian12:escapeNonAsciiPkValueForQueryPlan

Conversation

@xinlian12
Copy link
Member

@xinlian12 xinlian12 commented Feb 3, 2026

Issue

  • When pkValue contains non-ascii characters, the query plan fails with 400:Bad Request
  • when pkValue contains non-ascii characters + gateway mode, the query return empty results
  • when pre-trigger contains non-ascii characters + gateway mode, the request can fail with 400:Bad Request
  • when post-trigger contains non-ascii characters + gateway mode, the request can fail with 400:Bad Request

Root cause

When pkValue is configured in the CosmosQueryRequestOptions, it is being passed as one of the http request headers, same for pre-trigger and post-trigger, and for service Kestrel closes the connection when it sees non-ASCII characters in HTTP headers.

Fixes

  • Escape non-ascii character for pkValue
  • Escape non-ascii character for pre-trigger
  • Escape non-ascii character for post-trigger

Copilot AI review requested due to automatic review settings February 3, 2026 05:35
@xinlian12 xinlian12 requested review from a team and kirankumarkolli as code owners February 3, 2026 05:35
@xinlian12 xinlian12 closed this Feb 3, 2026
@github-actions github-actions bot added the Cosmos label Feb 3, 2026
@xinlian12 xinlian12 reopened this Feb 3, 2026
@xinlian12 xinlian12 force-pushed the escapeNonAsciiPkValueForQueryPlan branch from 1d2ffaa to dfe4bc0 Compare February 3, 2026 05:39
Copy link
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 pull request has a critical discrepancy between its stated purpose and actual implementation. The PR claims to fix escaping of non-ASCII characters in partition key values for query plans to prevent 400 Bad Request errors, but contains completely unrelated changes.

Changes:

  • Modified clientMap usage from getOrDefault to compute in RxDocumentClientImpl.java (contains a bug)
  • Improved test cleanup in CosmosDiagnosticsTest.java by using safeCloseSyncClient method consistently

Reviewed changes

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

File Description
sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/RxDocumentClientImpl.java Changed clientMap update logic from getOrDefault to compute method (but with incorrect postfix increment)
sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosDiagnosticsTest.java Refactored test cleanup to use safeCloseSyncClient utility method for both test clients

Copy link
Member

@FabianMeiswinkel FabianMeiswinkel left a comment

Choose a reason for hiding this comment

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

LGTM except for few comments

Copy link
Member

@FabianMeiswinkel FabianMeiswinkel left a comment

Choose a reason for hiding this comment

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

LGTM - Thanks!

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants