georust / georust/polyline

decode_polyline accepts a truncated varint and returns a wrong coordinate

Open Beginner friendly
#55 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
33
Forks
12
PR merge metrics
No merged PRs in 30d

Description

`decode_polyline` treats an input whose final coordinate varint is cut off at end of input as valid. When the last byte of a varint run still has its continuation bit set (value `>= 0x20` after subtracting 63) and no terminating byte follows, `decode_next` runs its `for` loop to the end of the byte iterator without ever reaching the `if byte < 0x20 { break; }` terminator. The loop then falls through and returns `Ok` with the partial `result` accumulated so far, so the caller gets a coordinate built from an incomplete varint. Tested against polyline 0.11.0, and the same `decode_next` is on main at the current tip.

## Reproducer

Encode a real point, then drop the last byte:

```rust
use geo_types::{Coord, LineString};
use polyline::{decode_polyline, encode_coordinates};

fn main() {
let ls = LineString(vec![Coord { x: -120.2, y: 38.5 }]);
let encoded = encode_coordinates(ls, 5).unwrap(); // "_p~iF~ps|U"
let truncated = &encoded[..encoded.len() - 1]; // "_p~iF~ps|"
println!("full {:?} -> {:?}", encoded, decode_polyline(&encoded, 5).unwrap().0);
println!("trunc {:?} -> {:?}", truncated, decode_polyline(truncated, 5));
}
```

The same result comes from a hand-built minimal string, decoded with no encode step:

```rust
use polyline::decode_polyline;

fn main() {
// "?_A": lat '?' = 0, lon '_' (continuation) + 'A' (terminator) -> lon 0.00032.
// "?_" drops the terminator, leaving the longitude varint cut off.
println!("{:?}", decode_polyline("?_A", 5).map(|l| l.0)); // Ok, lon 0.00032
println!("{:?}", decode_polyline("?_", 5).map(|l| l.0)); // Ok, lon 0.0
println!("{:?}", decode_polyline("?__", 5).map(|l| l.0)); // Ok, lon 0.0
}
```

## Observed

```
full "_p~iF~ps|U" -> [Coord { x: -120.2, y: 38.5 }]
trunc "_p~iF~ps|" -> Ok([Coord { x: -4.85664, y: 38.5 }])
```

The truncated string decodes to longitude `-4.85664` in place of `-120.2`, with no error.

## Expected

A polyline string that ends in the middle of a varint run is malformed. `decode_polyline` already returns `Err(PolylineError::DecodeError)` for a stray low byte and `Err(PolylineError::NoLongError)` when a longitude is missing, so a truncated varint fits the same handling and warrants an `Err`. Today it returns a coordinate assembled from incomplete bytes and gives the caller no signal that data was lost.

## Scope

A caller cannot tell a valid polyline from one that lost its final bytes in transit or storage. Decoding a truncated string succeeds and yields a coordinate that can sit far from the intended point (here the longitude moves by more than 100 degrees). Code that trusts the `Ok` path carries a silently corrupted position forward.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start by reading the `decode_next` function used by `decode_polyline`, then run the reproducer with the full and truncated strings. Add coverage for inputs such as `"?_"` and `"?__"`; done means a varint ending with its continuation bit set returns `Err(PolylineError::DecodeError)` instead of an incorrect coordinate, while existing valid decoding remains unchanged.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
data
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
76/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.