Skip to content
Permalink
Branch: develop
Commits on Mar 19, 2019
  1. [finagle-thrift] Allow users to specify stringLengthLimit and contain…

    isnotinvain authored and jenkins committed Mar 19, 2019
    …erLengthLimit
    
    Problem
    
    Currently we only expose 1 setting "readLength" which we end up using
    as the stringLengthLimit. containerLengthLimit is hardcoded to -1.
    Container length limit is important for dealing with corrupt records.
    
    Additionally, the documentation claims that readLength is used to
    limit object size, but it's actually only used to limit the size of
    each string encountered.
    
    Solution
    
    1) Rename readLimit argument to stringLengthLimit  in Protocols.binaryFactory
    2) Add containerLengthLimit argument in Protocols.binaryFactory
    3) Introduce 2 new system properties and marked the original
       (ambiguously named readLimit) as deprecated.  It appears that
       "org.apache.thrift.readLength" is actually no longer used in apache
       thrift, so I put the new properties under
       com.twitter.finagle.thrift instead.
    4) Introduce Protocols.compactFactory for TCompactProtocol
        which also respects the system properties
    
    Result
    
    Users can now configure both properties directly in code, or via
    system properties. The old system property is deprecated but still
    respected.
    
    Differential Revision: https://phabricator.twitter.biz/D286346
You can’t perform that action at this time.