[
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:36 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 should
have triggered failures across the board.
It doesn't seem like there's any action to be taken here at all.
was (Author: allengeorge):
[~shuoli84][~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)