Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/incubator-guacamole-server/pull/11#discussion_r64854778
--- Diff: src/common-ssh/guac_ssh.c ---
@@ -274,19 +275,36 @@ static void guac_common_ssh_kbd_callback(const char
*name, int name_len,
}
+void ssh_client_error(bool abortOnError, guac_client* client,
guac_protocol_status status,
+ const char* format, ...) {
+ va_list args;
+ va_start(args, format);
+ if(abortOnError) {
+ vguac_client_abort(client, status, format, args);
+ }
+ else {
+ vguac_client_log(client, GUAC_LOG_ERROR, format, args);
+ }
+ va_end(args);
+}
+
/**
* Authenticates the user associated with the given session over SSH. All
* required credentials must already be present within the user object
* associated with the given session.
*
- * @param session
+ * @param abortOnError
+ * If set to true, the connection will be aborted. Otherwise, the error
+ * will be logged.
+ *
+ * @param common_session
* The session associated with the user to be authenticated.
*
* @return
* Zero if authentication succeeds, or non-zero if authentication has
* failed.
*/
-static int guac_common_ssh_authenticate(guac_common_ssh_session*
common_session) {
+static int guac_common_ssh_authenticate(bool abortOnError,
guac_common_ssh_session* common_session) {
--- End diff --
Since the function is already defined as returning zero / non-zero
depending on whether the operation succeeds, why not leverage that and let the
caller dictate whether to abort?
It seems like we shouldn't have had `guac_common_ssh_authenticate()`
automatically handle the full blown abort in the first place - way out of scope
for what it's suppose to do / be aware of.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
