Thriftpy / thriftpy2

Pure python approach of Apache Thrift.
MIT License
567 stars 90 forks source link

patch binary type to string in all container types #199

Closed ethe closed 1 year ago

chenseanxy commented 1 year ago

Hi, is there expectation as to when this commit will see a release? Tkx

aisk commented 1 year ago

Hi, is there expectation as to when this commit will see a release? Tkx

There is a beta version with this patch: https://pypi.org/project/thriftpy2/0.4.16b0/

SuperElephant commented 1 year ago

Hi, is there expectation as to when this commit will see a release? Tkx

There is a beta version with this patch: https://pypi.org/project/thriftpy2/0.4.16b0/

It should be considered a serious bug, from my point of view 0.4.15 should be banned. During the serialization of structure, type 18 leaks into the binary. Since it is not a standard field-type in Thrift Binary protocol encoding, it will trigger undefined behavior in other tools (in my case, the cpp server get an empty req)

eg.

req = Req(kvs={'key??': '1111111111111111111', 'key12??': '2222222222222222222'})

for the following structure

// idl
struct Req {
    1: map<string, binary> kvs,
}

the binary value

binary value 0.4.14: 0d 0001 0b 0b 000000010000001333383933323239393230373539373036303737...
binery value 0.4.15: 0d 0001 0b 12 000000010000001333383933323239393230373539373036303737...
                     |  |   | |value type <binary> -> 11 ? 18
                     |  |   |key type <string> -> 11
                     |  |field id -> 1
                     |data type <map> -> 13
ethe commented 1 year ago

@SuperElephant thanks reporting, so have you been tested v0.4.16b0 in your case?

SuperElephant commented 1 year ago

@SuperElephant thanks reporting, so have you been tested v0.4.16b0 in your case?

yes, v0.4.16b0 is good