Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle more disconnect reasons #155

Merged
merged 8 commits into from
Oct 5, 2017

Conversation

samhed
Copy link
Contributor

@samhed samhed commented Oct 4, 2017

This PR adds handling for 15 additional disconnect reasons that we can receive from the server:

ERRINFO_CB_DESTINATION_NOT_FOUND
ERRINFO_CB_LOADING_DESTINATION
ERRINFO_CB_REDIRECTING_TO_DESTINATION
ERRINFO_CB_SESSION_ONLINE_VM_WAKE
ERRINFO_CB_SESSION_ONLINE_VM_BOOT
ERRINFO_CB_SESSION_ONLINE_VM_NO_DNS
ERRINFO_CB_DESTINATION_POOL_NOT_FREE
ERRINFO_CB_CONNECTION_CANCELLED
ERRINFO_CB_CONNECTION_ERROR_INVALID_SETTINGS
ERRINFO_CB_SESSION_ONLINE_VM_BOOT_TIMEOUT
ERRINFO_CB_SESSION_ONLINE_VM_SESSMON_FAILED
ERRINFO_REMOTEAPPSNOTENABLED
ERRINFO_UPDATESESSIONKEYFAILED
ERRINFO_DECRYPTFAILED
ERRINFO_ENCRYPTFAILED

This is the reference I used for what the server sends: https://msdn.microsoft.com/en-us/library/cc240544.aspx

The PR also fixes a few messages that was incorrect for existing disconnect codes:

BEFORE:
+------------------------+-----------------+--------------------------------
|        server          |      action     |    output
+------------------------+-----------------+--------------------------------
| Windows Server 2008 R2 | user disconnect | "Disconnected by administration tool"
| Windows Server 2008 R2 | user logout     | -
| Windows Server 2012 R2 | user disconnect | "Disconnected by administration tool"
| Windows Server 2012 R2 | user logout     | "Disconnect initiated by user"
| Windows Server 2016    | user disconnect | "Disconnected by administration tool"
| Windows Server 2016    | user logout     | "Disconnect initiated by user"
+------------------------+-----------------+--------------------------------


AFTER:
+------------------------+-----------------+--------------------------------
|        server          |      action     |    output
+------------------------+-----------------+--------------------------------
| Windows Server 2008 R2 | user disconnect | "Disconnect initiated by user"
| Windows Server 2008 R2 | user logout     | -
| Windows Server 2012 R2 | user disconnect | "Disconnect initiated by user"
| Windows Server 2012 R2 | user logout     | "Logout initiated by user"
| Windows Server 2016    | user disconnect | "Disconnect initiated by user"
| Windows Server 2016    | user logout     | "Logout initiated by user"
+------------------------+-----------------+--------------------------------

And tweaks the wording on some other messages as well to better fit with what the server sends.

To make them easier to search for - let's match the names with the
corresponding names on the server side:

https://msdn.microsoft.com/en-us/library/cc240544.aspx
There seems to have been confusion with regards to which exit code and
message was returned by rdesktop for the following cases:

* disconnected by admin
* logged out by admin
* disconnect by user
* logoff by user

Looking at Microsoft's official documentation as well as testing using
Windows Server 2008 R2, 2012 R2 and 2016 reveals that this commit fixes
this issue. They do now match the reasons sent by the server.
They are now a bit more specific and better match the description of
the disconnection reasons sent by the server.
Most of the RPD protocol errors (reason > 0x1000) would only be
triggered by coding errors in the client. A few of them can occur due
to server errors however. We should attempt to handle these cases.
@derfian
Copy link
Member

derfian commented Oct 4, 2017

It would be nice if these new rdesktop return codes would be documented in the manual page, doc/rdesktop.1

@samhed
Copy link
Contributor Author

samhed commented Oct 4, 2017

Ah, missed that. Absolutely!

@samhed
Copy link
Contributor Author

samhed commented Oct 5, 2017

Added a commit which updates the manual page

@hean01-cendio hean01-cendio merged commit 12b28b8 into rdesktop:master Oct 5, 2017
@hean01-cendio hean01-cendio added this to the rdesktop-1.8.4 milestone Oct 5, 2017
@samhed samhed deleted the disconnectreasons branch October 5, 2017 11:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants