[
https://issues.apache.org/jira/browse/THRIFT-6300?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jens Geyer resolved THRIFT-6300.
--------------------------------
Fix Version/s: 0.26.0
Assignee: Jens Geyer
Resolution: Fixed
> Smalltalk: TBinaryProtocol decodes what the transport returned without
> checking its length
> ------------------------------------------------------------------------------------------
>
> Key: THRIFT-6300
> URL: https://issues.apache.org/jira/browse/THRIFT-6300
> Project: Thrift
> Issue Type: Bug
> Components: Smalltalk - Library
> Reporter: Jens Geyer
> Assignee: Jens Geyer
> Priority: Minor
> Fix For: 0.26.0
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> {{TTransport}} has a read-exactly method that loops until it has the bytes it
> was asked for ({{lib/st/thrift.st:790}}):
> {code}
> readAll: anInteger
> ^ String streamContents: [:str |
> [str size < anInteger] whileTrue:
> [str nextPutAll: (self read: anInteger - str size)]]
> {code}
> {{TBinaryProtocol}} never calls it. {{readInt:}} ({{:254}}), {{readRawInt:}}
> ({{:298}}) and {{readString}} ({{:312}}) each take whatever one {{transport
> read:}} returns and decode that:
> {code}
> readInt: size
> | buf val |
> buf := transport read: size.
> val := self intFromByteArray: buf.
> ^ buf first > 16r7F
> ifTrue: [self unsignedInt: val size: size]
> ifFalse: [val]
> {code}
> {{TSocket>>read:}} ({{:730}}) raises only when the result {{isEmpty}}, so a
> short but non-empty result is passed straight through. {{intFromByteArray:}}
> ({{:210}}) then takes its width from {{buf size}} rather than from {{size}},
> so two bytes delivered for an {{i32}} produce a well-formed but wrong integer
> instead of an error, and {{buf first}} takes the sign from the wrong byte.
> {{readString}} checks the *declared* size against the string limit but not
> how many bytes actually arrived, so it can return a truncated string as if it
> were complete.
> {{lib/st}} ships no server transport, so this needs a peer that answers and
> then truncates the response.
> h2. Suggested
> Use {{readAll:}} in all three places. It already exists and already has the
> loop; nothing new has to be written.
> A caveat on verification: the binding cannot be built or run in the usual
> environments, so whether a given dialect's {{SocketStream>>next:}} actually
> yields a short non-empty result on a truncated peer response has not been
> observed, only read. Confirming it needs Pharo.
> _Drafted with AI assistance (Claude Opus 5); reviewed and filed by Jens
> Geyer._
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
