On 4/14/25 5:27 AM, Efraim Vagner wrote:
Hi,
No sure if this is a bug in guacd or the intended behavior and I just
don't use it correctly.
I use guacamole-common-js and the guacd docker image in my system. As
part of the system, in my own js code, I check if login was successful.
However, since version 1.5.4 it doesn't work. The issue is I don't get
an error message when login isn't successful (but in 1.5.3 I did get it).
Looking at the code, I noticed that in src/libguac/user.c <https://
github.com/apache/guacamole-server/blob/main/src/libguac/
user.c#L181> the error message is only sent to client-> socket, but as
far as I understand, because I am not logged in yet, my socket is
client->pending_socket, so I never get the error message. I fixed this
by changing this function to send to both sockets. So I copied the lines
that send to 1 socket to also send to the other socket:
guac_protocol_send_error(user->pending_socket, "Aborted. See
logs.", status);
guac_socket_flush(user->pending_socket);
Here is my implementation for the whole function:
void vguac_user_abort(guac_user* user, guac_protocol_status status,
const char* format, va_list ap) {
/* Only relevant if user is active */
if (user->active) {
/* Log detail of error */
vguac_user_log(user, GUAC_LOG_ERROR, format, ap);
/* Send error immediately, limit information given */
guac_protocol_send_error(user->socket, "Aborted. See logs.",
status);
guac_socket_flush(user->socket);
guac_protocol_send_error(user->pending_socket, "Aborted. See
logs.", status);
guac_socket_flush(user->pending_socket);
/* Stop user */
guac_user_stop(user);
}
}
So my questions are: Is the current behavior what's intended and I am
missing something? Or do you think that my change is a good fix?
Thanks!
Interesting.
I wouldn't say the current behavior is intentional per se. The
separation of pending users from fully joined users is intentional, but
communicating an error doesn't need that separation in the same way that
graphical updates do.
It makes sense that the new bulk join process could produce this
behavior, and that you would see this difference in 1.5.3 vs. 1.5.4 (the
pending_socket separation was introduced in 1.5.4 as part of the support
for bulk user join).
The fix you propose sounds sensible - I think it's worth a JIRA issue,
pull request, and review. I'd think it's vguac_client_abort() that would
need to be modified here, such that both joined and pending users
receive the "error" instruction. Further changes might be needed to
ensure that the abort/stop process is atomic, so that further users
cannot attempt to join after an abort (since those users would also
mysteriously not receive the error).
- Mike
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
