Fix BufferedIO to search with byteindex

[casperisfine]

As of #184, the buffer String is no longer BINARY but UTF-8.

I missed that the code that search for newlines was using .index instead of .byteindex, causing the buffer offset to go out of sync.

[casperisfine]

Arf, byteindex was added in 3.2...

[mperham]

What are the symptoms of this bug? What behavior or errors might the user see?

[byroot]

This bug was never released.

[byroot]

Hum, now that you mention it. I have no evidence it was possible, but if somehow the internal buffer was upgraded to UTF8 when appending bytes, it would lead to a ReadTimeout.

So perhaps, and I insist on the perhaps, it could explain some of the timeout issues reported, but only for people using the Ruby driver. And this is pure conjecture, I'll have a quick look to try to confirm whether this could ever happen before the performance refactor. And if that's the case, this only fixes it for Ruby 3.2+

[byroot]

Hum, so no, I tried various things, and I really don't think it was possible on 0.21.1.

The only two places the buffer is mutated are:

• https://github.com/redis-rb/redis-client/blob/v0.21.1/lib/redis_client/ruby_connection/buffered_io.rb#L134
• https://github.com/redis-rb/redis-client/blob/v0.21.1/lib/redis_client/ruby_connection/buffered_io.rb#L124

In both case it's handled by read_nonblock.

When passed a buffer, it doesn't change its encoding as long as there is a size provided (ruby/spec#1145)

And when not passed a buffer, similarly, the return string is binary if a size is provided.

I nonetheless simulated this with TCP/UNIX/SSL sockets to double check and couldn't cause any timeout. I also went to audit the OpenSSL gem, and couldn't see any edge case in which it would return a non-binary string.

All this being said, it could be interesting to ask people to experience these issues to log the buffer and offset when it happens.

[mperham]

Yeah, I wondered if it might be the cause of mysterious Sidekiq errors people have been seeing. I missed the recency of the change but this feels like the type of subtle error that can persist (and only trigger very occasionally). Thanks for checking for me.