Skip to content

THRIFT-6052: Limit struct read/write recursion depth in Smalltalk library #3557

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Jens-G merged 1 commit into from
Jun 3, 2026
+281 −8
Merged

THRIFT-6052: Limit struct read/write recursion depth in Smalltalk library #3557

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions compiler/cpp/src/thrift/generate/t_st_generator.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -706,9 +706,10 @@ string t_st_generator::struct_writer(t_struct* tstruct, string sname) {
const vector<t_field*>& fields = tstruct->get_sorted_members();
vector<t_field*>::const_iterator fld_iter;

out << "[oprot writeStructBegin: "
<< "(TStruct new name: '" + tstruct->get_name() + "')." << '\n';
out << "[oprot incrementRecursionDepth." << '\n';
indent_up();
out << indent() << "[oprot writeStructBegin: "
<< "(TStruct new name: '" + tstruct->get_name() + "')." << '\n';

for (fld_iter = fields.begin(); fld_iter != fields.end(); ++fld_iter) {
bool optional = (*fld_iter)->get_req() == t_field::T_OPTIONAL;
Expand All @@ -735,7 +736,8 @@ string t_st_generator::struct_writer(t_struct* tstruct, string sname) {
out << "." << '\n';
}

out << indent() << "oprot writeFieldStop; writeStructEnd] value";
out << indent()
<< "oprot writeFieldStop; writeStructEnd] ensure: [oprot decrementRecursionDepth]] value";
indent_down();

return out.str();
Expand All @@ -759,10 +761,11 @@ string t_st_generator::struct_reader(t_struct* tstruct, string clsName = "") {
// This is nasty, but without it we'll break things by prefixing TResult.
string name = ((capitalize(clsName) == "TResult") ? capitalize(clsName) : prefix(clsName));
out << indent() << val << " := " << name << " new." << '\n';
out << indent() << "iprot incrementRecursionDepth." << '\n';

out << indent() << "iprot readStructBegin." << '\n' << indent() << "[" << desc
<< " := iprot readFieldBegin." << '\n' << indent() << desc
<< " type = TType stop] whileFalse: [|" << found << "|" << '\n';
out << indent() << "[iprot readStructBegin." << '\n'
<< indent() << "[" << desc << " := iprot readFieldBegin." << '\n'
<< indent() << desc << " type = TType stop] whileFalse: [|" << found << "|" << '\n';
indent_up();

for (fld_iter = fields.begin(); fld_iter != fields.end(); ++fld_iter) {
Expand All @@ -779,7 +782,8 @@ string t_st_generator::struct_reader(t_struct* tstruct, string clsName = "") {
out << indent() << found << " ifNil: [iprot skip: " << desc << " type]]." << '\n';
indent_down();

out << indent() << "oprot readStructEnd." << '\n' << indent() << val << "] value";
out << indent() << "iprot readStructEnd] ensure: [iprot decrementRecursionDepth]." << '\n'
<< indent() << val << "] value";
indent_down();

return out.str();
Expand Down
101 changes: 101 additions & 0 deletions lib/st/test/RecursionDepthTest.thrift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

// Input for TProtocolRecursionDepthTest. The generated struct read/write path is
// driven deep enough to cross the recursion limit (64) so that the limit added
// in thrift.st / the Smalltalk generator can be exercised end to end.
//
// A *finite* chain of distinct struct types is used rather than a genuinely
// recursive type (as in test/Recursive.thrift): the Smalltalk generator
// inline-expands nested struct serialization, so a self- or mutually-recursive
// type makes it recurse without bound at code-generation time and crash. That
// is a separate, pre-existing generator limitation, tracked apart from this fix.
//
// Field 'f' is optional so an instance (or a crafted payload) can stop at any
// depth, letting one chain exercise both the at-limit and over-limit cases.

struct A1 { 1: optional A2 f }
struct A2 { 1: optional A3 f }
struct A3 { 1: optional A4 f }
struct A4 { 1: optional A5 f }
struct A5 { 1: optional A6 f }
struct A6 { 1: optional A7 f }
struct A7 { 1: optional A8 f }
struct A8 { 1: optional A9 f }
struct A9 { 1: optional A10 f }
struct A10 { 1: optional A11 f }
struct A11 { 1: optional A12 f }
struct A12 { 1: optional A13 f }
struct A13 { 1: optional A14 f }
struct A14 { 1: optional A15 f }
struct A15 { 1: optional A16 f }
struct A16 { 1: optional A17 f }
struct A17 { 1: optional A18 f }
struct A18 { 1: optional A19 f }
struct A19 { 1: optional A20 f }
struct A20 { 1: optional A21 f }
struct A21 { 1: optional A22 f }
struct A22 { 1: optional A23 f }
struct A23 { 1: optional A24 f }
struct A24 { 1: optional A25 f }
struct A25 { 1: optional A26 f }
struct A26 { 1: optional A27 f }
struct A27 { 1: optional A28 f }
struct A28 { 1: optional A29 f }
struct A29 { 1: optional A30 f }
struct A30 { 1: optional A31 f }
struct A31 { 1: optional A32 f }
struct A32 { 1: optional A33 f }
struct A33 { 1: optional A34 f }
struct A34 { 1: optional A35 f }
struct A35 { 1: optional A36 f }
struct A36 { 1: optional A37 f }
struct A37 { 1: optional A38 f }
struct A38 { 1: optional A39 f }
struct A39 { 1: optional A40 f }
struct A40 { 1: optional A41 f }
struct A41 { 1: optional A42 f }
struct A42 { 1: optional A43 f }
struct A43 { 1: optional A44 f }
struct A44 { 1: optional A45 f }
struct A45 { 1: optional A46 f }
struct A46 { 1: optional A47 f }
struct A47 { 1: optional A48 f }
struct A48 { 1: optional A49 f }
struct A49 { 1: optional A50 f }
struct A50 { 1: optional A51 f }
struct A51 { 1: optional A52 f }
struct A52 { 1: optional A53 f }
struct A53 { 1: optional A54 f }
struct A54 { 1: optional A55 f }
struct A55 { 1: optional A56 f }
struct A56 { 1: optional A57 f }
struct A57 { 1: optional A58 f }
struct A58 { 1: optional A59 f }
struct A59 { 1: optional A60 f }
struct A60 { 1: optional A61 f }
struct A61 { 1: optional A62 f }
struct A62 { 1: optional A63 f }
struct A63 { 1: optional A64 f }
struct A64 { 1: optional A65 f }
struct A65 { 1: optional i16 x }

service Deep {
A1 echo(1: A1 a)
}
153 changes: 153 additions & 0 deletions lib/st/test/TProtocolRecursionDepthTest.st
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
'Recursion-depth limit test for the *generated* Smalltalk struct read/write path.

Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.

This drives the recursion-depth limit through the generated struct read/write
code (DeepClient sendEchoA: / recvEcho), not by poking the depth counter in
isolation. The recursive types in test/Recursive.thrift cannot be used: the
Smalltalk generator inline-expands nested struct serialization and recurses
without bound at code-generation time on a recursive type, so a finite chain of
distinct struct types (A1..A65) is used to nest past the limit (64) instead.

To run (Squeak/Pharo, or GNU Smalltalk with SUnit):
thrift --gen st RecursionDepthTest.thrift
file in: thrift.st, the generated RecursionDepthTest.st, then this file.
Run the TProtocolRecursionDepthTest suite.
'!

"A minimal in-memory transport so the round-trip needs no socket: writeByte:
hands us an Array, writeString: a String; we normalise both to bytes and replay
them as a ByteArray (first -> integer for readByte, asString -> String)."
TTransport subclass: #TMemoryBuffer
instanceVariableNames: 'bytes pos'
classVariableNames: ''
poolDictionaries: ''
category: 'Thrift-Tests'!

!TMemoryBuffer class methodsFor: 'instance creation'!
new
^ super new init! !

!TMemoryBuffer methodsFor: 'init'!
init
bytes := OrderedCollection new.
pos := 0! !

!TMemoryBuffer methodsFor: 'transport'!
write: aCollection
aCollection do: [:each | bytes add: each asInteger]! !

!TMemoryBuffer methodsFor: 'transport'!
read: anInteger
| chunk |
chunk := bytes copyFrom: pos + 1 to: pos + anInteger.
pos := pos + anInteger.
^ chunk asByteArray! !

!TMemoryBuffer methodsFor: 'transport'!
flush! !

!TMemoryBuffer methodsFor: 'transport'!
open
^ self! !

!TMemoryBuffer methodsFor: 'transport'!
close! !

!TMemoryBuffer methodsFor: 'transport'!
isOpen
^ true! !

TestCase subclass: #TProtocolRecursionDepthTest
instanceVariableNames: ''
classVariableNames: ''
poolDictionaries: ''
category: 'Thrift-Tests'!

!TProtocolRecursionDepthTest methodsFor: 'support'!
recursionLimit
"The limit enforced by TProtocol>>incrementRecursionDepth."
^ 64! !

!TProtocolRecursionDepthTest methodsFor: 'support'!
clientOn: aTransport
^ DeepClient new
inProtocol: (TBinaryProtocol new transport: aTransport);
yourself! !

!TProtocolRecursionDepthTest methodsFor: 'support'!
chainOfDepth: depth
"An A1->A2->..->A`depth` chain of distinct structs; deeper links stay nil.
Writing it drives `depth` nested struct writers, i.e. depth levels deep."
| node |
node := nil.
depth to: 1 by: -1 do: [:i | | cur |
cur := (Smalltalk at: ('A', i printString) asSymbol) new.
i = 65 ifTrue: [cur x: 1] ifFalse: [cur f: node].
node := cur].
^ node! !

!TProtocolRecursionDepthTest methodsFor: 'support'!
craftReplyOfDepth: n
"Craft an echo_result REPLY whose success value nests n structs deep, using
raw protocol primitives so the depth counter is bypassed on the way in (a
normal write of an over-limit value would itself be rejected). Reading it
back drives the generated struct reader n (+1 for the result) levels deep."
| transport proto emit |
transport := TMemoryBuffer new.
proto := TBinaryProtocol new transport: transport.
proto writeMessageBegin: (TCallMessage new name: 'echo'; seqid: 0; yourself).
proto writeFieldBegin: (TField new name: 'success'; type: TType struct; id: 0).
emit := nil.
emit := [:i |
i < n ifTrue: [
proto writeFieldBegin: (TField new name: 'f'; type: TType struct; id: 1).
emit value: i + 1].
proto writeFieldStop].
emit value: 1.
proto writeFieldStop.
proto writeMessageEnd.
^ transport! !

!TProtocolRecursionDepthTest methodsFor: 'tests'!
testWriteAtLimitRoundTrips
"A chain exactly at the limit serializes without error."
self
shouldnt: [(self clientOn: TMemoryBuffer new) sendEchoA: (self chainOfDepth: self recursionLimit)]
raise: TProtocolError! !

!TProtocolRecursionDepthTest methodsFor: 'tests'!
testWriteOverLimitIsRejected
"A chain one level past the limit is rejected with TProtocolError."
self
should: [(self clientOn: TMemoryBuffer new) sendEchoA: (self chainOfDepth: self recursionLimit + 1)]
raise: TProtocolError! !

!TProtocolRecursionDepthTest methodsFor: 'tests'!
testReadWithinLimitRoundTrips
"A shallow payload is read back through the generated reader without error."
self
shouldnt: [(self clientOn: (self craftReplyOfDepth: 3)) recvEcho]
raise: TProtocolError! !

!TProtocolRecursionDepthTest methodsFor: 'tests'!
testReadOverLimitIsRejected
"A payload nested past the limit is rejected with TProtocolError on read."
self
should: [(self clientOn: (self craftReplyOfDepth: self recursionLimit + 6)) recvEcho]
raise: TProtocolError! !
17 changes: 16 additions & 1 deletion lib/st/thrift.st
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,10 @@ sizeLimit
unknown
^ 0! !

!TProtocolError class methodsFor: 'as yet unclassified'!
depthLimit
^ 6! !

TError subclass: #TTransportError
instanceVariableNames: ''
classVariableNames: ''
Expand Down Expand Up @@ -191,7 +195,7 @@ type: anInteger
type := anInteger! !

Object subclass: #TProtocol
instanceVariableNames: 'transport'
instanceVariableNames: 'transport recursionDepth'
classVariableNames: ''
poolDictionaries: ''
category: 'Thrift-Protocol'!
Expand Down Expand Up @@ -507,6 +511,17 @@ transport
transport: aTransport
transport := aTransport! !

!TProtocol methodsFor: 'as yet unclassified'!
incrementRecursionDepth
recursionDepth := (recursionDepth ifNil: [0]) + 1.
recursionDepth > 64 ifTrue: [
recursionDepth := recursionDepth - 1.
TProtocolError signal: 'Maximum recursion depth exceeded']! !

!TProtocol methodsFor: 'as yet unclassified'!
decrementRecursionDepth
recursionDepth := (recursionDepth ifNil: [0]) - 1! !

!TProtocol methodsFor: 'writing' stamp: 'pc 10/24/2007 19:37'!
writeBool: aBool! !

Expand Down
Loading