GUACAMOLE-567: Add specific connection stability test to tunnel implementations.#312
Merged
asfgit merged 5 commits intoapache:staging/1.0.0from Sep 7, 2018
Merged
Conversation
…ent connection state is lost otherwise.
…and UNSTABLE status as connected.
…test connection stability independently of the underlying Guacamole connection.
…e legacy WebSocket tunnel implementations.
Unlike the WebSocket tunnel, where a manual ping request/response must be explicitly implemented, we can rely on HTTP's own request/response to verify stability.
necouchman
approved these changes
Sep 7, 2018
Contributor
necouchman
left a comment
necouchman
left a comment
There was a problem hiding this comment.
Looks good. This ended up being quite the change 😄. Going to test it out before I merge it, but should be done here in a few.
Contributor
|
One question - is this code still Java 1.6 compatible? I was looking up the documentation on the RemoteEndpoint.Basic class, and I don't see it for 1.6 - only 1.7. Do we need to bump the target up? |
Contributor
Author
|
It is, or at least referencing that class doesn't itself require Java 1.7. That class isn't part of Java 1.7 so much as it is part of recent versions of servlet API, which the servlet container may or may not provide. Dynamic detection of WebSocket support takes place during Guacamole's startup. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This change corrects the behavior of the connection instability detection by adding tunnel-specific tests which are independent of the underlying Guacamole connection. Regardless of whether the remote desktop connection itself is returning data, these tests verify that the server side of the tunnel is reachable and responding.
For the WebSocket tunnel, these tests leverage the reserved opcode for tunnel-specific instructions to send a "ping" message which the tunnel is required to respond to. By sending these messages roughly every 500ms, the client side guarantees that there should always be traffic over the tunnel unless something is interrupting that traffic.
For the HTTP tunnel, these tests leverage the fact that HTTP already has a request/response pattern that is verified via the HTTP protocol. A simple "nop" instruction is sent to the Guacamole server to force the send half of the tunnel to effectively self test. If the server side of the HTTP tunnel fails to properly handle the HTTP request which results from sending an instruction, the tunnel is flagged as unstable.