[
https://issues.apache.org/jira/browse/THRIFT-6369?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
arian updated THRIFT-6369:
--------------------------
Description:
THRIFT-5885 (fixed in 0.23.0) added a generated {_}{{_}}setattr{{_}}{_} to
mutable structs with py:enum. It converts the raw i32 that fastbinary's
_fast_decode sets into the enum member. The fix only handles fields whose
declared type is the enum itself. With TBinaryProtocolAcceleratedFactory /
TCompactProtocolAcceleratedFactory, three cases still decode incorrectly:
# Enum field declared through a typedef (mutable struct). In t_py_generator.cc
the check is (*m_iter){-}>get_type(){-}>is_enum(), which is false for a
t_typedef. No {_}{{_}}setattr{{_}}{_} is generated, and the field stays a raw
int. The pure-Python read() resolves the typedef correctly
(Color(iprot.readI32())), so the two paths give different results.
# Enum values inside containers (mutable struct). The generated
{_}{{_}}setattr{{_}}{_} only converts scalar enum fields. Elements of
list<Enum>, set<Enum> and map<…, Enum> stay raw ints.
# Immutable structs (python.immutable). Here {_}fast_decode calls the
generated __init{_}_, which still contains the original THRIFT-5885 bug:
{code:java}
super().__setattr__('color', color if hasattr(color, 'value') else
Color.__members__.get(color)){code}
"__members__" is keyed by name, so an int gives None.
*Steps to reproduce*
repro.thrift
{code:java}
namespace py repro
enum Color {
RED = 1,
GREEN = 2,
}
typedef Color ColorAlias
// control case: fixed by THRIFT-5885
struct MutableDirect {
1: required Color color,
}
// case 1: typedef'd enum
struct MutableTypedef {
1: required ColorAlias color,
}
// case 2: enum inside a container
struct MutableList {
1: required list<Color> colors,
}
// case 3: immutable struct
struct ImmutableDirect {
1: required Color color,
} (python.immutable = "")
{code}
Generate
{code:java}
thrift --gen py:enum repro.thrift{code}
repro.py
{code:java}
import sys
sys.path.insert(0, "gen-py")
from thrift.protocol import TBinaryProtocol, TJSONProtocol
from thrift.transport import TTransport
from repro.ttypes import Color, ImmutableDirect, MutableDirect, MutableList,
MutableTypedef
ACCELERATED = TBinaryProtocol.TBinaryProtocolAcceleratedFactory()
PLAIN = TBinaryProtocol.TBinaryProtocolFactory()
JSON = TJSONProtocol.TJSONProtocolFactory()
def serialize(obj, factory):
buf = TTransport.TMemoryBuffer()
obj.write(factory.getProtocol(buf))
return buf.getvalue()
def deserialize(cls, data, factory):
protocol = factory.getProtocol(TTransport.TMemoryBuffer(data))
if isinstance(cls.__dict__.get("read"), classmethod): # python.immutable
return cls.read(protocol)
obj = cls()
obj.read(protocol)
return obj
assert ACCELERATED.getProtocol(TTransport.TMemoryBuffer(b""))._fast_decode is
not None, (
"fastbinary C extension is not available"
)
cases = [
("MutableDirect ", MutableDirect(color=Color.GREEN), lambda o: o.color),
("MutableTypedef ", MutableTypedef(color=Color.GREEN), lambda o: o.color),
("MutableList ", MutableList(colors=[Color.RED, Color.GREEN]), lambda o:
o.colors),
("ImmutableDirect", ImmutableDirect(color=Color.GREEN), lambda o: o.color),
]
for label, obj, get in cases:
data = serialize(obj, PLAIN)
plain_value = get(deserialize(type(obj), data, PLAIN))
fast_obj = deserialize(type(obj), data, ACCELERATED)
fast_value = get(fast_obj)
try:
serialize(fast_obj, JSON)
reserialize = "ok"
except Exception as e:
reserialize = f"{type(e).__name__}: {e}"
print(f"{label} plain={plain_value!r:<35} accelerated={fast_value!r:<20}
json re-serialize: {reserialize}")
{code}
Result
{code:java}
MutableDirect plain=<Color.GREEN: 2> accelerated=<Color.GREEN: 2> json
re-serialize: ok
MutableTypedef plain=<Color.GREEN: 2> accelerated=2 json
re-serialize: AttributeError: 'int' object has no attribute 'value'
MutableList plain=[<Color.RED: 1>, <Color.GREEN: 2>] accelerated=[1, 2]
json re-serialize: AttributeError: 'int' object has no attribute 'value'
ImmutableDirect plain=<Color.GREEN: 2> accelerated=None json
re-serialize: TProtocolException: Required field color is unset!{code}
was:
THRIFT-5885 (fixed in 0.23.0) added a generated {_}{{_}}setattr{{_}}{_} to
mutable structs with py:enum. It converts the raw i32 that fastbinary's
_fast_decode sets into the enum member. The fix only handles fields whose
declared type is the enum itself. With TBinaryProtocolAcceleratedFactory /
TCompactProtocolAcceleratedFactory, three cases still decode incorrectly:
# Enum field declared through a typedef (mutable struct). In t_py_generator.cc
the check is (*m_iter){-}>get_type(){-}>is_enum(), which is false for a
t_typedef. No {_}{{_}}setattr{{_}}{_} is generated, and the field stays a raw
int. The pure-Python read() resolves the typedef correctly
(Color(iprot.readI32())), so the two paths give different results.
# Enum values inside containers (mutable struct). The generated
{_}{{_}}setattr{{_}}{_} only converts scalar enum fields. Elements of
list<Enum>, set<Enum> and map<…, Enum> stay raw ints.
# Immutable structs (python.immutable). Here {_}fast_decode calls the
generated __init{_}_, which still contains the original THRIFT-5885 bug:
{code:java}
super().__setattr__('color', color if hasattr(color, 'value') else
Color.__members__.get(color)){code}
{_}__{_}members__ is keyed by name, so an int gives None.
*Steps to reproduce*
repro.thrift
{code:java}
namespace py repro
enum Color {
RED = 1,
GREEN = 2,
}
typedef Color ColorAlias
// control case: fixed by THRIFT-5885
struct MutableDirect {
1: required Color color,
}
// case 1: typedef'd enum
struct MutableTypedef {
1: required ColorAlias color,
}
// case 2: enum inside a container
struct MutableList {
1: required list<Color> colors,
}
// case 3: immutable struct
struct ImmutableDirect {
1: required Color color,
} (python.immutable = "")
{code}
Generate
{code:java}
thrift --gen py:enum repro.thrift{code}
repro.py
{code:java}
import sys
sys.path.insert(0, "gen-py")
from thrift.protocol import TBinaryProtocol, TJSONProtocol
from thrift.transport import TTransport
from repro.ttypes import Color, ImmutableDirect, MutableDirect, MutableList,
MutableTypedef
ACCELERATED = TBinaryProtocol.TBinaryProtocolAcceleratedFactory()
PLAIN = TBinaryProtocol.TBinaryProtocolFactory()
JSON = TJSONProtocol.TJSONProtocolFactory()
def serialize(obj, factory):
buf = TTransport.TMemoryBuffer()
obj.write(factory.getProtocol(buf))
return buf.getvalue()
def deserialize(cls, data, factory):
protocol = factory.getProtocol(TTransport.TMemoryBuffer(data))
if isinstance(cls.__dict__.get("read"), classmethod): # python.immutable
return cls.read(protocol)
obj = cls()
obj.read(protocol)
return obj
assert ACCELERATED.getProtocol(TTransport.TMemoryBuffer(b""))._fast_decode is
not None, (
"fastbinary C extension is not available"
)
cases = [
("MutableDirect ", MutableDirect(color=Color.GREEN), lambda o: o.color),
("MutableTypedef ", MutableTypedef(color=Color.GREEN), lambda o: o.color),
("MutableList ", MutableList(colors=[Color.RED, Color.GREEN]), lambda o:
o.colors),
("ImmutableDirect", ImmutableDirect(color=Color.GREEN), lambda o: o.color),
]
for label, obj, get in cases:
data = serialize(obj, PLAIN)
plain_value = get(deserialize(type(obj), data, PLAIN))
fast_obj = deserialize(type(obj), data, ACCELERATED)
fast_value = get(fast_obj)
try:
serialize(fast_obj, JSON)
reserialize = "ok"
except Exception as e:
reserialize = f"{type(e).__name__}: {e}"
print(f"{label} plain={plain_value!r:<35} accelerated={fast_value!r:<20}
json re-serialize: {reserialize}")
{code}
Result
{code:java}
MutableDirect plain=<Color.GREEN: 2> accelerated=<Color.GREEN: 2> json
re-serialize: ok
MutableTypedef plain=<Color.GREEN: 2> accelerated=2 json
re-serialize: AttributeError: 'int' object has no attribute 'value'
MutableList plain=[<Color.RED: 1>, <Color.GREEN: 2>] accelerated=[1, 2]
json re-serialize: AttributeError: 'int' object has no attribute 'value'
ImmutableDirect plain=<Color.GREEN: 2> accelerated=None json
re-serialize: TProtocolException: Required field color is unset!{code}
> py:enum + accelerated protocols: THRIFT-5885 fix misses typedef'd enums,
> enums in containers, and immutable structs
> -------------------------------------------------------------------------------------------------------------------
>
> Key: THRIFT-6369
> URL: https://issues.apache.org/jira/browse/THRIFT-6369
> Project: Thrift
> Issue Type: Bug
> Components: Python - Compiler
> Affects Versions: 0.24.0
> Environment: Thrift compiler 0.24.0 (--gen py:enum)
> Python library thrift==0.24.0, with the fastbinary C extension built
> Python 3.11+
> Linux and Windows
> Reporter: arian
> Priority: Major
>
> THRIFT-5885 (fixed in 0.23.0) added a generated {_}{{_}}setattr{{_}}{_} to
> mutable structs with py:enum. It converts the raw i32 that fastbinary's
> _fast_decode sets into the enum member. The fix only handles fields whose
> declared type is the enum itself. With TBinaryProtocolAcceleratedFactory /
> TCompactProtocolAcceleratedFactory, three cases still decode incorrectly:
> # Enum field declared through a typedef (mutable struct). In
> t_py_generator.cc the check is (*m_iter){-}>get_type(){-}>is_enum(), which is
> false for a t_typedef. No {_}{{_}}setattr{{_}}{_} is generated, and the field
> stays a raw int. The pure-Python read() resolves the typedef correctly
> (Color(iprot.readI32())), so the two paths give different results.
> # Enum values inside containers (mutable struct). The generated
> {_}{{_}}setattr{{_}}{_} only converts scalar enum fields. Elements of
> list<Enum>, set<Enum> and map<…, Enum> stay raw ints.
> # Immutable structs (python.immutable). Here {_}fast_decode calls the
> generated __init{_}_, which still contains the original THRIFT-5885 bug:
> {code:java}
> super().__setattr__('color', color if hasattr(color, 'value') else
> Color.__members__.get(color)){code}
> "__members__" is keyed by name, so an int gives None.
> *Steps to reproduce*
> repro.thrift
> {code:java}
> namespace py repro
> enum Color {
> RED = 1,
> GREEN = 2,
> }
> typedef Color ColorAlias
> // control case: fixed by THRIFT-5885
> struct MutableDirect {
> 1: required Color color,
> }
> // case 1: typedef'd enum
> struct MutableTypedef {
> 1: required ColorAlias color,
> }
> // case 2: enum inside a container
> struct MutableList {
> 1: required list<Color> colors,
> }
> // case 3: immutable struct
> struct ImmutableDirect {
> 1: required Color color,
> } (python.immutable = "")
> {code}
> Generate
> {code:java}
> thrift --gen py:enum repro.thrift{code}
> repro.py
> {code:java}
> import sys
> sys.path.insert(0, "gen-py")
> from thrift.protocol import TBinaryProtocol, TJSONProtocol
> from thrift.transport import TTransport
> from repro.ttypes import Color, ImmutableDirect, MutableDirect, MutableList,
> MutableTypedef
> ACCELERATED = TBinaryProtocol.TBinaryProtocolAcceleratedFactory()
> PLAIN = TBinaryProtocol.TBinaryProtocolFactory()
> JSON = TJSONProtocol.TJSONProtocolFactory()
> def serialize(obj, factory):
> buf = TTransport.TMemoryBuffer()
> obj.write(factory.getProtocol(buf))
> return buf.getvalue()
> def deserialize(cls, data, factory):
> protocol = factory.getProtocol(TTransport.TMemoryBuffer(data))
> if isinstance(cls.__dict__.get("read"), classmethod): # python.immutable
> return cls.read(protocol)
> obj = cls()
> obj.read(protocol)
> return obj
> assert ACCELERATED.getProtocol(TTransport.TMemoryBuffer(b""))._fast_decode is
> not None, (
> "fastbinary C extension is not available"
> )
> cases = [
> ("MutableDirect ", MutableDirect(color=Color.GREEN), lambda o: o.color),
> ("MutableTypedef ", MutableTypedef(color=Color.GREEN), lambda o: o.color),
> ("MutableList ", MutableList(colors=[Color.RED, Color.GREEN]), lambda
> o: o.colors),
> ("ImmutableDirect", ImmutableDirect(color=Color.GREEN), lambda o:
> o.color),
> ]
> for label, obj, get in cases:
> data = serialize(obj, PLAIN)
> plain_value = get(deserialize(type(obj), data, PLAIN))
> fast_obj = deserialize(type(obj), data, ACCELERATED)
> fast_value = get(fast_obj)
> try:
> serialize(fast_obj, JSON)
> reserialize = "ok"
> except Exception as e:
> reserialize = f"{type(e).__name__}: {e}"
> print(f"{label} plain={plain_value!r:<35} accelerated={fast_value!r:<20}
> json re-serialize: {reserialize}")
> {code}
> Result
> {code:java}
> MutableDirect plain=<Color.GREEN: 2> accelerated=<Color.GREEN: 2> json
> re-serialize: ok
> MutableTypedef plain=<Color.GREEN: 2> accelerated=2 json
> re-serialize: AttributeError: 'int' object has no attribute 'value'
> MutableList plain=[<Color.RED: 1>, <Color.GREEN: 2>] accelerated=[1, 2]
> json re-serialize: AttributeError: 'int' object has no attribute 'value'
> ImmutableDirect plain=<Color.GREEN: 2> accelerated=None json
> re-serialize: TProtocolException: Required field color is unset!{code}
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
