Re: [PR] GUACAMOLE-600: Add support for setting SSH and SFTP timeouts. [guacamole-server]

[via GitHub]

mike-jumper merged PR #414:
URL: https://github.com/apache/guacamole-server/pull/414


-- 
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.

To unsubscribe, e-mail: dev-unsubscr...@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] GUACAMOLE-600: Add support for setting SSH and SFTP timeouts. [guacamole-server]

[via GitHub]

necouchman commented on PR #414:
URL: https://github.com/apache/guacamole-server/pull/414#issuecomment-2130667080

   Okay, I think this is ready, again, for review.


-- 
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.

To unsubscribe, e-mail: dev-unsubscr...@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] GUACAMOLE-600: Add support for setting SSH and SFTP timeouts. [guacamole-server]

[via GitHub]

necouchman commented on PR #414:
URL: https://github.com/apache/guacamole-server/pull/414#issuecomment-2130175336

   Actually, converting this back to a draft. Since I used a previous change to 
collect the common socket code together in a single place, this needs to be 
reworked a bit...


-- 
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.

To unsubscribe, e-mail: dev-unsubscr...@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] GUACAMOLE-600: Add support for setting SSH and SFTP timeouts. [guacamole-server]

[via GitHub]

necouchman commented on code in PR #414:
URL: https://github.com/apache/guacamole-server/pull/414#discussion_r1613891115


##
src/common-ssh/ssh.c:
##
@@ -453,17 +455,43 @@ guac_common_ssh_session* 
guac_common_ssh_create_session(guac_client* client,
 return NULL;
 }
 
-/* Connect */
-if (connect(fd, current_address->ai_addr,
-current_address->ai_addrlen) == 0) {
+/* Set socket to non-blocking */
+fcntl(fd, F_SETFL, O_NONBLOCK);
+
+/* Set up timeout. */
+fd_set fdset;
+FD_ZERO(&fdset);
+FD_SET(fd, &fdset);
+
+struct timeval tv;
+tv.tv_sec = timeout; /* 10 second timeout */

Review Comment:
   Fixed via rebase.



-- 
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.

To unsubscribe, e-mail: dev-unsubscr...@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] GUACAMOLE-600: Add support for setting SSH and SFTP timeouts. [guacamole-server]

[via GitHub]

necouchman commented on code in PR #414:
URL: https://github.com/apache/guacamole-server/pull/414#discussion_r1613889314


##
src/common-ssh/ssh.c:
##
@@ -453,17 +455,43 @@ guac_common_ssh_session* 
guac_common_ssh_create_session(guac_client* client,
 return NULL;
 }
 
-/* Connect */
-if (connect(fd, current_address->ai_addr,
-current_address->ai_addrlen) == 0) {
+/* Set socket to non-blocking */
+fcntl(fd, F_SETFL, O_NONBLOCK);
+
+/* Set up timeout. */
+fd_set fdset;
+FD_ZERO(&fdset);
+FD_SET(fd, &fdset);
+
+struct timeval tv;
+tv.tv_sec = timeout; /* 10 second timeout */

Review Comment:
   Hmmm...going to guess that was some copy-pasta, there, particularly since we 
don't usually put comments out to the right of the code...



-- 
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.

To unsubscribe, e-mail: dev-unsubscr...@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] GUACAMOLE-600: Add support for setting SSH and SFTP timeouts. [guacamole-server]

[via GitHub]

jmuehlner commented on code in PR #414:
URL: https://github.com/apache/guacamole-server/pull/414#discussion_r1613864540


##
src/common-ssh/ssh.c:
##
@@ -453,17 +455,43 @@ guac_common_ssh_session* 
guac_common_ssh_create_session(guac_client* client,
 return NULL;
 }
 
-/* Connect */
-if (connect(fd, current_address->ai_addr,
-current_address->ai_addrlen) == 0) {
+/* Set socket to non-blocking */
+fcntl(fd, F_SETFL, O_NONBLOCK);
+
+/* Set up timeout. */
+fd_set fdset;
+FD_ZERO(&fdset);
+FD_SET(fd, &fdset);
+
+struct timeval tv;
+tv.tv_sec = timeout; /* 10 second timeout */

Review Comment:
   "10 second timeout"?



-- 
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.

To unsubscribe, e-mail: dev-unsubscr...@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org