Bugifx: Index out of bounds in Server#extract_conn_info

[spuun]

WHAT is this pull request doing?

When we try to autodetect proxy protocol version we sometimes get a Index ouf of bounds because there isn't enough data on the socket for IO::Buffered#peek to read, so our peek[x,y] call raises.

This will add an overload #peek(size) to IO::Buffered which will ensure the buffer contains at least size bytes.

Maybe we'll get something similar in the stdlib in the future: crystal-lang/crystal#15075

HOW can this pull request be tested?

Run specs to verify #peek(size)…

[kickster97]

When i try so send an MQTT ping without doing a proper connect first, i get stuck in peek and my client times out.
before, the connection factory was able to try and connect, and then send a connack letting the client know its not allowed.

[kickster97]

also these look different now

2024-10-11T14:50:37.650125Z   INFO - lmq.server: Listening on [::1]:62815
2024-10-11T14:50:37.650135Z   INFO - lmq.server: Listening on [::1]:62814

[spuun]

When i try so send an MQTT ping without doing a proper connect first, i get stuck in peek and my client times out. before, the connection factory was able to try and connect, and then send a connack letting the client know its not allowed.

Probably related to read timeout in lavinmq.

I wonder if that spec is passing but for the wrong reason. I need to investigate that. I think the socket has been closed because of the bug this PR is trying to fix, not because the wrong mqtt packet was sent.

[spuun]

also these look different now

2024-10-11T14:50:37.650125Z   INFO - lmq.server: Listening on [::1]:62815
2024-10-11T14:50:37.650135Z   INFO - lmq.server: Listening on [::1]:62814

Different how? :)

[kickster97]

also these look different now

2024-10-11T14:50:37.650125Z INFO - lmq.server: Listening on [::1]:62815

2024-10-11T14:50:37.650135Z INFO - lmq.server: Listening on [::1]:62814

Different how? :)

The [::1]:62815 i think... but maybe its just my spec 😅

[spuun]

The [::1]:62815 i think... but maybe its just my spec 😅

I think it's related to other changes, like the uniformed logging.

[carlhoerberg]

I would probably just check the length of the peek buffer to avoid the indexerror. I dont know when only part of the Proxy protocol header would be sent, so if the buffer isnt long enough we can be certain that it isnt a a proxy protocol at least

[carlhoerberg]

Actually, just use Slice#[]?, https://devdocs.io/crystal/slice#%5B%5D%3F%28start%3AInt%2Ccount%3AInt%29%3ASlice%28T%29%7CNil-instance-method

[spuun]

Yeah, but I thought it maybe could be a contribution to the stdlib.