Azure / Azure/azure-sdk-for-rust
Continuation-token EPK bounds are parsed leniently
- 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
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