#Refactoring out DepthLimiter

9 messages · Page 1 of 1 (latest)

chilly lion
#

I've been looking at how to add width limiting and other options to stellar-xdr, and I don't see a good way to add additional options with the existing pattern that's been used when we added depth limiting with DepthLimiter/DepthLimitedRead/DepthLimitedWrite.

If we followed the same pattern we're using now, we'd add a new trait, and two impls for each trait, and then pass around an impl DepthLimitedRead<R> + WidthLimitedRead<R> + OtherOptionRead<R> which I'm not actually sure is possible because of there are limitations on combining traits.

We're using the Rust trait and type system to add additional options to a Read, which is pretty complicated, but we could just pass those options around as a simple struct.

So I'm thinking of removing the DepthLimiter type system, and replacing it with every read_xdr accepting a r: impl Read and a o: Options, where Options is a simple struct that contains any remaining depth, remaining width, or other options.

Thoughts?

#

I think I'd also remove functions like from_xdr_base64_with_depth_limit and add a mandatory Options parameter to from_xdr_base64, and have Options::default() return the defaults we're using today for depth limiting.

soft zephyr
#

I'm not sure i understand what the use is? Is it for formatting

chilly lion
#

It sets enforces limits when parsing XDR.

soft zephyr
#

sorry can you explain the difference between depth and width in this context why can't you just enforce a limit of bytes for example. did my brain break?

chilly lion
#

The terms aren't formal and we're using them liberally, or rather in made up ways. By depth we're referring to XDR objs containing other XDR objs. So if we set a limit of 2, you couldn't have 3 XDR objs nested. It's stop parsing after the second and exit. By width we're referring to the number of bytes consumed.

soft zephyr
#

Thanks. Great answer

chilly lion
#

Size limited would be a better name than width limited.

soft zephyr
#

Yes but depth limiter is intuitive still but agree on sizelimiter