Justin Mills created THRIFT-3781:
------------------------------------
Summary: [ruby] Thrift::Client can become permanently corrupt when
Thrift::ProtocolException is raised
Key: THRIFT-3781
URL: https://issues.apache.org/jira/browse/THRIFT-3781
Project: Thrift
Issue Type: Bug
Components: Ruby - Library
Affects Versions: 0.9.3
Reporter: Justin Mills
{{Thrift::Client#send_message_args}} rescues from {{StandardError}}, but
{{Thrift::ProtocolException}} inherits from {{Exception}}. This means if you
have an argument to a function that is not valid (ie, missing a required
attribute), you can get the following error:
{code}
Thrift::ProtocolException: Required field address is unset!
from
~/.rvm/gems/ruby-2.3.0/gems/my-thrift-gem-1.0/lib/my-thrift-gem/gen-rb/my-thrift-gem_types.rb:32:in
`validate'
from
~/.rvm/gems/ruby-2.3.0/gems/thrift-0.9.3.0/lib/thrift/client.rb:44:in `write'
from
~/.rvm/gems/ruby-2.3.0/gems/thrift-0.9.3.0/lib/thrift/client.rb:44:in
`send_message_args'
from
~/.rvm/gems/ruby-2.3.0/gems/thrift-0.9.3.0/lib/thrift/client.rb:30:in
`send_message'
from
~/.rvm/gems/ruby-2.3.0/gems/my-thrift-gem-1.0/lib/my-thrift-gem/gen-rb/my_service.rb:22:in
`send_example_1'
from
~/.rvm/gems/ruby-2.3.0/gems/my-thrift-gem-1.0/lib/my-thrift-gem/gen-rb/my_service.rb:17:in
`example_1'
{code}
Since ProtocolException is not a StandardError, the exception handler is not
called, and the {{@oprot.trans.close}} line is not executed, leaving the
transport with data in it's buffers. All subsequent calls will fail that try to
re-use this client because of this data.
It is also possible that this problem only occurs when the validation error
happens in the 2nd and beyond arguments to a function, as only then is data
likely to have been written.
I can see a couple of potential fixes:
- Make Thrift::ProtocolException inherit from StandardError
- Change the rescue block in client.rb to rescue from all Thrift exceptions
that do not inherit from StandardError
- Have the client validate each parameter before attempting to serialize
anything, ensuring that the data is more likely to serialize successfully.
>From a ruby perspective, I think the first of these is the best fix as I'm not
>sure a ProtocolException is something that warrants subclassing directly from
>Exception. It seems more aligned to IOError. Unless the thought here is that
>ProtocolExceptions are reserved for cases where the client should be abandoned
>and re-built. If that's the case, I think the last of the options might be
>best since data validation could be done prior to serialization and need not
>waste the effort of serialization, nor burn the client connection because of
>it.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
