Need guidance with suspected auto-reconnect bug

[Jonathan Hankins <jh...@homewood.k12.al.us>]

I think I've found a bug, but I wanted to ask here before I file a bug
report, since I am new to the codebase.

I'm using 0.9.13.

I have a single connection to an RDP server. The server is set up to only
allow one connection per user, so when I leave one guacamole session
connected, and start up another one (e.g., open A in chrome and B in an
incognito window), the session detaches from A and attaches to B.

This (erroneously) triggers the 15 second auto-reconnect countdown in A.
Once it reconnects, B is disconnected and starts the 15 second countdown.
It ping-pongs back and forth.

Running "guacd -L debug -f" I see this:

1 ERRINFO_DISCONNECTED_BY_OTHER_CONNECTION (0x00000005):
2 Another user connected to the server, forcing the disconnection of the
current connection.
3 guacd[9344]: ERROR:     Error handling RDP file descriptors

Lines 1 and 2 actually come from libfreerdp, and indicate that the error
code coming from RDP is 0x5. Looking at guacd: src/protocols/rdp/error.c

     82         /* Another user logged on, disconnecting this user */
     83         case 0x5: /* ERRINFO_DISCONNECTED_BY_OTHER_CONNECTION */
     84             status = GUAC_PROTOCOL_STATUS_SESSION_CONFLICT;
     85             message = "Disconnected by other connection.";
     86             break;

So a 0x5 return from RDP should translate to
GUAC_PROTOCOL_STATUS_SESSION_CONFLICT (0x209) being sent to the client.

Looking at the client,
in guacamole/target/guacamole-0.9.13-incubating/guacamole.js:

  13877     /**
  13878      * All error codes for which automatic reconnection is
appropriate when a
  13879      * client error occurs.
  13880      */
  13881     var CLIENT_AUTO_RECONNECT = {
  13882         0x0200: true,
  13883         0x0202: true,
  13884         0x0203: true,
  13885         0x0207: true,
  13886         0x0208: true,
  13887         0x0301: true,
  13888         0x0308: true
  13889     };

0x0209 isn't in the list that should auto-reconnect. So I added this in
guacamole.js:

  console.log('status = ' + status.toString(16).toUpperCase());

In the session B console in Chrome, when the reconnect in session A, for
example, disconnects session B, I see this:

status = 208

That's not 209, but it IS in the auto-reconnect list.

So I started tracking down why libfreerdp sends 0x5 to guacd, which should
send 0x209 to the client, but instead is sending 0x208. Here's what I found
(the clue was line 3 above in the guacd debug output:

  guacd[9344]: ERROR:     Error handling RDP file descriptors

 In function guac_rdp_handle_connection:

    765                 /* Check the libfreerdp fds */
    766                 if (!freerdp_check_fds(rdp_inst)) {
    767                     guac_client_abort(client,
    768
 GUAC_PROTOCOL_STATUS_UPSTREAM_UNAVAILABLE,
    769                             "Error handling RDP file descriptors");
    770                     pthread_mutex_unlock(&(rdp_client->rdp_lock));
    771                     return 1;
    772                 }

and lower down:

    804                 /* Handle RDP disconnect */
    805                 if (freerdp_shall_disconnect(rdp_inst)) {
    806                     guac_rdp_client_abort(client);
    807                     pthread_mutex_unlock(&(rdp_client->rdp_lock));
    808                     return 1;
    809                 }
    810

the first bit is returning GUAC_PROTOCOL_STATUS_UPSTREAM_UNAVAILABLE, which
is 0x208, to the client and returning. This means guac_rdp_client_abort,
which maps the RDP error codes to GUAC_PROTOCOL_STATUS*, is never reached.
I poked it a bit and realized I was in over my head. I'd need to understand
more about freerdp_check_fds and freerdp_shall_disconnect before I could
decide how to rework it.

I am not sure what all error conditions from RDP will trigger the first bit
and return before guac_rdp_client_abort is called, but I am guessing that
more than 0x5 is getting short-circuited and arriving at the client as a
0x208.

If this needs to be filed as a bug report, I'd welcome advice to condense
it down -- I know I've been pretty verbose, but since I'm not certain about
the actual location of the logic error, I wanted to provide enough context
for someone who is more familiar with the codebase.

Thanks,

-Jonathan Hankins

-- 
This e-mail is intended only for the recipient and may contain confidential 
or proprietary information. If you are not the intended recipient, the 
review, distribution, duplication or retention of this message and its 
attachments is prohibited. Please notify the sender of this error 
immediately by reply e-mail, and permanently delete this message and its 
attachments in any form in which they may have been preserved.