You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by darchons <gi...@git.apache.org> on 2018/05/16 04:54:44 UTC
[GitHub] guacamole-server pull request #166: GUACAMOLE-564: Hide APC escape sequence.
GitHub user darchons opened a pull request:
https://github.com/apache/guacamole-server/pull/166
GUACAMOLE-564: Hide APC escape sequence.
I see some programs emitting these sequences. An APC escape sequence contains an arbitrary string command between (ESC _) and (ESC \) sequences. While we don't support any APC commands, we
should ignore any commands that we do encounter so they're not echoed onto the screen.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/darchons/guacamole-server guac-564-hide-apc-escape-sequence
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/guacamole-server/pull/166.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #166
----
commit e146c5c8bace64ad51503b77ac52aec7d17d79fa
Author: Jim Chen <nc...@...>
Date: 2018-01-10T03:36:46Z
GUACAMOLE-564: Hide APC escape sequence.
An APC escape sequence contains an arbitrary string command between
(ESC _) and (ESC \) sequences. While we don't support any APC commands, we
should ignore any commands that we do encounter.
----
---
[GitHub] guacamole-server pull request #166: GUACAMOLE-564: Hide APC escape sequence.
Posted by mike-jumper <gi...@git.apache.org>.
Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/guacamole-server/pull/166#discussion_r189178815
--- Diff: src/terminal/terminal_handlers.c ---
@@ -1373,3 +1377,22 @@ int guac_terminal_ctrl_func(guac_terminal* term, unsigned char c) {
}
+/**
+ * xterm does not implement APC functions and neither do we.
+ */
+int guac_terminal_apc(guac_terminal* term, unsigned char c) {
+
+ /* Look for the "ESC \" (string terminator) sequence,
--- End diff --
Is `ESC \` the only sequence that is valid as the string terminator for APC? Other sequences have involved other characters for the string terminator:
https://github.com/apache/guacamole-server/blob/b61a6ab758177b4d4cec4a3a0a2f51a3e76c3772/src/terminal/terminal_handlers.c#L1335-L1337
---
[GitHub] guacamole-server pull request #166: GUACAMOLE-564: Hide APC escape sequence.
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/guacamole-server/pull/166
---
[GitHub] guacamole-server pull request #166: GUACAMOLE-564: Hide APC escape sequence.
Posted by mike-jumper <gi...@git.apache.org>.
Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/guacamole-server/pull/166#discussion_r189180014
--- Diff: src/terminal/terminal_handlers.c ---
@@ -1373,3 +1377,22 @@ int guac_terminal_ctrl_func(guac_terminal* term, unsigned char c) {
}
+/**
--- End diff --
`guac_terminal_apc()` is correctly documented at the header level. There shouldn't be a duplicate Doxygen comment attached to the function body which provides additional information. The comment should either be moved within the function, or the comment should be deleted and the information instead added to the existing header-level comment.
---
[GitHub] guacamole-server pull request #166: GUACAMOLE-564: Hide APC escape sequence.
Posted by darchons <gi...@git.apache.org>.
Github user darchons commented on a diff in the pull request:
https://github.com/apache/guacamole-server/pull/166#discussion_r191063521
--- Diff: src/terminal/terminal_handlers.c ---
@@ -1373,3 +1377,22 @@ int guac_terminal_ctrl_func(guac_terminal* term, unsigned char c) {
}
+/**
+ * xterm does not implement APC functions and neither do we.
+ */
+int guac_terminal_apc(guac_terminal* term, unsigned char c) {
+
+ /* Look for the "ESC \" (string terminator) sequence,
--- End diff --
AFAICT it's always `ESC \`
---
[GitHub] guacamole-server pull request #166: GUACAMOLE-564: Hide APC escape sequence.
Posted by mike-jumper <gi...@git.apache.org>.
Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/guacamole-server/pull/166#discussion_r189180598
--- Diff: src/terminal/terminal_handlers.c ---
@@ -1373,3 +1377,22 @@ int guac_terminal_ctrl_func(guac_terminal* term, unsigned char c) {
}
+/**
+ * xterm does not implement APC functions and neither do we.
+ */
+int guac_terminal_apc(guac_terminal* term, unsigned char c) {
+
+ /* Look for the "ESC \" (string terminator) sequence,
+ * while ignoring other chars. */
+ static char escaping = 0;
--- End diff --
If representing a boolean value, please use the semantically-correct `bool`.
We have to live with using `int` within public APIs like libguac to avoid issues with third-party code that redefines `bool` in a non-standard way, as well as within anything dealing with FreeRDP (an example of such third-party code), but the internal terminal emulator is definitely safe to use `bool`.
---