[ 
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)

Reply via email to