You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by "Yuxuan Wang (Jira)" <ji...@apache.org> on 2022/08/05 21:43:00 UTC

[jira] [Created] (THRIFT-5609) TJSONProtocol and TSimpleJSONProtocol are unsafe to be used with TDeserializerPool and TSerializerPool

Yuxuan Wang created THRIFT-5609:
-----------------------------------

             Summary: TJSONProtocol and TSimpleJSONProtocol are unsafe to be used with TDeserializerPool and TSerializerPool
                 Key: THRIFT-5609
                 URL: https://issues.apache.org/jira/browse/THRIFT-5609
             Project: Thrift
          Issue Type: Bug
          Components: Go - Library
    Affects Versions: 0.16.0
            Reporter: Yuxuan Wang


We just realized that when using TJSONProtocol with TDeserializerPool like this:

{code:go}
var deserializerPool = thrift.NewTDeserializerPoolSizeFactory(1024, thrift.NewTJSONProtocolFactory())
{code}

It only works when it never fails (e.g. it never encounters invalid json blobs). Whenever a failure happens, because of the internal state stack and other states in TJSONProtocol, it will be put back into the pool with the wrong state, and when it's next used it will fail again.

There are a few approaches we can take to fix it:

# The simplest "fix" would be just document in TDeserializerPool and TSerializerPool that TJSONProtocol and TSimpleJSONProtocol are not safe to be used, and maybe provide some examples of how to use them (only pool the TMemoryBuffer and recreate TProtocol every time)
# In TJSONProtocol and TSimpleJSONProtocol's [Read|Write][Message|Struct]Begin, implicitly reset the internal state. But I'm not sure how safe that is
# Make a breaking change to TProtocol to add a Reset (or maybe ResetState) API to be used by TDeserializer/TSerializer (similar to how they always reset the TMemoryBuffer before doing anything)
# Similar to 3 but less breaking, just add a Reset/ResetState to TJSONProtocol and TSimpleJSONProtocol (but not TProtocol), and in TDeserializer/TSerializer do a type assertion and call the Reset API on the protocol if it exists.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)