Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃攰 Warn when messages are sent before the v1.1 handshake #607

Merged
merged 1 commit into from
May 2, 2023

Conversation

alecgibson
Copy link
Collaborator

@alecgibson alecgibson commented May 2, 2023

Fixes #605

At the moment, it's possible for messages to be sent before the client- server handshake.

Sending messages before the handshake has happened has undefined behaviour, and can result in errors such as in:
#605

We can't just ignore these messages, because old clients might potentially be on v1.0 of the client-server protocol, in which the server informs the client when it's ready, but not the other way around, so it's impossible to know when a client should be considered "ready", and its messages acceptable.

Instead, we add a warning for clients on v1.1 who have sent a message before their handshake. In order to aid with debugging, we keep track of the first message received, and log it when the handshake is received (which means that v1.0 clients will never get such a warning).

@coveralls
Copy link

coveralls commented May 2, 2023

Coverage Status

Coverage: 97.515% (+0.008%) from 97.507% when pulling 6ee645d on pre-handshake-warning into c1f1474 on master.

At the moment, it's possible for messages to be sent before the client-
server handshake.

Sending messages before the handshake has happened has undefined
behaviour, and can result in errors such as in:
#605

We can't just ignore these messages, because old clients might
potentially be on v1.0 of the client-server protocol, in which the
server informs the client when it's ready, but not the other way around,
so it's impossible to know when a client should be considered "ready",
and its messages acceptable.

Instead, we add a warning for clients on v1.1 who have sent a message
before their handshake. In order to aid with debugging, we keep track of
the first message received, and log it when the handshake is received
(which means that v1.0 clients will never get such a warning).
@alecgibson alecgibson merged commit 26d5d49 into master May 2, 2023
8 checks passed
@alecgibson alecgibson deleted the pre-handshake-warning branch May 2, 2023 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants