alazzaretti opened a new pull request, #3344:
URL: https://github.com/apache/thrift/pull/3344
Client: rs
`skip_till_depth()` calls `read_string()` for unknown TType::String fields,
which performs a String::from_utf8() check. This **fails** on binary fields and
throws an error, instead of gracefully skipping the field. This happens because
string and binary share the same wire type. The expected behavior would be to
skip an unknown binary field if an older client consumes a thrift with a new
optional binary field.
The bug triggers any time a newer schema adds a binary field to a struct
that an older binary is still deserializing — the older binary hits the skip()
path for the unknown field and fails (throws a ProtocolError) when it tries to
skip the field.
<!-- Explain the changes in the pull request below: -->
One line change. Inside skip_till_depth(). Replace:
`TType::String => self.read_string().map(|_| ()) `
with
`TType::String => self.read_bytes().map(|_| ()) `
Also added few tests validating new behavior
The patch only affects fields that will be discarded, so it does not affect
validation for known string fields. read_string() is just read_bytes() +
String::from_utf8(), so both consume identical wire bytes (4-byte length + N
bytes).
<!--
The Contributing Guide at:
https://github.com/apache/thrift/blob/master/CONTRIBUTING.md
has more details and tips for committing properly.
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
