[ 
https://issues.apache.org/jira/browse/THRIFT-4781?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Andrej Nazarov updated THRIFT-4781:
-----------------------------------
    Description: 
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.

  was:
If exceptions are typedefed in the IDL, they're generated as pointers in C++.  
*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.


> 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
>
> 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
(v7.6.3#76005)

Reply via email to