[
https://issues.apache.org/jira/browse/THRIFT-4781?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17280862#comment-17280862
]
Andrej Nazarov commented on THRIFT-4781:
----------------------------------------
Hi [~kashirin.alex]. Thank you for suggestion. I no longer use Thrift. Last
time I tried I could not get it to build from source locally on a Windows
machine at home - one of the dependencies did not compile (I believe some
headers libevent, but not sure, it was a year ago). I wasted quite a bit of
time to see why it fails, to get it to build, but didn't get anywhere so I gave
up in the end. Unless the build from source experience for Thrift on Windows
has vastly improved, I cannot try the fix suggested. In the JIRA description I
have provided the IDL necessary to reproduce the problem - it is very simple:
1) typedef an exception type
2) define and implement a service that throws it in the server side code
3) try to deserialize it on C++ client side.
If someone can build Thrift locally, they are free to try your suggested fix.
> C++ clients crash when exceptions are typedefed in the IDL
> ----------------------------------------------------------
>
> Key: THRIFT-4781
> URL: https://issues.apache.org/jira/browse/THRIFT-4781
> Project: Thrift
> Issue Type: Bug
> Components: C++ - Compiler
> Affects Versions: 0.11.0, 0.12.0
> Reporter: Andrej Nazarov
> Priority: Major
> Attachments: image-2020-05-06-20-15-44-986.png,
> image-2020-05-06-21-37-46-809.png
>
>
> If exceptions are typedefed in the IDL, they're generated as pointers in Cpp.
> This causes a runtime crash (memory access violation) on the C++ client-side
> when a server sends that exception and the client tries to read it. Example
> follows:
> {code:java|title=service.thrift}
> namespace * thrifttest.service
> include "errors.thrift"
> typedef errors.FooError FooError
> service FooBarService
> {
> string getFooString(1: i32 stringLength) throws (1: FooError e);
> string getBarString(1: i32 stringLength) throws (1: errors.BarError e);
> }
> {code}
> {code:java|title=errors.thrift}
> namespace * thrifttest.errors
> exception FooError {
> 1: string message
> }
> exception BarError {
> 1: string message
> }
> {code}
> {code:java|title=FooBarService.h}
> class FooBarService_getFooString_presult {
> public:
> virtual ~FooBarService_getFooString_presult() throw();
> std::string* success;
> FooError* e; //note pointer declaration of the exception field
> // snip...
> class FooBarService_getBarString_presult {
> public:
> virtual ~FooBarService_getBarString_presult() throw();
> std::string* success;
> ::thrifttest::errors::BarError e; //note different declaration of the
> exception field
> //snip
> {code}
> {code:java|title=FooBarService.cpp}
> uint32_t
> FooBarService_getFooString_presult::read(::apache::thrift::protocol::TProtocol*
> iprot) {
> // snip...
> while (true)
> {
> // snip...
> switch (fid)
> {
> // snip...
> case 1:
> if (ftype == ::apache::thrift::protocol::T_STRUCT) {
> xfer += (*(this->e)).read(iprot); // <-- this line causes access
> violation crash because the pointer is not initialized
> this->__isset.e = true;
> // snip...
> uint32_t
> FooBarService_getBarString_presult::read(::apache::thrift::protocol::TProtocol*
> iprot) {
> // snip...
> while (true)
> {
> // snip...
> switch (fid)
> {
> // snip...
> case 1:
> if (ftype == ::apache::thrift::protocol::T_STRUCT) {
> xfer += this->e.read(iprot); //<-- this gets read OK.
> this->__isset.e = true;
> //snip
> {code}
> This happens regardless of server language (reproducible if server throwing
> the exceptions is Java, Python or C++)
> I guess this logic in
> [t_cpp_generator.cc:1104|https://github.com/apache/thrift/blob/0.11.0/compiler/cpp/src/thrift/generate/t_cpp_generator.cc#L1104]
> gets deceived in case of typedefed exceptions:
> {code:java|title=t_cpp_generator.cc}
> (pointers && !(*m_iter)->get_type()->is_xception()),
> {code}
> I'm no Thrift compiler expert, but I assume there is a reason why you don't
> want exceptions to be declared as pointers. Yet in this case they clearly are.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
