arthurnn / memcached

A Ruby interface to the libmemcached C client
Academic Free License v3.0
432 stars 127 forks source link

Avoid overflowing stack with too many keys #205

Closed simon-engledew closed 2 years ago

simon-engledew commented 2 years ago

If a client attempts to to a get_multi with a very large number of keys, inspect_keys will fail with stack level too deep (SystemStackError)

The underlying cause of the overflow is due to the way inspect_keys creates a hash, and can be illustrated with:

Hash[*(['a', 'a'] * 2000000000)]

Because of SystemStackError's parent, unless you explicitly rescue with rescue Exception it will sail through an empty rescue block:

irb(main):038:1* begin
irb(main):039:1*   raise SystemStackError.new("hi")
irb(main):040:1* rescue
irb(main):041:1*   puts "fine"
irb(main):042:0> end
(irb):39:in `<main>': hi (SystemStackError)

This overflow can be fixed using an alternative form for Hash[] which passes the values as a single array argument.

It'll probably still break horribly, but hopefully in a more conventional way. 😅

arthurnn commented 2 years ago

I think this is 👍 @tenderlove @byroot is this syntax equivalent?

byroot commented 2 years ago

Yes:

>> Hash[1, 2, 3, 4]
=> {1=>2, 3=>4}
>> Hash[[1, 2], [3, 4]]
=> {[1, 2]=>[3, 4]}

However if you are supplying pairs, you might as well use Array#to_h:

>> [[1, 2], [3, 4]].to_h
=> {1=>2, 3=>4}

Which also takes a block starting in Ruby 2.6:

>> [1, 2].to_h { |i| [i, i]}
=> {1=>1, 2=>2}

I'm not sure what's the format of the received argument here, so I'll ned to dig a bit to see if this change is sound, but on the surface it looks good to me.

simon-engledew commented 2 years ago

I'm going to close this as stale. I think this change has made it into a few places, but it sounds like it should be fixed differently in the most recent version of this library. 🙇