read_struct_end does not restore last_read_field_id, corrupting field ids decoded after nested structs
- Dominant language
- Rust
- Stars
- 167
- Forks
- 25
- PR merge metrics
- No merged PRs in 30d
Description
## Bug Report
### Version
```
pilota 0.13.1
```
### Platform
```
Linux x86_64
```
### Crates
- `pilota` — `pilota/src/thrift/compact.rs`, affects both `TCompactInputProtocol` and `TAsyncCompactProtocol`
### Description
In pilota 0.13.1, the Thrift compact protocol reader does not restore the outer struct's field-id base when a nested struct ends. `read_struct_begin` pushes the current `last_read_field_id` onto a stack and resets it, but `read_struct_end` is a no-op (`Ok(())`): it neither pops the stack nor writes the popped value back — unlike the write side, whose `write_struct_end` pops and restores `last_write_field_id`. Since compact protocol encodes field ids as deltas against `last_read_field_id`, any field decoded after a nested struct gets a wrong id, which then triggers a decode error.
I tried this code (write `Outer { 1:"a"(string), 3:"b"(string), 5:[Inner{1:i32, 2:i32}], 6:"c"(string) }`, then read it back):
```rust
let mut trans = BytesMut::new();
let mut o = test_output_prot_bytesmut(&mut trans);
// outer: field1 string, field3 string, field5 list, field6 string
o.write_struct_begin(&TStructIdentifier { name: "Outer" }).unwrap();
o.write_field_begin(TType::Binary, 1).unwrap();
o.write_string("a").unwrap();
o.write_field_end().unwrap();
o.write_field_begin(TType::Binary, 3).unwrap();
o.write_string("b").unwrap();
o.write_field_end().unwrap();
o.write_field_begin(TType::List, 5).unwrap();
o.write_list_begin(TListIdentifier { element_type: TType::Struct, size: 1 }).unwrap();
o.write_struct_begin(&TStructIdentifier { name: "Inner" }).unwrap();
o.write_field_begin(TType::I32, 1).unwrap();
o.write_i32(1).unwrap();
o.write_field_end().unwrap();
o.write_field_begin(TType::I32, 2).unwrap();
o.write_i32(2).unwrap();
o.write_field_end().unwrap();
o.write_field_stop().unwrap();
o.write_struct_end().unwrap();
o.write_list_end().unwrap();
o.write_field_end().unwrap();
// regression point: field AFTER nested struct
o.write_field_begin(TType::Binary, 6).unwrap();
o.write_string("c").unwrap();
o.write_field_end().unwrap();
o.write_field_stop().unwrap();
o.write_struct_end().unwrap();
let mut bytes = Bytes::copy_from_slice(&trans[..]);
let mut input = test_input_prot_bytes(&mut bytes);
input.read_struct_begin().unwrap();
assert_eq!(input.read_field_begin().unwrap().id, Some(1));
assert_eq!(input.read_string().unwrap(), "a");
input.read_field_end().unwrap();
assert_eq!(input.read_field_begin().unwrap().id, Some(3));
assert_eq!(input.read_string().unwrap(), "b");
input.read_field_end().unwrap();
assert_eq!(input.read_field_begin().unwrap().id, Some(5));
input.read_list_begin().unwrap();
input.read_struct_begin().unwrap();
input.read_field_begin().unwrap();
input.read_i32().unwrap();
input.read_field_end().unwrap();
input.read_field_begin().unwrap();
input.read_i32().unwrap();
input.read_field_end().unwrap();
input.read_field_begin().unwrap(); // STOP
input.read_struct_end().unwrap();
input.read_list_end().unwrap();
input.read_field_end().unwrap();
let f = input.read_field_begin().unwrap(); // BUG: returns Some(3) on unpatched 0.13.1
assert_eq!(f.id, Some(6));
assert_eq!(input.read_string().unwrap(), "c");
```
I expected to see this happen: the field after the nested struct is decoded with its real id `6`, and decoding completes normally.
Instead, this happened: `read_field_begin` returns `Some(3)` — computed from the stale `last_read_field_id = 2` plus the compact delta `1` . Verified locally: the test above fails on unpatched pilota 0.13.1 with `left: Some(3), right: Some(6)`.
Contributor guide
Research direction
Start in pilota/src/thrift/compact.rs at read_struct_begin and read_struct_end, then compare both TCompactInputProtocol and TAsyncCompactProtocol with the write-side struct handling. Reproduce the nested-struct round trip from the issue and add coverage for the field after the nested struct. Done means that field decodes as id 6 and both readers complete normally.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- api
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 82/100