[ 
https://issues.apache.org/jira/browse/THRIFT-5609?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17576066#comment-17576066
 ] 

Yuxuan Wang commented on THRIFT-5609:
-------------------------------------

It looks like there are cases we call ReadStructBegin after ReadMessageBegin, 
so trying to reset the state in ReadStructBegin will mess things up.

> 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
>            Assignee: Yuxuan Wang
>            Priority: Major
>
> 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)

Reply via email to