[
https://issues.apache.org/jira/browse/THRIFT-5300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17224972#comment-17224972
]
Allen George edited comment on THRIFT-5300 at 11/2/20, 9:37 PM:
----------------------------------------------------------------
[~shuoli84] and [~ulidtko]: I just checked the spec again (it's been a long
time since I looked at the code and the spec) and I noticed that the spec _is_
correct as well. It clearly points out that the field encoding in structs is
different from the field encoding in lists/sets. I've taken screenshots of the
spec and attached them to the ticket. The values in the rust implementation are
correct.
So:
* There is no bug in the spec.
* There is no bug in the implementation.
I'll also point out that the rust client/server (like most other language
implementations) does cross-language tests. There are definitely bugs (see the
list of explicitly-disabled cross-tests) in the implementation, but this
particular bug should have triggered failures across the board. Separately, I
would have checked other implementations to make sure that the rust one was
consistent (the spec is not exhaustive, and one definitely has to make some
judgement calls in some cases.)
It doesn't seem like there's any action to be taken here at all.
was (Author: allengeorge):
[~shuoli84] and [~ulidtko]: I just checked the spec again (it's been a long
time since I looked at the code and the spec) and I noticed that the spec _is_
correct as well. It clearly points out that the field encoding in structs is
different from the field encoding in lists/sets. I've taken screenshots of the
spec and attached them to the ticket. The values in the rust implementation are
correct.
So:
* There is no bug in the spec.
* There is no bug in the implementation.
I'll also point out that the rust client/server (like most other language
implementations) does cross-language tests. There are definitely bugs (see the
list of explicitly-disabled cross-tests) in the implementation, but this should
have triggered failures across the board.
It doesn't seem like there's any action to be taken here at all.
> rs compact protocol collection elem type to ttype mapping wrong
> ---------------------------------------------------------------
>
> Key: THRIFT-5300
> URL: https://issues.apache.org/jira/browse/THRIFT-5300
> Project: Thrift
> Issue Type: Bug
> Components: Rust - Library
> Affects Versions: 0.13.0
> Reporter: shuo li
> Assignee: Allen George
> Priority: Major
> Attachments: Screen Shot 2020-11-02 at 16.31.09.png, Screen Shot
> 2020-11-02 at 16.31.31.png
>
>
> collection_u8_to_type only overrides bool, but in spec, other types are
> different too.
> In field:
> * {{BOOLEAN_TRUE}}, encoded as {{1}}
> * {{BOOLEAN_FALSE}}, encoded as {{2}}
> * {{BYTE}}, encoded as {{3}}
> * {color:#FF0000}{{I16}}, encoded as {{4}}{color}
> * {color:#FF0000}{{I32}}, encoded as {{5}}{color}
> * {{I64}}, encoded as {{6}}
> * {{DOUBLE}}, encoded as {{7}}
> * {{BINARY}}, used for binary and string fields, encoded as {{8}}
> * {{LIST}}, encoded as {{9}}
> * {{SET}}, encoded as {{10}}
> * {{MAP}}, encoded as {{11}}
> * {{STRUCT}}, used for both structs and union fields, encoded as {{12}}
> In colleciton:
> * {{BOOL}}, encoded as {{2}}
> * {{BYTE}}, encoded as {{3}}
> * {{DOUBLE}}, encoded as {{4}}
> * {color:#FF0000}{{I16}}, encoded as {{6}}{color}
> * {color:#FF0000}{{I32}}, encoded as {{8}}{color}
> * {{I64}}, encoded as {{10}}
> * {{STRING}}, used for binary and string fields, encoded as {{11}}
> * {{STRUCT}}, used for structs and union fields, encoded as {{12}}
> * {{MAP}}, encoded as {{13}}
> * {{SET}}, encoded as {{14}}
> * {{LIST}}, encoded as {{15}}
> {code:java}
> // code placeholder
> fn collection_u8_to_type(b: u8) -> crate::Result<TType> {
> match b {
> 0x01 => Ok(TType::Bool),
> o => u8_to_type(o),
> }
> }
> fn u8_to_type(b: u8) -> crate::Result<TType> {
> match b {
> 0x00 => Ok(TType::Stop),
> 0x03 => Ok(TType::I08), // equivalent to TType::Byte
> 0x04 => Ok(TType::I16),
> 0x05 => Ok(TType::I32),
> 0x06 => Ok(TType::I64),
> 0x07 => Ok(TType::Double),
> 0x08 => Ok(TType::String),
> 0x09 => Ok(TType::List),
> 0x0A => Ok(TType::Set),
> 0x0B => Ok(TType::Map),
> 0x0C => Ok(TType::Struct),
> unkn => Err(crate::Error::Protocol(crate::ProtocolError {
> kind: crate::ProtocolErrorKind::InvalidData, message:
> format!("cannot convert {} into TType", unkn),
> })),
> }}
> {code}
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
