mike-jumper commented on code in PR #395:
URL: https://github.com/apache/guacamole-server/pull/395#discussion_r982812012
##########
src/protocols/ssh/clipboard.c:
##########
@@ -47,14 +48,30 @@ int guac_ssh_clipboard_blob_handler(guac_user* user,
guac_stream* stream,
/* Append new data */
guac_client* client = user->client;
guac_ssh_client* ssh_client = (guac_ssh_client*) client->data;
- guac_terminal_clipboard_append(ssh_client->term, data, length);
+ guac_common_clipboard_append(ssh_client->clipboard, (char*) data, length);
return 0;
}
int guac_ssh_clipboard_end_handler(guac_user* user, guac_stream* stream) {
- /* Nothing to do - clipboard is implemented within client */
+ guac_client* client = user->client;
+ guac_ssh_client* ssh_client = (guac_ssh_client*) client->data;
+
+ char output_data[GUAC_COMMON_CLIPBOARD_MAX_LENGTH];
+
+ const char* input = ssh_client->clipboard->buffer;
+ char* output = output_data;
+ guac_iconv_write* writer = ssh_client->settings->clipboard_crlf ?
GUAC_WRITE_CP1252_CRLF : GUAC_WRITE_CP1252;
Review Comment:
This will result in the clipboard contents being transformed into [Windows
CP-1252](https://en.wikipedia.org/wiki/Windows-1252).
##########
src/protocols/ssh/client.c:
##########
@@ -44,6 +44,9 @@ int guac_client_init(guac_client* client) {
guac_ssh_client* ssh_client = calloc(1, sizeof(guac_ssh_client));
client->data = ssh_client;
+ /* Init clipboard */
+ ssh_client->clipboard = guac_common_clipboard_alloc();
Review Comment:
I don't think we should avoid using the clipboard provided by the terminal:
* The underlying issue here likely affects all terminal-related protocols,
not just SSH.
* Switching to a dedicated terminal here would mean that options like
`disable-copy` and `disable-paste` would need to be manually reimplemented.
Instead, this should be done at the terminal level. If there needs to be an
option for this, then a terminal option can be provided, though I suspect we
can safely normalize to UNIX-style newlines for all terminal protocols because
of how the terminal clipboard functions.
##########
src/protocols/ssh/settings.h:
##########
@@ -178,6 +178,19 @@ typedef struct guac_ssh_settings {
*/
bool sftp_disable_upload;
+ /**
+ * Whether line endings within the clipboard should be automatically
+ * normalized to Unix-style newline characters.
+ */
+ int normalize_clipboard;
+
+ /**
+ * Whether Unix-style newline characters within the clipboard should be
+ * automatically translated to CRLF sequences before transmission to the
+ * SSH server.
+ */
+ int clipboard_crlf;
Review Comment:
Is there any case where the terminal clipboard contents shouldn't be
normalized to UNIX-style newlines? When data is pasted within a terminal
emulator session, it's passed through to the remote end as if entered by the
user. In all cases, if a user presses the "Enter" or "Return" key, only a
single character will be sent:
https://github.com/apache/guacamole-server/blob/b20afa275a78ca4a54f4e088413e99c93901a3bc/src/terminal/terminal.c#L1675
If we selectively normalize or provide an option to transform these newlines
into CRLF pairs, we'd be adding an additional character and producing a
sequence that isn't normally produced by user input.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
