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]


Reply via email to