necouchman commented on a change in pull request #228:
URL: https://github.com/apache/guacamole-server/pull/228#discussion_r446569698
##########
File path: src/libguac/protocol.c
##########
@@ -961,6 +975,50 @@ int guac_protocol_send_rect(guac_socket* socket,
}
+/**
+ * Sends the "required" instruction on the given socket, looping
+ * through all the items provided in the NULL-terminated array,
+ * and closing out the instruction. Returns zero on success, or
+ * non-zero on error.
+ *
+ * @param socket
+ * The socket on which to write the instruction.
+ *
+ * @param required
+ * The NULL-terminated array of required parameters to send
+ * to the client.
+ *
+ * @return
+ * Zero if successful, non-zero on error.
+ */
+static int __guac_protocol_send_required(guac_socket* socket,
+ const char** required) {
+
+ // The socket should be kept alive while waiting for user response.
+ guac_socket_require_keep_alive(socket);
Review comment:
So, had a question related to this below on the owner thread, but,
question, here - it looks like at least SSH and VNC already set this in the
main thread to handle lengthy connects. Presumably once it's set on a socket
it does not have to be re-set - so the usages toward the beginning of the
`guac_vnc_client_thread()` and `ssh_client_thread()` functions set it for the
duration of the SSH session, correct? Or do the sockets come and go for a
particular connection and it would have to be set, again, during other phases
of the connection?
If my assumptions are correct...
* Is there any reason not to just add it to the main RDP thread, prior to
authentication?
* With 3 out of 5 protocols implementing the requirement for keep-alives
very early on in the connection process, and assuming the keep-alives are then
active for the duration of the connections, is there any reason not to just set
it across the board?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]