[
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)
