[jira] [Commented] (THRIFT-5609) TJSONProtocol is unsafe to be used with TDeserializerPool

["Yuxuan Wang (Jira)" <ji...@apache.org>]

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

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

Also updated the issue to be only about TJSONProtocol and TDeserializerPool (removed TSimpleJSONProtocol and TSerializerPool), because:

# A TSerializerPool cannot really fail (the only possible failure is write fail and the TMemoryBuffer cannot fail)
# TSimpleJSONProtocol cannot be deserialized to begin with (even without a pool)

But I did also add Reset call to TSerializer.

> TJSONProtocol is unsafe to be used with TDeserializerPool
> ---------------------------------------------------------
>
>                 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
>          Time Spent: 10m
>  Remaining Estimate: 0h
>
> 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)