Regression test for THRIFT-4002: Immutable exception deserialization#3349
Conversation
Client: py Patch: Jens Geyer Generated-by: Opencode big-pickle This test verifies that immutable structs (including exceptions, which are immutable by default since Thrift 0.14.0) can be properly deserialized without triggering the __setattr__ TypeError. The bug manifests when: 1. A struct class is marked immutable (has __setattr__ that raises TypeError) 2. Thrift's deserialization tries to set attributes via setattr instead of using the kwargs constructor Test coverage: - Immutable exception creation and hashability - Immutable exception blocks modification/deletion - Round-trip serialization/deserialization with TBinaryProtocol - Round-trip serialization/deserialization with TCompactProtocol - Accelerated protocol tests (C extension) when available Related: THRIFT-4002, THRIFT-5715
Jens-G
force-pushed
the
THRIFT-4002-immutable-exception-deserialization
branch
from
db99552 to
2dc4dbb
Compare
| from thrift.protocol import TBinaryProtocol, TCompactProtocol | ||
|
|
||
|
|
||
| class ImmutableException(TException): |
There was a problem hiding this comment.
Hi @Jens-G - i just bumped into this when making a patch I was going to submit (will do shortly).
However, the docstring here says:
Test exception that mimics generated immutable exception behavior.
but if we run codegen to create an immutable exception,
gen_dynbaseclass_frozen_exc_ = "TFrozenExceptionBase";
would mean this would gen to something like
class ImmutableException(TFrozenExceptionBase):
do you think this test might be OK changing to that?
There was a problem hiding this comment.
+1 Agreed.
I ran into the issue myself at some unrelated occasion, which gave me the idea to include a regression test. However, the current implementation has certainly room for improvement.
There was a problem hiding this comment.
fwiw i thought about changing this then reverted that based on my findings here
#3377 (comment)
2 participants
Client: py
Patch: Jens Geyer
Generated-by: Opencode big-pickle
This test verifies that immutable structs (including exceptions, which are immutable by default since Thrift 0.14.0) can be properly deserialized without triggering the setattr TypeError.
The bug manifests when:
Test coverage:
Related: THRIFT-4002, THRIFT-5715