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’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ping timer bugfix, connection timeout adjustment #3294
Comments
Thanks for the patch. Some commentary or just a few words to go along with it would be nice. |
it does few things.
|
I think it would be neat if the connection timeout and the ping interval were settings instead. Then users could increment them themselves if necessary. |
of course. |
sss123next in mumble-voip#3294 reports that the current ping timer logic causes problems with slow TLS handshakes, such as when connecting to servers with large DH parameters. This commit ensures pings are not sent before the TLS handshake has completed.
@sss123next Is the change to use a 120 second connection timeout necessary on the server(s) you're testing against? |
Preliminary PR for the ping timer issue at #3303 |
sss123next in mumble-voip#3294 reports that the current ping timer logic causes problems with slow TLS handshakes, such as when connecting to servers with large DH parameters. This commit ensures pings are not sent before the TLS handshake has completed. This was tested against a Grumble instance built against a modified crypto/tls package that sleeps during the handshake. I verified that isEncrypted() returns false when the handshake has not finished, and returns true when the handshake has completed.
@mkrautz i think it is not, i have set it before discovered ping timer bug. |
i think it's better to have settings. |
The text was updated successfully, but these errors were encountered: