Jens-G opened a new pull request, #3918: URL: https://github.com/apache/thrift/pull/3918
> **Stacked on [#3916](https://github.com/apache/thrift/pull/3916) (THRIFT-6062).** Same file, and 6062 fixed the mirror image of this on the write side. Review that first; this branch contains it. `TClient` keeps two protocols, `iprot` and `oprot`, and offers `inProtocol:` and `outProtocol:` so a client can read and write over different ones. The generated `recv` methods read the message envelope from the wrong one: ```smalltalk msg := oprot readMessageBegin. "<- output protocol" self validateRemoteMessage: msg. res := [ ... iprot readStructBegin ... ] value. "<- input protocol" oprot readMessageEnd. "<- output protocol" ``` The envelope from `oprot`, the struct inside it from `iprot`, in the same method. ## Why nobody noticed ```smalltalk inProtocol: aProtocol iprot := aProtocol. oprot ifNil: [oprot := aProtocol] ``` A client built the usual way has both pointing at the same object, so reading the envelope from either is the same thing. A client that calls `outProtocol:` with a different protocol — the only reason that setter exists — reads the reply envelope from the protocol it *writes to*. It cannot work. ## The fix Both reads use `iprot`. The flush is a write-side operation and stays on `oprot`. The unused temporary `f`, declared on the line being changed, goes with it. THRIFT-6062 fixed the mirror image: `write_val` emitted `iprot write...` for base types and enums while the rest of the write path used `oprot`. ## Two-state, with genuinely separate transports A reply written to the **input** buffer only, and a client given `inProtocol:` and `outProtocol:` pointing at different protocols over different buffers: | | `recvEcho` | |---|---| | before | `SubscriptOutOfBounds: '1 is not between 0 and 0 in an OrderedCollection()'` — reading the empty *output* buffer | | after | `inner a=42 name='hi'` | The generated line confirms it directly: ``` 6062 (before): msg := oprot readMessageBegin 6328 (after): msg := iprot readMessageBegin ``` ## The ordinary path is untouched - wire bytes from `sendEchoO:` **identical** to what the compiler produced before #3916 — same 41 bytes - lib/st suites on code from this compiler: recursion-depth **6/6**, string-size-limit **11/11** - compiler ctest: **15/15**, including `st_recursive_types` from #3916 ## Note This has no automated regression guard. A compiler test can only check that it generates; catching *which* protocol it reads from needs a running client with two transports, and `lib/st` has no such suite. The Pharo check above is reproducible — I can add it as a suite once [#3913](https://github.com/apache/thrift/pull/3913) lands, if you want one. JIRA: [THRIFT-6328](https://issues.apache.org/jira/browse/THRIFT-6328) 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
