[
https://issues.apache.org/jira/browse/THRIFT-6164?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jens Geyer updated THRIFT-6164:
-------------------------------
Description:
h3. Problem
{{thrift_protocol:skip/2}} walks type ids taken off the wire rather than the
types the IDL
declared, so the peer chooses both the shape of the nesting and how deep it
goes. {{skip/2}}
and the four skip loops -- {{skip_struct_loop}}, {{skip_map_loop}},
{{skip_set_loop}},
{{skip_list_loop}} -- call one another with nothing carrying a depth.
A struct level costs the sender three bytes. 200 levels of nesting is 801 bytes
on the wire.
h3. Change
{{skip/2}} seeds {{DEFAULT_RECURSION_DEPTH}}, 64 -- the same value {{lib/py}}
and {{lib/cpp}}
already use -- and each level of nesting spends one. {{skip/3}} is exported so
a caller who
knows what its own schema can contain can ask for a lower ceiling. Exceeding
the ceiling raises
{code}
error({protocol_error, max_skip_depth_exceeded})
{code}
matching the binding's existing {{negative_size}} idiom.
h3. Two things the fix deliberately does not do
* *No guard on the typed read path.* {{read/2}}'s {{struct}}, {{list}}, {{map}}
and {{set}}
clauses recurse on a type taken from the generated {{struct_info}}, never from
the wire, so
their depth is fixed by the IDL and a peer cannot drive it. That is why the
allowance starts
fresh at {{skip/2}} instead of being threaded down from the reader.
* *No charge for element counts.* The three container loops recurse once per
element in tail
position, which costs no stack. Charging them there would refuse large flat
containers while
leaving the dimension that does consume stack wide open.
h3. Tests
Seven, over a memory buffer: one per container type, one pinning the wire cost
of 200 struct
levels at 801 bytes, one checking ordinary nesting still skips, and one
checking an explicit
ceiling passed to {{skip/3}} is honoured. Five fail against the unmodified
library.
_Drafted with AI assistance (Claude Opus 5); filed by Jens Geyer._
was:
h3. Problem
{{thrift_protocol:skip/2}} walks type ids taken off the wire rather than the
types the IDL
declared, so the peer chooses both the shape of the nesting and how deep it
goes. {{skip/2}}
and the four skip loops -- {{skip_struct_loop}}, {{skip_map_loop}},
{{skip_set_loop}},
{{skip_list_loop}} -- call one another with nothing carrying a depth.
A struct level costs the sender three bytes. 200 levels of nesting is 801 bytes
on the wire.
h3. Change
{{skip/2}} seeds {{DEFAULT_RECURSION_DEPTH}}, 64 -- the same value {{lib/py}}
and {{lib/cpp}}
already use -- and each level of nesting spends one. {{skip/3}} is exported so
a caller who
knows what its own schema can contain can ask for a lower ceiling. Exceeding
the ceiling raises
{code}
error({protocol_error, max_skip_depth_exceeded})
{code}
matching the binding's existing {{negative_size}} idiom.
h3. Two things the fix deliberately does not do
* *No guard on the typed read path.* {{read/2}}'s {{struct}}, {{list}}, {{map}}
and {{set}}
clauses recurse on a type taken from the generated {{struct_info}}, never from
the wire, so
their depth is fixed by the IDL and a peer cannot drive it. That is why the
allowance starts
fresh at {{skip/2}} instead of being threaded down from the reader.
* *No charge for element counts.* The three container loops recurse once per
element in tail
position, which costs no stack. Charging them there would refuse large flat
containers while
leaving the dimension that does consume stack wide open.
h3. Tests
Seven, over a memory buffer: one per container type, one pinning the wire cost
of 200 struct
levels at 801 bytes, one checking ordinary nesting still skips, and one
checking an explicit
ceiling passed to {{skip/3}} is honoured. Five fail against the unmodified
library.
> Erlang: bound the depth thrift_protocol:skip/2 will follow
> ----------------------------------------------------------
>
> Key: THRIFT-6164
> URL: https://issues.apache.org/jira/browse/THRIFT-6164
> Project: Thrift
> Issue Type: Bug
> Components: Erlang - Library
> Reporter: Jens Geyer
> Priority: Major
> Fix For: 0.25.0
>
> Time Spent: 10m
> Remaining Estimate: 0h
>
> h3. Problem
> {{thrift_protocol:skip/2}} walks type ids taken off the wire rather than the
> types the IDL
> declared, so the peer chooses both the shape of the nesting and how deep it
> goes. {{skip/2}}
> and the four skip loops -- {{skip_struct_loop}}, {{skip_map_loop}},
> {{skip_set_loop}},
> {{skip_list_loop}} -- call one another with nothing carrying a depth.
> A struct level costs the sender three bytes. 200 levels of nesting is 801
> bytes on the wire.
> h3. Change
> {{skip/2}} seeds {{DEFAULT_RECURSION_DEPTH}}, 64 -- the same value {{lib/py}}
> and {{lib/cpp}}
> already use -- and each level of nesting spends one. {{skip/3}} is exported
> so a caller who
> knows what its own schema can contain can ask for a lower ceiling. Exceeding
> the ceiling raises
> {code}
> error({protocol_error, max_skip_depth_exceeded})
> {code}
> matching the binding's existing {{negative_size}} idiom.
> h3. Two things the fix deliberately does not do
> * *No guard on the typed read path.* {{read/2}}'s {{struct}}, {{list}},
> {{map}} and {{set}}
> clauses recurse on a type taken from the generated {{struct_info}}, never
> from the wire, so
> their depth is fixed by the IDL and a peer cannot drive it. That is why the
> allowance starts
> fresh at {{skip/2}} instead of being threaded down from the reader.
> * *No charge for element counts.* The three container loops recurse once per
> element in tail
> position, which costs no stack. Charging them there would refuse large flat
> containers while
> leaving the dimension that does consume stack wide open.
> h3. Tests
> Seven, over a memory buffer: one per container type, one pinning the wire
> cost of 200 struct
> levels at 801 bytes, one checking ordinary nesting still skips, and one
> checking an explicit
> ceiling passed to {{skip/3}} is honoured. Five fail against the unmodified
> library.
> _Drafted with AI assistance (Claude Opus 5); filed by Jens Geyer._
--
This message was sent by Atlassian Jira
(v8.20.10#820010)