GUACAMOLE-518: Merge correct behavior of keys typed with AltGr.

src/protocols/rdp/keyboard.c

@@ -28,39 +28,6 @@
 
 #include <stdlib.h>
 
-/**
- * Translates the given keysym into the corresponding modifier flag, as defined
- * by keymap.h. If the given keysym does not represent a modifier key, zero is
- * returned.
- *
- * @param keysym
- *     The keysym to translate into a modifier flag.
- *
- * @return
- *     The modifier flag which corresponds to the given keysym, or zero if the
- *     given keysym does not represent a modifier key.
- */
-static int guac_rdp_keyboard_modifier_flag(int keysym) {
-
-    /* Translate keysym into corresponding modifier flag */
-    switch (keysym) {
-
-        /* Shift */
-        case 0xFFE1:
-        case 0xFFE2:
-            return GUAC_RDP_KEYMAP_MODIFIER_SHIFT;
-
-        /* AltGr */
-        case 0xFE03:
-            return GUAC_RDP_KEYMAP_MODIFIER_ALTGR;
-
-    }
-
-    /* Not a modifier */
-    return 0;
-
-}
-
 /**
  * Translates the given keysym into the corresponding lock flag, as would be
  * required by the RDP synchronize event. If the given keysym does not
@@ -79,19 +46,19 @@ static int guac_rdp_keyboard_lock_flag(int keysym) {
     switch (keysym) {
 
         /* Scroll lock */
-        case 0xFF14:
+        case GUAC_RDP_KEYSYM_SCROLL_LOCK:
             return KBD_SYNC_SCROLL_LOCK;
 
         /* Kana lock */
-        case 0xFF2D:
+        case GUAC_RDP_KEYSYM_KANA_LOCK:
             return KBD_SYNC_KANA_LOCK;
 
         /* Num lock */
-        case 0xFF7F:
+        case GUAC_RDP_KEYSYM_NUM_LOCK:
             return KBD_SYNC_NUM_LOCK;
 
         /* Caps lock */
-        case 0xFFE5:
+        case GUAC_RDP_KEYSYM_CAPS_LOCK:
             return KBD_SYNC_CAPS_LOCK;
 
     }
@@ -288,6 +255,8 @@ static int guac_rdp_count_bits(unsigned int value) {
 static int guac_rdp_keyboard_get_cost(guac_rdp_keyboard* keyboard,
         const guac_rdp_keysym_desc* def) {
 
+    unsigned int modifier_flags = guac_rdp_keyboard_get_modifier_flags(keyboard);
+
     /* Each change to any key requires one event, by definition */
     int cost = 1;
 
@@ -296,7 +265,7 @@ static int guac_rdp_keyboard_get_cost(guac_rdp_keyboard* keyboard,
     cost += guac_rdp_count_bits(update_locks) * 2;
 
     /* Each change to a modifier requires one key event */
-    unsigned int update_modifiers = (def->clear_modifiers & keyboard->modifier_flags) | (def->set_modifiers & ~keyboard->modifier_flags);
+    unsigned int update_modifiers = (def->clear_modifiers & modifier_flags) | (def->set_modifiers & ~modifier_flags);
     cost += guac_rdp_count_bits(update_modifiers);
 
     return cost;
@@ -489,6 +458,37 @@ int guac_rdp_keyboard_is_defined(guac_rdp_keyboard* keyboard, int keysym) {
 
 }
 
+int guac_rdp_keyboard_is_pressed(guac_rdp_keyboard* keyboard, int keysym) {
+
+    guac_rdp_key* key = guac_rdp_keyboard_get_key(keyboard, keysym);
+    return key != NULL && key->pressed != NULL;
+
+}
+
+unsigned int guac_rdp_keyboard_get_modifier_flags(guac_rdp_keyboard* keyboard) {
+
+    unsigned int modifier_flags = 0;
+
+    /* Shift */
+    if (guac_rdp_keyboard_is_pressed(keyboard, GUAC_RDP_KEYSYM_LSHIFT)
+            || guac_rdp_keyboard_is_pressed(keyboard, GUAC_RDP_KEYSYM_RSHIFT))
+        modifier_flags |= GUAC_RDP_KEYMAP_MODIFIER_SHIFT;
+
+    /* Dedicated AltGr key */
+    if (guac_rdp_keyboard_is_pressed(keyboard, GUAC_RDP_KEYSYM_RALT)
+            || guac_rdp_keyboard_is_pressed(keyboard, GUAC_RDP_KEYSYM_ALTGR))
+        modifier_flags |= GUAC_RDP_KEYMAP_MODIFIER_ALTGR;
+
+    /* AltGr via Ctrl+Alt */
+    if (guac_rdp_keyboard_is_pressed(keyboard, GUAC_RDP_KEYSYM_LALT)
+            && (guac_rdp_keyboard_is_pressed(keyboard, GUAC_RDP_KEYSYM_RCTRL)
+                || guac_rdp_keyboard_is_pressed(keyboard, GUAC_RDP_KEYSYM_LCTRL)))
+        modifier_flags |= GUAC_RDP_KEYMAP_MODIFIER_ALTGR;
+
+    return modifier_flags;
+
+}
+
 /**
  * Presses/releases the requested key by sending one or more RDP key events, as
  * defined within the keymap defining that key.
@@ -599,27 +599,33 @@ void guac_rdp_keyboard_update_locks(guac_rdp_keyboard* keyboard,
 void guac_rdp_keyboard_update_modifiers(guac_rdp_keyboard* keyboard,
         unsigned int set_flags, unsigned int clear_flags) {
 
+    unsigned int modifier_flags = guac_rdp_keyboard_get_modifier_flags(keyboard);
+
     /* Only clear modifiers that are set */
-    clear_flags &= keyboard->modifier_flags;
+    clear_flags &= modifier_flags;
 
     /* Only set modifiers that are currently cleared */
-    set_flags &= ~keyboard->modifier_flags;
+    set_flags &= ~modifier_flags;
 
     /* Press/release Shift as needed */
     if (set_flags & GUAC_RDP_KEYMAP_MODIFIER_SHIFT) {
-        guac_rdp_keyboard_update_keysym(keyboard, 0xFFE1, 1, GUAC_RDP_KEY_SOURCE_SYNTHETIC);
+        guac_rdp_keyboard_update_keysym(keyboard, GUAC_RDP_KEYSYM_LSHIFT, 1, GUAC_RDP_KEY_SOURCE_SYNTHETIC);
     }
     else if (clear_flags & GUAC_RDP_KEYMAP_MODIFIER_SHIFT) {
-        guac_rdp_keyboard_update_keysym(keyboard, 0xFFE1, 0, GUAC_RDP_KEY_SOURCE_SYNTHETIC);
+        guac_rdp_keyboard_update_keysym(keyboard, GUAC_RDP_KEYSYM_LSHIFT, 0, GUAC_RDP_KEY_SOURCE_SYNTHETIC);
-        guac_rdp_keyboard_update_keysym(keyboard, 0xFFE2, 0, GUAC_RDP_KEY_SOURCE_SYNTHETIC);
+        guac_rdp_keyboard_update_keysym(keyboard, GUAC_RDP_KEYSYM_RSHIFT, 0, GUAC_RDP_KEY_SOURCE_SYNTHETIC);
     }
 
     /* Press/release AltGr as needed */
     if (set_flags & GUAC_RDP_KEYMAP_MODIFIER_ALTGR) {
-        guac_rdp_keyboard_update_keysym(keyboard, 0xFE03, 1, GUAC_RDP_KEY_SOURCE_SYNTHETIC);
+        guac_rdp_keyboard_update_keysym(keyboard, GUAC_RDP_KEYSYM_ALTGR, 1, GUAC_RDP_KEY_SOURCE_SYNTHETIC);
     }
     else if (clear_flags & GUAC_RDP_KEYMAP_MODIFIER_ALTGR) {
-        guac_rdp_keyboard_update_keysym(keyboard, 0xFE03, 0, GUAC_RDP_KEY_SOURCE_SYNTHETIC);
+        guac_rdp_keyboard_update_keysym(keyboard, GUAC_RDP_KEYSYM_ALTGR, 0, GUAC_RDP_KEY_SOURCE_SYNTHETIC);
+        guac_rdp_keyboard_update_keysym(keyboard, GUAC_RDP_KEYSYM_LALT, 0, GUAC_RDP_KEY_SOURCE_SYNTHETIC);
+        guac_rdp_keyboard_update_keysym(keyboard, GUAC_RDP_KEYSYM_RALT, 0, GUAC_RDP_KEY_SOURCE_SYNTHETIC);
+        guac_rdp_keyboard_update_keysym(keyboard, GUAC_RDP_KEYSYM_LCTRL, 0, GUAC_RDP_KEY_SOURCE_SYNTHETIC);
+        guac_rdp_keyboard_update_keysym(keyboard, GUAC_RDP_KEYSYM_RCTRL, 0, GUAC_RDP_KEY_SOURCE_SYNTHETIC);
     }
 
 }
@@ -639,60 +645,54 @@ int guac_rdp_keyboard_update_keysym(guac_rdp_keyboard* keyboard,
 
     }
 
-    /* If key is known, ignoring the key event entirely if state is not
-     * actually changing */
     guac_rdp_key* key = guac_rdp_keyboard_get_key(keyboard, keysym);
-    if (key != NULL) {
+
-        if ((!pressed && key->pressed == NULL) || (pressed && key->pressed != NULL))
+    /* Update tracking of client-side keyboard state but only for keys which
-            return 0;
+     * are tracked server-side, as well (to ensure that the key count remains
+     * correct, even if a user sends extra unbalanced or excessive press and
+     * release events) */
+    if (source == GUAC_RDP_KEY_SOURCE_CLIENT && key != NULL) {
+        if (pressed && !key->user_pressed) {
+            keyboard->user_pressed_keys++;
+            key->user_pressed = 1;
+        }
+        else if (!pressed && key->user_pressed) {
+            keyboard->user_pressed_keys--;
+            key->user_pressed = 0;
+        }
     }
 
-    /* Toggle locks and set modifiers on keydown */
+    /* Send events and update server-side lock state only if server-side key
-    if (pressed) {
+     * state is changing (or if server-side state of this key is untracked) */
-        keyboard->lock_flags ^= guac_rdp_keyboard_lock_flag(keysym);
+    if (key == NULL || (pressed && key->pressed == NULL) || (!pressed && key->pressed != NULL)) {
-        keyboard->modifier_flags |= guac_rdp_keyboard_modifier_flag(keysym);
-    }
 
-    /* Clear modifiers on keyup */
+        /* Toggle locks on keydown */
-    else {
+        if (pressed)
-        keyboard->modifier_flags &= ~guac_rdp_keyboard_modifier_flag(keysym);
+            keyboard->lock_flags ^= guac_rdp_keyboard_lock_flag(keysym);
-    }
 
-    /* If key is known, update state and attempt to send using normal RDP key
+        /* If key is known, update state and attempt to send using normal RDP key
-     * events */
+         * events */
-    const guac_rdp_keysym_desc* definition = NULL;
+        const guac_rdp_keysym_desc* definition = NULL;
-    if (key != NULL) {
-        definition = guac_rdp_keyboard_send_defined_key(keyboard, key, pressed);
-        key->pressed = pressed ? definition : NULL;
-    }
-
-    /* Fall back to dead keys or Unicode events if otherwise undefined inside
-     * current keymap (note that we only handle "pressed" here, as neither
-     * Unicode events nor dead keys can have a pressed/released state) */
-    if (definition == NULL && pressed) {
-        guac_rdp_keyboard_send_missing_key(keyboard, keysym);
-    }
-
-    if (source == GUAC_RDP_KEY_SOURCE_CLIENT) {
-
-        /* Update tracking of client-side keyboard state but only for keys
-         * which are tracked server-side, as well (to ensure that the key count
-         * remains correct, even if a user sends extra unbalanced or excessive
-         * press and release events) */
         if (key != NULL) {
-            if (pressed)
+            definition = guac_rdp_keyboard_send_defined_key(keyboard, key, pressed);
-                keyboard->user_pressed_keys++;
+            key->pressed = pressed ? definition : NULL;
-            else
-                keyboard->user_pressed_keys--;
         }
 
-        /* Reset RDP server keyboard state (releasing any automatically pressed
+        /* Fall back to dead keys or Unicode events if otherwise undefined inside
-         * keys) once all keys have been released on the client side */
+         * current keymap (note that we only handle "pressed" here, as neither
-        if (keyboard->user_pressed_keys == 0)
+         * Unicode events nor dead keys can have a pressed/released state) */
-            guac_rdp_keyboard_reset(keyboard);
+        if (definition == NULL && pressed) {
+            guac_rdp_keyboard_send_missing_key(keyboard, keysym);
+        }
 
     }
 
+    /* Reset RDP server keyboard state (releasing any automatically
+     * pressed keys) once all keys have been released on the client
+     * side */
+    if (source == GUAC_RDP_KEY_SOURCE_CLIENT && keyboard->user_pressed_keys == 0)
+        guac_rdp_keyboard_reset(keyboard);
+
     return 0;
 
 }

src/protocols/rdp/keyboard.h

@@ -84,6 +84,12 @@ typedef struct guac_rdp_key {
      */
     const guac_rdp_keysym_desc* pressed;
 
+    /**
+     * Whether this key is currently pressed by the user, and is included among
+     * the total tracked by user_pressed_keys within guac_rdp_keyboard.
+     */
+    int user_pressed;
+
 } guac_rdp_key;
 
 /**
@@ -97,15 +103,6 @@ typedef struct guac_rdp_keyboard {
      */
     guac_client* client;
 
-    /**
-     * The local state of all known modifier keys, as a bitwise OR of the
-     * modified flags used by the keymaps.
-     *
-     * @see GUAC_RDP_KEYMAP_MODIFIER_SHIFT
-     * @see GUAC_RDP_KEYMAP_MODIFIER_ALTGR
-     */
-    unsigned int modifier_flags;
-
     /**
      * The local state of all known lock keys, as a bitwise OR of all RDP lock
      * key flags. Legal flags are KBD_SYNC_SCROLL_LOCK, KBD_SYNC_NUM_LOCK,
@@ -207,6 +204,37 @@ void guac_rdp_keyboard_free(guac_rdp_keyboard* keyboard);
  */
 int guac_rdp_keyboard_is_defined(guac_rdp_keyboard* keyboard, int keysym);
 
+/**
+ * Returns whether the key having the given keysym is currently pressed.
+ *
+ * @param keyboard
+ *     The guac_rdp_keyboard instance to check.
+ *
+ * @param keysym
+ *     The keysym of the key being checked.
+ *
+ * @return
+ *     Non-zero if the key is currently pressed, zero otherwise.
+ */
+int guac_rdp_keyboard_is_pressed(guac_rdp_keyboard* keyboard, int keysym);
+
+/**
+ * Returns the local state of all known modifier keys, as a bitwise OR of the
+ * modifier flags used by the keymaps. Alternative methods of producing the
+ * effect of certain modifiers, such as holding Ctrl+Alt for AltGr when a
+ * dedicated AltGr key is unavailable, are taken into account.
+ *
+ * @see GUAC_RDP_KEYMAP_MODIFIER_SHIFT
+ * @see GUAC_RDP_KEYMAP_MODIFIER_ALTGR
+ *
+ * @param keyboard
+ *     The guac_rdp_keyboard associated with the current RDP session.
+ *
+ * @return
+ *     The local state of all known modifier keys.
+ */
+unsigned int guac_rdp_keyboard_get_modifier_flags(guac_rdp_keyboard* keyboard);
+
 /**
  * Updates the local state of the lock keys (such as Caps lock or Num lock),
  * synchronizing the remote state of those keys if it is expected to differ.

src/protocols/rdp/keymap.c

@@ -21,28 +21,6 @@
 
 #include <string.h>
 
-const int GUAC_KEYSYMS_SHIFT[] = {0xFFE1, 0};
-const int GUAC_KEYSYMS_ALL_SHIFT[] = {0xFFE1, 0xFFE2, 0};
-
-const int GUAC_KEYSYMS_ALTGR[] = {0xFFEA, 0};
-const int GUAC_KEYSYMS_SHIFT_ALTGR[] = {0xFFE1, 0xFFEA, 0};
-const int GUAC_KEYSYMS_ALL_SHIFT_ALTGR[] = {0xFFE1, 0xFFE2, 0xFFEA, 0};
-
-const int GUAC_KEYSYMS_CTRL[] = {0xFFE3, 0};
-const int GUAC_KEYSYMS_ALL_CTRL[] = {0xFFE3, 0xFFE4, 0};
-
-const int GUAC_KEYSYMS_ALT[] = {0xFFE9, 0};
-const int GUAC_KEYSYMS_ALL_ALT[] = {0xFFE9, 0xFFEA, 0};
-
-const int GUAC_KEYSYMS_CTRL_ALT[] = {0xFFE3, 0xFFE9, 0};
-
-const int GUAC_KEYSYMS_ALL_MODIFIERS[] = {
-    0xFFE1, 0xFFE2, /* Left and right shift */
-    0xFFE3, 0xFFE4, /* Left and right control */
-    0xFFE9, 0xFFEA, /* Left and right alt (AltGr) */
-    0
-};
-
 const guac_rdp_keymap* guac_rdp_keymap_find(const char* name) {
 
     /* For each keymap */

src/protocols/rdp/keymap.h

@@ -22,6 +22,61 @@
 
 #include <winpr/wtypes.h>
 
+/**
+ * The X11 keysym for Num Lock.
+ */
+#define GUAC_RDP_KEYSYM_NUM_LOCK 0xFF7F
+
+/**
+ * The X11 keysym for Scroll Lock.
+ */
+#define GUAC_RDP_KEYSYM_SCROLL_LOCK 0xFF14
+
+/**
+ * The X11 keysym for Caps Lock.
+ */
+#define GUAC_RDP_KEYSYM_CAPS_LOCK 0xFFE5
+
+/**
+ * The X11 keysym for Kana Lock.
+ */
+#define GUAC_RDP_KEYSYM_KANA_LOCK 0xFF2D
+
+/**
+ * The X11 keysym for Left Shift.
+ */
+#define GUAC_RDP_KEYSYM_LSHIFT 0xFFE1
+
+/**
+ * The X11 keysym for Right Shift.
+ */
+#define GUAC_RDP_KEYSYM_RSHIFT 0xFFE2
+
+/**
+ * The X11 keysym for Left Ctrl.
+ */
+#define GUAC_RDP_KEYSYM_LCTRL 0xFFE3
+
+/**
+ * The X11 keysym for Right Ctrl.
+ */
+#define GUAC_RDP_KEYSYM_RCTRL 0xFFE4
+
+/**
+ * The X11 keysym for Left Alt.
+ */
+#define GUAC_RDP_KEYSYM_LALT 0xFFE9
+
+/**
+ * The X11 keysym for Right Alt.
+ */
+#define GUAC_RDP_KEYSYM_RALT 0xFFEA
+
+/**
+ * The X11 keysym for AltGr.
+ */
+#define GUAC_RDP_KEYSYM_ALTGR 0xFE03
+
 /**
  * Bitwise flag value representing the Shift modifier.
  */

src/protocols/rdp/keymaps/base.keymap

@@ -84,6 +84,7 @@ map      0x1D ~ 0xffe3 # Control_L
 map +ext 0x1D ~ 0xffe4 # Control_R
 map      0x38 ~ 0xffe9 # Alt_L
 map +ext 0x38 ~ 0xffea # Alt_R
+map +ext 0x38 ~ 0xfe03 # AltGr
 map +ext 0x5B ~ 0xffeb # Super_L
 map +ext 0x5C ~ 0xffec # Super_R
 map +ext 0x5D ~ 0xff67 # Menu