You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by "bbennett-ks (via GitHub)" <gi...@apache.org> on 2026/02/20 16:49:12 UTC

[PR] I choase 1. Now give a description of the changesGUACAMOLE-2220: Support Shift/Alt/Ctrl/Meta sequences for terminal navigation and function keys. [guacamole-server]

bbennett-ks opened a new pull request, #637:
URL: https://github.com/apache/guacamole-server/pull/637

   (no comment)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GUACAMOLE-2220: Support Shift/Alt/Ctrl/Meta sequences for terminal navigation and function keys. [guacamole-server]

Posted by "bbennett-ks (via GitHub)" <gi...@apache.org>.
bbennett-ks commented on PR #637:
URL: https://github.com/apache/guacamole-server/pull/637#issuecomment-4383504121

   > ... should the setting also be added to the Kubernetes protocol? I'm not sure if the Linux keycodes apply there the same way they do with Telnet and SSH?
   @necouchman @mike-jumper 
   I've added `terminal-type` to `Kubernetes`. I've tested Byobu in a Kubernetes pod and it works fine. However, there is an issue ...
   
   Guacamole supports two mechanisms to attaching to a (running) container:
   1. `attach`: Uses the Kubernetes `Attach` API to attach to the containers already-running main process/tty stream. This is used when the `Command (exec)` field is blank. With this mechanism, there is no way to "automatically" pass `TERM`. Though the user can do this manually.
   2. `exec`: Uses the `Exec` API to start a new process withing the container. For example, `/bin/bash`. With this mechanism, we can pass `TERM`.
   
   **The core issue is #1: Unlike `ssh` and `Telnet`, we cannot pass TERM.**
   
   I still think we should do it, to support Byobu, tmux... for power users.
   
   I can see several ways to handle this, none really ideal.
   
   1. For consistency (amongst Kubernetes), don't set `TERM` for `Exec`: Add note to Kubernetes setting `TERM` this must be done manually.
   2. If `Command (exec)` is blank, use `/bin/bash`. Or even have a `Default shell` field. (Uhg)
   3. Set `TERM` for `Exec`, and have a note that `TERM` needs to be set manually if `Command (exec)` is blank.
   4. Not support `terminal-type` for Kubernetes.
   
   I'm leaning towards #3...


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GUACAMOLE-2220: Support Shift/Alt/Ctrl/Meta sequences for terminal navigation and function keys. [guacamole-server]

Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on code in PR #637:
URL: https://github.com/apache/guacamole-server/pull/637#discussion_r3175638141


##########
src/terminal/terminal.c:
##########
@@ -1423,6 +1544,188 @@ int guac_terminal_send_string(guac_terminal* term, const char* data) {
 
 }
 
+/**
+ * Returns true if the given keysym is an arrow key.
+ */

Review Comment:
   These functions should all be documented using the same JSDoc-style comments that are generally used throughout the code. Really this just means adding the `@param` and `@return` tags.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GUACAMOLE-2220: Support Shift/Alt/Ctrl/Meta sequences for terminal navigation and function keys. [guacamole-server]

Posted by "bbennett-ks (via GitHub)" <gi...@apache.org>.
bbennett-ks commented on PR #637:
URL: https://github.com/apache/guacamole-server/pull/637#issuecomment-4364081480

   > ... should the setting also be added to the Kubernetes protocol? I'm not sure if the Linux keycodes apply there the same way they do with Telnet and SSH?
   
   Yep, sounds very reasonable to add terminal type to Kubernetes. I'll create a Kubernetes setup to confirm & validate.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GUACAMOLE-2220: Support Shift/Alt/Ctrl/Meta sequences for terminal navigation and function keys. [guacamole-server]

Posted by "bbennett-ks (via GitHub)" <gi...@apache.org>.
bbennett-ks commented on code in PR #637:
URL: https://github.com/apache/guacamole-server/pull/637#discussion_r3266765389


##########
src/terminal/terminal.c:
##########
@@ -55,6 +55,126 @@
 #include <guacamole/timestamp.h>
 #include <guacamole/user.h>
 
+/*
+ * Terminal key input is represented and encoded across several layers:
+ *
+ * 1) Incoming keys use X11 keysyms (e.g. XK_Left, XK_F1, modifier keysyms),
+ *    including Unicode keysyms (0x01000000 | codepoint).
+ *
+ * 2) Some Ctrl+key combinations map directly to ASCII C0 control bytes
+ *    (0x00-0x1F) plus DEL (0x7F), such as Ctrl+C -> ETX and Ctrl+[ -> ESC.
+ *
+ * 3) Non-printable/navigation/function keys are typically emitted as ANSI/xterm
+ *    escape sequences beginning with ESC (0x1B), including CSI sequences
+ *    (ESC [ ...), and xterm-style modifier parameters.
+ *    This includes arrow/navigation keys, function keys, and keypad variants;
+ *    when Shift/Alt/Ctrl/Meta are held, xterm-style modifier parameters are
+ *    appended to encode those combinations in the resulting sequence so
+ *    terminal applications can distinguish and bind keyboard shortcuts.
+ *
+ * Keyboard emulation strategy:
+ *     For all terminal types, keyboard emulation supports xterm-style extended
+ *     behavior for function keys (F1-F12), arrow keys (Up, Down, Left, Right),
+ *     cursor editing keys (Home, End, Insert, Delete, Page Up, Page Down), keypad
+ *     keys, and Ctrl/Shift/Alt/Meta modifier encoding via xterm-style CSI modifier
+ *     parameters (`ESC [ ... ; <mod> ...`), even if the configured terminal type
+ *     does not natively support these extended features.
+ *
+ *     The default terminal type (`linux`) preserves legacy Linux console keycodes
+ *     for F1-F5, keypad F1-F4, and Home/End to match the sequences expected by
+ *     `$TERM=linux` terminfo entries.
+ *
+ * References:
+ * - X.Org keysyms: https://cgit.freedesktop.org/xorg/proto/x11proto/tree/keysymdef.h
+ * - xterm control sequences: https://invisible-island.net/xterm/ctlseqs/ctlseqs.html
+ * - ASCII control codes (C0/DEL): https://www.rfc-editor.org/rfc/rfc20
+ */
+
+/* See X11 keysymdef.h */
+#define GUAC_KEYSYM_UNICODE_MASK    0xFFFF0000 /* Mask to test for 0x0100xxxx Unicode-keysym prefix. */
+#define GUAC_KEYSYM_UNICODE_PREFIX  0x01000000 /* X11 Unicode keysym prefix (0x01000000 | codepoint). */
+
+#define GUAC_KEYSYM_KP_ASCII_OFFSET 0xFF80 /* Keypad keysyms start at 0xFF80 (XK_KP_Space). */
+
+typedef enum guac_terminal_keysym {

Review Comment:
   Because of the density, I opted for the member comment style of typedef `enum guac_composite_mode`.  Let me know if you prefer the more prevalent style.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GUACAMOLE-2220: Support Shift/Alt/Ctrl/Meta sequences for terminal navigation and function keys. [guacamole-server]

Posted by "bbennett-ks (via GitHub)" <gi...@apache.org>.
bbennett-ks commented on PR #637:
URL: https://github.com/apache/guacamole-server/pull/637#issuecomment-4384341628

   **Command specified: TERM passed to session.**
   <img width="337" height="867" alt="image" src="https://github.com/user-attachments/assets/c7dca914-e5fd-4f33-92ae-1fc88657dfc3" />
   
   **Command not specified: TERM not passed  passed to session.**
   <img width="410" height="849" alt="image" src="https://github.com/user-attachments/assets/763b8936-8cbe-4bbf-b73a-38e25dd219f8" />
   
   I have a UI change that makes option #3 (Set TERM for Exec, and have a note that TERM needs to be set manually if Command (exec) is blank) more palatable. I've added support for the ability for the protocol Json files to dynamically change field labels base on the the value of other fields. e.g. a `terminal-type` field label `Terminal type (TERM must be set manually` is displayed if `Command (exec):` is empty. 
   
   This done by adding `conditionalLabel` to the field definition, an optional condition that allows a field's label to vary based on the value of another field (in the same form).
   ```
                   {
                       "name"      : "terminal-type",
                       "type"      : "ENUM",
                       "options"   : [ "", "xterm", "xterm-256color", "vt220", "vt100", "ansi", "linux" ],
                       "conditionalLabel" : {
                           "field"    : "exec-command",
                           "equals"   : "",
                           "labelKey" : "FIELD_HEADER_TERMINAL_TYPE_MANUAL_TERM"
                       }
                   }
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GUACAMOLE-2220: Support Shift/Alt/Ctrl/Meta sequences for terminal navigation and function keys. [guacamole-server]

Posted by "bbennett-ks (via GitHub)" <gi...@apache.org>.
bbennett-ks commented on code in PR #637:
URL: https://github.com/apache/guacamole-server/pull/637#discussion_r3266754355


##########
src/terminal/terminal.c:
##########
@@ -55,6 +55,126 @@
 #include <guacamole/timestamp.h>
 #include <guacamole/user.h>
 
+/*
+ * Terminal key input is represented and encoded across several layers:
+ *
+ * 1) Incoming keys use X11 keysyms (e.g. XK_Left, XK_F1, modifier keysyms),
+ *    including Unicode keysyms (0x01000000 | codepoint).
+ *
+ * 2) Some Ctrl+key combinations map directly to ASCII C0 control bytes
+ *    (0x00-0x1F) plus DEL (0x7F), such as Ctrl+C -> ETX and Ctrl+[ -> ESC.
+ *
+ * 3) Non-printable/navigation/function keys are typically emitted as ANSI/xterm
+ *    escape sequences beginning with ESC (0x1B), including CSI sequences
+ *    (ESC [ ...), and xterm-style modifier parameters.
+ *    This includes arrow/navigation keys, function keys, and keypad variants;
+ *    when Shift/Alt/Ctrl/Meta are held, xterm-style modifier parameters are
+ *    appended to encode those combinations in the resulting sequence so
+ *    terminal applications can distinguish and bind keyboard shortcuts.
+ *
+ * Keyboard emulation strategy:
+ *     For all terminal types, keyboard emulation supports xterm-style extended
+ *     behavior for function keys (F1-F12), arrow keys (Up, Down, Left, Right),
+ *     cursor editing keys (Home, End, Insert, Delete, Page Up, Page Down), keypad
+ *     keys, and Ctrl/Shift/Alt/Meta modifier encoding via xterm-style CSI modifier
+ *     parameters (`ESC [ ... ; <mod> ...`), even if the configured terminal type
+ *     does not natively support these extended features.
+ *
+ *     The default terminal type (`linux`) preserves legacy Linux console keycodes
+ *     for F1-F5, keypad F1-F4, and Home/End to match the sequences expected by
+ *     `$TERM=linux` terminfo entries.
+ *
+ * References:
+ * - X.Org keysyms: https://cgit.freedesktop.org/xorg/proto/x11proto/tree/keysymdef.h
+ * - xterm control sequences: https://invisible-island.net/xterm/ctlseqs/ctlseqs.html
+ * - ASCII control codes (C0/DEL): https://www.rfc-editor.org/rfc/rfc20
+ */
+
+/* See X11 keysymdef.h */
+#define GUAC_KEYSYM_UNICODE_MASK    0xFFFF0000 /* Mask to test for 0x0100xxxx Unicode-keysym prefix. */
+#define GUAC_KEYSYM_UNICODE_PREFIX  0x01000000 /* X11 Unicode keysym prefix (0x01000000 | codepoint). */
+
+#define GUAC_KEYSYM_KP_ASCII_OFFSET 0xFF80 /* Keypad keysyms start at 0xFF80 (XK_KP_Space). */

Review Comment:
   Done!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GUACAMOLE-2220: Support Shift/Alt/Ctrl/Meta sequences for terminal navigation and function keys. [guacamole-server]

Posted by "bbennett-ks (via GitHub)" <gi...@apache.org>.
bbennett-ks commented on code in PR #637:
URL: https://github.com/apache/guacamole-server/pull/637#discussion_r3266766917


##########
src/protocols/kubernetes/url.c:
##########
@@ -89,6 +90,72 @@ int guac_kubernetes_escape_url_component(char* output, int length,
 
 }
 
+/**
+ * Splits exec_command on whitespace and appends each argument as a separate
+ * Kubernetes "command=..." query parameter.
+ *
+ * Quoting is not supported. "$TERM" is replaced with terminal_type unless it
+ * is part of a longer shell variable name, such as "$TERMINAL".
+ *
+ * @return Zero on success, non-zero if the buffer is too small.
+ */

Review Comment:
   Done!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GUACAMOLE-2220: Support Shift/Alt/Ctrl/Meta sequences for terminal navigation and function keys. [guacamole-server]

Posted by "bbennett-ks (via GitHub)" <gi...@apache.org>.
bbennett-ks commented on PR #637:
URL: https://github.com/apache/guacamole-server/pull/637#issuecomment-3936116031

   I'm keeping this as a draft until I finish re-testing after a few more changes.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GUACAMOLE-2220: Support Shift/Alt/Ctrl/Meta sequences for terminal navigation and function keys. [guacamole-server]

Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman merged PR #637:
URL: https://github.com/apache/guacamole-server/pull/637


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GUACAMOLE-2220: Support Shift/Alt/Ctrl/Meta sequences for terminal navigation and function keys. [guacamole-server]

Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on code in PR #637:
URL: https://github.com/apache/guacamole-server/pull/637#discussion_r3244260987


##########
src/terminal/terminal.c:
##########
@@ -55,6 +55,126 @@
 #include <guacamole/timestamp.h>
 #include <guacamole/user.h>
 
+/*
+ * Terminal key input is represented and encoded across several layers:
+ *
+ * 1) Incoming keys use X11 keysyms (e.g. XK_Left, XK_F1, modifier keysyms),
+ *    including Unicode keysyms (0x01000000 | codepoint).
+ *
+ * 2) Some Ctrl+key combinations map directly to ASCII C0 control bytes
+ *    (0x00-0x1F) plus DEL (0x7F), such as Ctrl+C -> ETX and Ctrl+[ -> ESC.
+ *
+ * 3) Non-printable/navigation/function keys are typically emitted as ANSI/xterm
+ *    escape sequences beginning with ESC (0x1B), including CSI sequences
+ *    (ESC [ ...), and xterm-style modifier parameters.
+ *    This includes arrow/navigation keys, function keys, and keypad variants;
+ *    when Shift/Alt/Ctrl/Meta are held, xterm-style modifier parameters are
+ *    appended to encode those combinations in the resulting sequence so
+ *    terminal applications can distinguish and bind keyboard shortcuts.
+ *
+ * Keyboard emulation strategy:
+ *     For all terminal types, keyboard emulation supports xterm-style extended
+ *     behavior for function keys (F1-F12), arrow keys (Up, Down, Left, Right),
+ *     cursor editing keys (Home, End, Insert, Delete, Page Up, Page Down), keypad
+ *     keys, and Ctrl/Shift/Alt/Meta modifier encoding via xterm-style CSI modifier
+ *     parameters (`ESC [ ... ; <mod> ...`), even if the configured terminal type
+ *     does not natively support these extended features.
+ *
+ *     The default terminal type (`linux`) preserves legacy Linux console keycodes
+ *     for F1-F5, keypad F1-F4, and Home/End to match the sequences expected by
+ *     `$TERM=linux` terminfo entries.
+ *
+ * References:
+ * - X.Org keysyms: https://cgit.freedesktop.org/xorg/proto/x11proto/tree/keysymdef.h
+ * - xterm control sequences: https://invisible-island.net/xterm/ctlseqs/ctlseqs.html
+ * - ASCII control codes (C0/DEL): https://www.rfc-editor.org/rfc/rfc20
+ */
+
+/* See X11 keysymdef.h */
+#define GUAC_KEYSYM_UNICODE_MASK    0xFFFF0000 /* Mask to test for 0x0100xxxx Unicode-keysym prefix. */
+#define GUAC_KEYSYM_UNICODE_PREFIX  0x01000000 /* X11 Unicode keysym prefix (0x01000000 | codepoint). */
+
+#define GUAC_KEYSYM_KP_ASCII_OFFSET 0xFF80 /* Keypad keysyms start at 0xFF80 (XK_KP_Space). */

Review Comment:
   I'd suggest that all three of these be properly documented rather than having the end-of-line documentation.



##########
src/terminal/terminal.c:
##########
@@ -55,6 +55,126 @@
 #include <guacamole/timestamp.h>
 #include <guacamole/user.h>
 
+/*
+ * Terminal key input is represented and encoded across several layers:
+ *
+ * 1) Incoming keys use X11 keysyms (e.g. XK_Left, XK_F1, modifier keysyms),
+ *    including Unicode keysyms (0x01000000 | codepoint).
+ *
+ * 2) Some Ctrl+key combinations map directly to ASCII C0 control bytes
+ *    (0x00-0x1F) plus DEL (0x7F), such as Ctrl+C -> ETX and Ctrl+[ -> ESC.
+ *
+ * 3) Non-printable/navigation/function keys are typically emitted as ANSI/xterm
+ *    escape sequences beginning with ESC (0x1B), including CSI sequences
+ *    (ESC [ ...), and xterm-style modifier parameters.
+ *    This includes arrow/navigation keys, function keys, and keypad variants;
+ *    when Shift/Alt/Ctrl/Meta are held, xterm-style modifier parameters are
+ *    appended to encode those combinations in the resulting sequence so
+ *    terminal applications can distinguish and bind keyboard shortcuts.
+ *
+ * Keyboard emulation strategy:
+ *     For all terminal types, keyboard emulation supports xterm-style extended
+ *     behavior for function keys (F1-F12), arrow keys (Up, Down, Left, Right),
+ *     cursor editing keys (Home, End, Insert, Delete, Page Up, Page Down), keypad
+ *     keys, and Ctrl/Shift/Alt/Meta modifier encoding via xterm-style CSI modifier
+ *     parameters (`ESC [ ... ; <mod> ...`), even if the configured terminal type
+ *     does not natively support these extended features.
+ *
+ *     The default terminal type (`linux`) preserves legacy Linux console keycodes
+ *     for F1-F5, keypad F1-F4, and Home/End to match the sequences expected by
+ *     `$TERM=linux` terminfo entries.
+ *
+ * References:
+ * - X.Org keysyms: https://cgit.freedesktop.org/xorg/proto/x11proto/tree/keysymdef.h
+ * - xterm control sequences: https://invisible-island.net/xterm/ctlseqs/ctlseqs.html
+ * - ASCII control codes (C0/DEL): https://www.rfc-editor.org/rfc/rfc20
+ */
+
+/* See X11 keysymdef.h */
+#define GUAC_KEYSYM_UNICODE_MASK    0xFFFF0000 /* Mask to test for 0x0100xxxx Unicode-keysym prefix. */
+#define GUAC_KEYSYM_UNICODE_PREFIX  0x01000000 /* X11 Unicode keysym prefix (0x01000000 | codepoint). */
+
+#define GUAC_KEYSYM_KP_ASCII_OFFSET 0xFF80 /* Keypad keysyms start at 0xFF80 (XK_KP_Space). */
+
+typedef enum guac_terminal_keysym {

Review Comment:
   Please document this `typedef`.



##########
src/protocols/kubernetes/url.c:
##########
@@ -89,6 +90,72 @@ int guac_kubernetes_escape_url_component(char* output, int length,
 
 }
 
+/**
+ * Splits exec_command on whitespace and appends each argument as a separate
+ * Kubernetes "command=..." query parameter.
+ *
+ * Quoting is not supported. "$TERM" is replaced with terminal_type unless it
+ * is part of a longer shell variable name, such as "$TERMINAL".
+ *
+ * @return Zero on success, non-zero if the buffer is too small.
+ */

Review Comment:
   This documentation block needs:
   * `@param` tags for the parameters.
   * Proper formatting in the `@return` tag (newline).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GUACAMOLE-2220: Support Shift/Alt/Ctrl/Meta sequences for terminal navigation and function keys. [guacamole-server]

Posted by "bbennett-ks (via GitHub)" <gi...@apache.org>.
bbennett-ks commented on PR #637:
URL: https://github.com/apache/guacamole-server/pull/637#issuecomment-4390278518

   I've pushed an additional commit, adding Kubernetes support for `terminal-type`. I've for option #1 (For consistency (amongst Kubernetes), don't set TERM), as passing `TERM` by `env` breaks non-POSIX containers and other containers that have minimal environments that don't support `env`.
   
   Instead, `$TERM` can be used in the  Exec command to pass the `terminal-type` to the container. e.g. `env TERM=$TERM /bin/bash`. This will be included in the documentation. I corresponding client and manual PRs will be created.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org