Azure / Azure/azure-sdk-for-rust

Continuation-token EPK bounds are parsed leniently

Open
#4,980 0 comments 1 reaction 0 assignees View on GitHub
Client Cosmos
Dominant language
Rust
Stars
884
Forks
365
Avg merge
2d 19h
Merged PRs (30d)
109

Description

### Bug Title

Continuation-token EPK range bounds are parsed leniently, so a corrupted token can silently change resumed coverage

### Crate Name

`azure_data_cosmos_driver`

### Crate Version

`0.7.0`

### Description

`EffectivePartitionKey::from(&str)` is infallible and therefore deliberately lenient: `hex_to_bytes` (`src/models/effective_partition_key.rs`) `break`s at the first pair that fails to parse, and `chunks_exact(2)` silently discards an odd trailing nibble. That is the right behavior for the call sites it was designed for — service pkrange bounds, query-plan ranges, and the `"…FF"` sentinel are always well-formed even-length hex.

It is *not* the right behavior for continuation tokens, which are round-tripped through the caller and can arrive truncated, re-encoded, or hand-edited. Three continuation-token validators currently parse their saved EPK bounds through this lenient path:

| Site | Introduced by |
| --- | --- |
| `driver/dataflow/planner.rs` — `validate_streaming_order_by_snapshot` | #4800 |
| `driver/dataflow/planner.rs` — `validate_saved_snapshot` | #4550 |
| `driver/dataflow/planner.rs` — `validate_unordered_merge_tokens` (change feed) | #4621 |

A bound corrupted from `4080` to `40G0` parses as `40`, widening the resumed range; truncation of a `max` bound narrows it. Nothing downstream catches this. The validators check that `min < max`, that entries are sorted, and that they do not overlap — but they never check that the saved ranges still tile the query scope (they legitimately shrink as ranges drain), and the streaming ORDER BY query fingerprint covers query text, parameters, and feed scope but not the saved range list. The result is a silently wrong resume — duplicated or omitted rows — instead of an error.

A strict parser with exactly the required semantics already exists in the same module: `try_hex_to_bytes` rejects both non-hex characters and odd lengths. It is private and currently used only by `PartialEq`.

Suggested fix: expose it as an inherent `EffectivePartitionKey::try_from_hex` and route all three validators through it, rejecting malformed bounds with the continuation-token status each validator already uses (`CLIENT_CONTINUATION_TOKEN_ORDER_BY_STATE_INVALID` for streaming ORDER BY, `CLIENT_CONTINUATION_TOKEN_INVALID_EPK_RANGE` for the other two).

Note that this cannot be a `TryFrom<&str>` impl: std's blanket `impl> TryFrom for T` conflicts with the existing `From<&str>`. It has to be an inherent method.

Non-token call sites should keep using `From`, which is correct for them.

Originally raised in review on #4800: https://github.com/Azure/azure-sdk-for-rust/pull/4800#discussion_r3714691028

### Steps to Reproduce

1. Start a cross-partition query that produces a continuation token whose saved range bounds have more than one hex byte, e.g. a range of `["4080", "8000")`.
2. Decode the continuation token and corrupt one hex character of a `minEpk`, e.g. `4080` -> `40G0`. (Equivalently, truncate the string to an odd length: `408`.)
3. Pass the modified token back to resume the query.
4. Expected: the resume fails with a continuation-token validation error.
Actual: the bound silently parses as `40`, the resumed range is wider than the one that was saved, and rows already returned before the pause are emitted a second time.

A unit-level reproduction is simpler — call the affected validator directly with an `OrderByRangeToken { min_epk: "40G0".to_owned(), max_epk: "80".to_owned(), .. }` and observe that it is accepted as the range `[40, 80)` rather than rejected.

### Checklist

- [x] Follow our [Code of Conduct](https://github.com/Azure/azure-sdk-for-rust/blob/main/CODE_OF_CONDUCT.md)
- [x] Check that there isn't already an issue that request the same bug to avoid creating a duplicate.
- [x] The provided reproduction is a [minimal reproducible example](https://stackoverflow.com/help/minimal-reproducible-example) of the bug.

Contributor guide

Open the contributing guide

Research direction

Start in src/models/effective_partition_key.rs by comparing EffectivePartitionKey::from with the existing try_hex_to_bytes behavior. Then inspect the three validators in driver/dataflow/planner.rs and their continuation-token status errors, using the OrderByRangeToken reproduction with malformed bounds. Done means malformed or odd-length saved EPK bounds are rejected by all three validators while non-token parsing remains unchanged.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
databases
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.