override keyboard infomation #1950 (1st) #1952
Conversation
TOMATO-ONE
force-pushed
the
xrdp_allow_overrite_keyboard
branch
from
7b7ed43
to
28a6de1
Compare
There was a problem hiding this comment.
Hi @TOMATO-ONE. Thanks for this. Sorry it took a while to get to it.
common/xrdp_client_info.h
Outdated
| @@ -172,9 +183,12 @@ struct xrdp_client_info | |||
|
|
|||
| int enable_token_login; | |||
| char domain_user_separator[16]; | |||
|
|
|||
| /* xrdp.overrode_* values */ | |||
There was a problem hiding this comment.
Small point. xrdp.override_* rather than xrdp.overrode_*.
There was a problem hiding this comment.
It was a typo. Sorry.
I will correct it.
libxrdp/xrdp_sec.c
Outdated
| if ((ko->type | ko->subtype | ko->layout) != 0) | ||
| { | ||
| LOG(LOG_LEVEL_INFO, "xrdp_load_keyboard_layout: Overrode keyboard" | ||
| " infomation, keyboard_type:[0x%02X], keyboard_subtype:[0x%02X]," | ||
| " keylayout:[0x%08X]", | ||
| client_info->keyboard_type, client_info->keyboard_subtype, | ||
| client_info->keylayout); | ||
| } |
There was a problem hiding this comment.
Suggest removing 410-417 entirely, and making the preceding messages LOG_LEVEL_INFO. You've already logged the client info at line 380-384. The override messages should also be at LOG_LEVEL_INFO level so we get a complete picture of the keyboard layout variables when running at LOG_LEVEL_INFO.
Otherwise we're just duplicating information when running at LOG_LEVEL_DEBUG.
There was a problem hiding this comment.
Thank you.
I was adding this for future code additions.
I've changed my mind and will remove lines 410-417.
I will change the override message to LOG_LEVEL_INFO.
common/xrdp_client_info.h
Outdated
| int original_type; | ||
| int original_subtype; | ||
| int original_layout; |
There was a problem hiding this comment.
I don't think the original_* values are needed(?)
There was a problem hiding this comment.
I had added it in case the original client information was needed after the override.
This is not needed for this PR and will be removed.
TOMATO-ONE
force-pushed
the
xrdp_allow_overrite_keyboard
branch
from
7b96aef
to
badc612
Compare
|
@metalefty - when you get a moment can you look at this? It looks like it should be useful for debugging keyboard issues. One thing which occurs to me is that tf we merge it for 0.9.17 we'll need to update xorgxrdp too, as the client_info has changed. At the moment that isn't necessary. |
libxrdp/xrdp_sec.c
Outdated
| client_info->keyboard_type, client_info->keyboard_subtype, | ||
| client_info->keylayout); | ||
|
|
||
| if (ko->type != 0) |
There was a problem hiding this comment.
I have found a bug in this part.
In xrdp.ini, put the following.
xrdp.override_keyboard_type.type = 0x00
In this case, for this if statement
It will not override client_info->keyboard_type .
The same is true for the next two.
TOMATO-ONE
force-pushed
the
xrdp_allow_overrite_keyboard
branch
from
b3bfdcc
to
7847b23
Compare
|
I set the initial value of the variable Please advise if there is a better way. |
|
I think that's a good a way as any other. |
|
LGTM. Needs to be triaged whether to include it in 0.9.17 or not. |
|
@matt335672 @metalefty This PR is part of the discussion #1950. Either way, I respect the thoughts of the team. |
|
I'm happy for this to go in to 0.9.17. As it stands, it doesn't cause any changes in behaviour that I can determine, so I see it as very low risk. |
This is a PR for Discussions #1950.
In this discussion we are considering several issues.
This is the first one.
add an option to force the RDP Client to overwrite any keyboard information sent to it with the specified value for debugging purposes.
I welcome suggestions for corrections to redundant code or confusing comments.