bug#36879: 26.2; OSC 52 paste in term/xterm.el not working

bug#36879: 26.2; OSC 52 paste in term/xterm.el not working

[Daniel Eklöf]

I'm trying to use the OSC 52 paste feature of term/xterm.el. There's a
comment in term/xterm.el that says it has to be explicitly enabled.

I did so by adding 'getSelection' to 'xterm-extra-capabilities'.

I have tried this in xterm-347, and in my own terminal emulator. Now,
when pasting, I see the OSC 52 escape sequence being sent to the
terminal, and Emacs seems to get the reply, sort of.

In xterm, nothing appears to happen, except that the modeline quickly
flashes "Quit". Please note that xterm *is* sending the reply (verified
by manually sending the OSC 52 request). There's also no timeout in
Emacs so I'm confident it did read the reply.

In my own terminal emulator, Emacs seems to be stuck in a keyboard input
sequence; the modeline shows

  "C-y <base64 encoded clipboard data> ESC \-".

The most likely reason for the difference in observed behavior is that
xterm terminates the OSC 52 reply with BEL (it echoes the terminator
from the OSC 52 request, which in Emacs' case is BEL), while my terminal
emulator terminates with an ST sequence, "\e\\".

Other than the terminator, the byte sequence sent from my terminal
emulator is exactly the same as sent by xterm.

Note: this is on the stable 26.2 release. But the responsible function,
term/xterm.el:gui-backend-get-selection, appears to be identical in
latest master.

Have I configured something wrong? Or is this a bug?

Regards,
Daniel

In GNU Emacs 26.2 (build 1, x86_64-pc-linux-gnu)
 of 2019-05-25 built on svetlemodry
System Description:	Arch Linux

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
Mark set [2 times]
Quit [2 times]
Making completion list... [2 times]

Configured using:
 'configure --prefix=/usr --sysconfdir=/etc --libexecdir=/usr/lib
 --localstatedir=/var --without-x --without-sound --with-modules
 'CFLAGS=-march=x86-64 -mtune=generic -O2 -pipe -fno-plt'
 CPPFLAGS=-D_FORTIFY_SOURCE=2
 LDFLAGS=-Wl,-O1,--sort-common,--as-needed,-z,relro,-z,now'

Configured features:
DBUS NOTIFY ACL GNUTLS LIBXML2 ZLIB MODULES THREADS LIBSYSTEMD

Important settings:
  value of $LC_CTYPE: en_US.UTF-8
  value of $LC_MESSAGES: en_US.UTF-8
  value of $LANG: sv_SE.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  show-paren-mode: t
  display-line-numbers-mode: t
  which-function-mode: t
  global-whitespace-mode: t
  global-semanticdb-minor-mode: t
  global-semantic-idle-scheduler-mode: t
  global-semantic-idle-local-symbol-highlight-mode: t
  global-semantic-decoration-mode: t
  global-semantic-highlight-func-mode: t
  semantic-mode: t
  global-company-mode: t
  company-mode: t
  global-flycheck-mode: t
  flycheck-mode: t
  winner-mode: t
  cl-old-struct-compat-mode: t
  xterm-mouse-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
/usr/share/emacs/site-lisp/flim/md4 hides /usr/share/emacs/26.2/lisp/md4
/usr/share/emacs/site-lisp/flim/hex-util hides /usr/share/emacs/26.2/lisp/hex-util
/usr/share/emacs/site-lisp/flim/ntlm hides /usr/share/emacs/26.2/lisp/net/ntlm
/usr/share/emacs/site-lisp/flim/hmac-def hides /usr/share/emacs/26.2/lisp/net/hmac-def
/usr/share/emacs/site-lisp/flim/sasl-digest hides /usr/share/emacs/26.2/lisp/net/sasl-digest
/usr/share/emacs/site-lisp/flim/sasl hides /usr/share/emacs/26.2/lisp/net/sasl
/usr/share/emacs/site-lisp/flim/hmac-md5 hides /usr/share/emacs/26.2/lisp/net/hmac-md5
/usr/share/emacs/site-lisp/flim/sasl-ntlm hides /usr/share/emacs/26.2/lisp/net/sasl-ntlm
/usr/share/emacs/site-lisp/flim/sasl-cram hides /usr/share/emacs/26.2/lisp/net/sasl-cram

Features:
(shadow sort flyspell ispell mail-extr emacsbug add-log term/xterm xterm
paren display-line-numbers which-func imenu elec-pair company-oddmuse
company-keywords company-etags etags xref project company-gtags
company-dabbrev-code company-dabbrev company-files company-capf
company-cmake company-xcode company-clang company-semantic company-eclim
company-template company-bbdb .emacs systemd url-parse url-vars
conf-mode mu4e-alert time ht s alert notifications dbus xml mu4e desktop
frameset mu4e-speedbar speedbar sb-image dframe mu4e-main mu4e-view
cal-menu calendar cal-loaddefs thingatpt browse-url comint ansi-color
gnus-art mm-uu mml2015 mm-view mml-smime smime dig mailcap gnus-sum
gnus-group gnus-undo gnus-start gnus-cloud nnimap nnmail mail-source tls
gnutls utf7 netrc nnoo parse-time gnus-spec gnus-int gnus-range gnus-win
gnus nnheader wid-edit mu4e-headers mu4e-compose mu4e-context mu4e-draft
mu4e-actions ido rfc2368 smtpmail auth-source sendmail mu4e-mark
mu4e-message flow-fill mu4e-proc mu4e-utils doc-view jka-compr
image-mode mu4e-lists mu4e-vars message rmc puny dired dired-loaddefs
format-spec rfc822 mml mml-sec password-cache gnus-util rmail tool-bar
rmail-loaddefs time-date mm-decode mm-bodies mm-encode mail-parse
rfc2231 rfc2047 rfc2045 mm-util ietf-drums mail-prsvr mailabbrev
mail-utils gmm-utils mailheader hl-line fringe mu4e-meta whitespace
midnight semantic/bovine/gcc semantic/dep semantic/db-mode semantic/db
eieio-base semantic/idle semantic/format ezimage image semantic/ctxt
semantic/decorate/mode semantic/tag-ls semantic/find semantic/decorate
pulse semantic/util-modes semantic/util semantic semantic/tag
semantic/lex semantic/fw eieio eieio-core cl-macs eieio-loaddefs
mode-local cedet company pcase flycheck regexp-opt cl-extra json map
find-func help-mode rx easymenu subr-x seq byte-opt gv bytecomp
byte-compile cconv dash edmacro kmacro windmove winner ring cl-seq
smart-mode-line-dark-theme smart-mode-line advice rich-minority
cl-loaddefs cl-lib zenburn-theme epa derived epg epg-config xt-mouse
disp-table mule-util tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type tabulated-list replace newcomment text-mode elisp-mode
lisp-mode prog-mode register page menu-bar rfn-eshadow isearch timer
select mouse jit-lock font-lock syntax facemenu font-core
term/tty-colors frame cl-generic cham georgian utf-8-lang misc-lang
vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms cp51932
hebrew greek romanian slovak czech european ethiopic indian cyrillic
chinese composite charscript charprop case-table epa-hook jka-cmpr-hook
help simple abbrev obarray minibuffer cl-preloaded nadvice loaddefs
button faces cus-face macroexp files text-properties overlay sha1 md5
base64 format env code-pages mule custom widget hashtable-print-readable
backquote threads dbusbind inotify multi-tty make-network-process emacs)

Memory information:
((conses 16 260142 13833)
 (symbols 48 36419 1)
 (miscs 40 82 206)
 (strings 32 77860 5282)
 (string-bytes 1 2421895)
 (vectors 16 34057)
 (vector-slots 8 684478 5244)
 (floats 8 283 667)
 (intervals 56 386 104)
 (buffers 992 13))

-- 
Daniel

bug#36879:

[Daniel Eklöf]

s/modeline/minibuffer

bug#36879: 26.2; OSC 52 paste in term/xterm.el not working

[Mattias Engdegård]

[-- Attachment #1: Type: text/plain, Size: 2228 bytes --]

tags 36879 patch
quit

Daniel Eklöf skrev:

>Have I configured something wrong? Or is this a bug?

I don't think you did anything wrong; I can reproduce the bug (in
XTerm; I don't have your fancy emulator).

The question is rather, how did this code ever work in the first
place? As you observed, when XTerm sends the reply, it uses BEL as
terminator. Emacs uses BEL (C-g) as INTR char, which means that not
only is special effort required to avoid having it quit the current
elisp code -- this could have been done using inhibit-quit -- but when
the pty receives the BEL from XTerm, it immediately discards unread
characters and raises SIGINT.

Thus, it's very much a race: the only way it could ever work would be
if Emacs has been able to read the entire reply except the BEL, and be
sitting inside (read-char) when the BEL reaches the pty. Needless to
say, this is rather unlikely.

We could tell the tty not to discard the queue upon INTR by setting the
NOFLSH flag, but (1) I don't know how portable that is, (2) it's not
what we normally want when C-g is used interactively, and (3) it would
still process the BEL out-of-order with respect to earlier chars.

Attached is a rather heavy-handed patch which temporarily changes the
quit-char to something unlikely while sending the OSC 52 request and
reading the reply. It also allows the reply to be terminated by ESC \
(ST) as well.

Since XTerm parrots our choice of string terminator (BEL or ST), this
suggests a simpler solution: just use ST, and the trouble with BEL is
no more. Unfortunately the code has provisions for screen/tmux, where
the entire request is wrapped in a DCS request:

 ESC P ... ESC \

which means that we cannot use ST as terminator in that case. However,
I haven't been able to make this facility work with tmux at all, and
with screen only by reverting 4183482f4d (bug#20356) AND explicitly
setting TERM=screen (the default is screen.xterm-256color).

In addition, changing quit-char can be visually annoying; it causes
reinitialisation of the entire tty, something you don't want every time
you press C-y. Perhaps it's fine to drop screen support from this
particular function? I attached another, alternative patch that does
that instead.



[-- Attachment #2: heavy.patch --]
[-- Type: text/x-patch, Size: 3218 bytes --]

From df344bde2bbe459eb5aea668388d2606a38fe12c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mattias=20Engdeg=C3=A5rd?= <mattiase@acm.org>
Date: Sat, 3 Aug 2019 12:08:27 +0200
Subject: [PATCH] Fix XTerm OSC 52 selection retrieval (bug#36879)

When asking XTerm for the selection via OSC 52, set the quit char to
something other than BEL since that char is used as string terminator
in the reply.  Otherwise, the pty will raise SIGINT as soon as it sees
the BEL, discarding any unread chars.

Also allow the reply to be terminated by ST as well as BEL.

* lisp/term/xterm.el (gui-backend-get-selection):
Change quit char temporarily.  Detect ST as string terminator in reply.
Use time-out when reading reply.
---
 lisp/term/xterm.el | 37 +++++++++++++++++++++++++++++--------
 1 file changed, 29 insertions(+), 8 deletions(-)

diff --git a/lisp/term/xterm.el b/lisp/term/xterm.el
index c4b0a8fb6e..29460ea55b 100644
--- a/lisp/term/xterm.el
+++ b/lisp/term/xterm.el
@@ -954,14 +954,35 @@ gui-backend-get-selection
          (query (concat "\e]52;" (xterm--selection-char type) ";")))
     (with-temp-buffer
       (set-buffer-multibyte nil)
-      (xterm--query
-       (concat (when screen "\eP") query "?\a" (when screen "\e\\"))
-       (list (cons query (lambda ()
-                           (while (let ((char (read-char)))
-                                    (unless (eq char ?\a)
-                                      (insert char)
-                                      t))))))
-       'no-async)
+      (let ((old-quit-char (nth 3 (current-input-mode))))
+        ;; Temporarily set the quit char to something else, because
+        ;; C-g may be sent from the terminal as a string terminator,
+        ;; and the resulting SIGINT (which occurs when the interrupt
+        ;; char is received by the tty, not when we read it) would
+        ;; flush unread chars from the tty input queue.
+        (unwind-protect
+            (progn
+              (set-quit-char ?\377)
+              (xterm--query
+               (concat (when screen "\eP") query "?\a" (when screen "\e\\"))
+               (list (cons query
+                           (lambda ()
+                             ;; Read data up to the string terminator,
+                             ;; either ST (ESC \) or BEL.
+                             (let (char last)
+                               (while (and (setq char (read-char
+                                                       nil nil
+                                                       xterm-query-timeout))
+                                           (not (or (eq char ?\a)
+                                                    (and (eq char ?\\)
+                                                         (eq last ?\e)))))
+                                 (when last
+                                   (insert last))
+                                 (setq last char))
+                               (when (eq char ?\a)
+                                 (insert last))))))
+               'no-async))
+        (set-quit-char old-quit-char)))
       (base64-decode-region (point-min) (point-max))
       (decode-coding-region (point-min) (point-max) 'utf-8-unix t))))
 
-- 
2.21.0


[-- Attachment #3: light.patch --]
[-- Type: text/x-patch, Size: 2917 bytes --]

From da122da5273a57b25edfe3e8885ea2250b88bf5d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mattias=20Engdeg=C3=A5rd?= <mattiase@acm.org>
Date: Sat, 3 Aug 2019 12:08:27 +0200
Subject: [PATCH] Fix XTerm OSC 52 selection retrieval (bug#36879)

When asking XTerm for the selection via OSC 52, use ST as string
terminator in the request to get ST as terminator in the reply,
because BEL is messy to receive in many ways.

* lisp/term/xterm.el (gui-backend-get-selection):
Use ST as string terminator in request and reply.
Use a time-out when reading the reply.
---
 lisp/term/xterm.el | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/lisp/term/xterm.el b/lisp/term/xterm.el
index c4b0a8fb6e..4b56b2ce4a 100644
--- a/lisp/term/xterm.el
+++ b/lisp/term/xterm.el
@@ -946,21 +946,31 @@ gui-backend-get-selection
     (type data-type
      &context (window-system nil)
               ;; Only applies to terminals which have it enabled.
-              ((terminal-parameter nil 'xterm--get-selection) (eql t)))
+              ((terminal-parameter nil 'xterm--get-selection) (eql t))
+              ;; Doesn't work in screen; see bug#36879.
+              ((eq (terminal-parameter nil 'terminal-initted)
+                   'terminal-init-screen)
+               (eql nil)))
   (unless (eq data-type 'STRING)
     (error "Unsupported data type %S" data-type))
-  (let* ((screen (eq (terminal-parameter nil 'terminal-initted)
-                     'terminal-init-screen))
-         (query (concat "\e]52;" (xterm--selection-char type) ";")))
+  (let ((query (concat "\e]52;" (xterm--selection-char type) ";")))
     (with-temp-buffer
       (set-buffer-multibyte nil)
       (xterm--query
-       (concat (when screen "\eP") query "?\a" (when screen "\e\\"))
-       (list (cons query (lambda ()
-                           (while (let ((char (read-char)))
-                                    (unless (eq char ?\a)
-                                      (insert char)
-                                      t))))))
+       ;; Use ST as query terminator to get ST as reply terminator (bug#36879).
+       (concat query "?\e\\")
+       (list (cons query
+                   (lambda ()
+                     ;; Read data up to the string terminator, ST.
+                     (let (char last)
+                       (while (and (setq char (read-char
+                                               nil nil
+                                               xterm-query-timeout))
+                                   (not (and (eq char ?\\)
+                                             (eq last ?\e))))
+                         (when last
+                           (insert last))
+                         (setq last char))))))
        'no-async)
       (base64-decode-region (point-min) (point-max))
       (decode-coding-region (point-min) (point-max) 'utf-8-unix t))))
-- 
2.21.0

bug#36879: 26.2; OSC 52 paste in term/xterm.el not working

[Eli Zaretskii]

> From: Mattias Engdegård <mattiase@acm.org>
> Date: Sat, 03 Aug 2019 13:41:03 +0200
> Cc: Philipp Stephani <phst@google.com>,
>  Stefan Monnier <monnier@iro.umontreal.ca>, 36879@debbugs.gnu.org
> 
> The question is rather, how did this code ever work in the first
> place? As you observed, when XTerm sends the reply, it uses BEL as
> terminator. Emacs uses BEL (C-g) as INTR char, which means that not
> only is special effort required to avoid having it quit the current
> elisp code -- this could have been done using inhibit-quit -- but when
> the pty receives the BEL from XTerm, it immediately discards unread
> characters and raises SIGINT.

You are saying that TTY frames cannot handle inhibit-quit correctly?

bug#36879: 26.2; OSC 52 paste in term/xterm.el not working

[Mattias Engdegård]

3 aug. 2019 kl. 13.52 skrev Eli Zaretskii <eliz@gnu.org>:
> 
> You are saying that TTY frames cannot handle inhibit-quit correctly?

Right; it works if the input is from a slowly typing user, but a burst that includes BEL is likely to be read incorrectly.

A more modern approach would be to not have C-g as interrupting char in the TTY at all, but that's a bit more work.

bug#36879: 26.2; OSC 52 paste in term/xterm.el not working

[Eli Zaretskii]

> From: Mattias Engdegård <mattiase@acm.org>
> Date: Sat, 3 Aug 2019 14:02:02 +0200
> Cc: daniel@ekloef.se, phst@google.com, monnier@iro.umontreal.ca,
>         36879@debbugs.gnu.org
> 
> 3 aug. 2019 kl. 13.52 skrev Eli Zaretskii <eliz@gnu.org>:
> > 
> > You are saying that TTY frames cannot handle inhibit-quit correctly?
> 
> Right; it works if the input is from a slowly typing user, but a burst that includes BEL is likely to be read incorrectly.

Where is the code that discards input if it arrives quickly and
includes C-g?

bug#36879: 26.2; OSC 52 paste in term/xterm.el not working

[Mattias Engdegård]

3 aug. 2019 kl. 14.08 skrev Eli Zaretskii <eliz@gnu.org>:
> 
> Where is the code that discards input if it arrives quickly and
> includes C-g?

In the kernel. When it receives the INTR char, it flushes any unread chars, unless NOFLSH is set.
You can try it out in your shell:

$ sleep 10
abc^C
$          ("abc" was discarded)
$ stty noflsh
$ sleep 10
xyz^C
$ xyz      ("xyz" was read by the shell)

However, NOFLSH doesn't help us in Emacs.

bug#36879: 26.2; OSC 52 paste in term/xterm.el not working

[Eli Zaretskii]

> From: Mattias Engdegård <mattiase@acm.org>
> Date: Sat, 3 Aug 2019 14:26:29 +0200
> Cc: daniel@ekloef.se, phst@google.com, monnier@iro.umontreal.ca,
>         36879@debbugs.gnu.org
> 
> However, NOFLSH doesn't help us in Emacs.

Why not?

bug#36879: 26.2; OSC 52 paste in term/xterm.el not working

[Mattias Engdegård]

3 aug. 2019 kl. 15.36 skrev Eli Zaretskii <eliz@gnu.org>:
> 
>> However, NOFLSH doesn't help us in Emacs.
> 
> Why not?

Because we still wouldn't know where in the input stream the BEL should be placed. By the time we have read the chars preceding it, more could have arrived. Besides, flushing is usually what we want (mash the keyboard, nothing happens, press C-g).

If we want to receive a character reliably and in order, we cannot set it up as an interrupt char in the TTY.

bug#36879: 26.2; OSC 52 paste in term/xterm.el not working

[Eli Zaretskii]

> From: Mattias Engdegård <mattiase@acm.org>
> Date: Sat, 3 Aug 2019 16:32:09 +0200
> Cc: daniel@ekloef.se, phst@google.com, monnier@iro.umontreal.ca,
>         36879@debbugs.gnu.org
> 
> 3 aug. 2019 kl. 15.36 skrev Eli Zaretskii <eliz@gnu.org>:
> > 
> >> However, NOFLSH doesn't help us in Emacs.
> > 
> > Why not?
> 
> Because we still wouldn't know where in the input stream the BEL should be placed. By the time we have read the chars preceding it, more could have arrived. Besides, flushing is usually what we want (mash the keyboard, nothing happens, press C-g).

Then what is NOFLSH good for?

bug#36879: 26.2; OSC 52 paste in term/xterm.el not working

[Mattias Engdegård]

3 aug. 2019 kl. 18.59 skrev Eli Zaretskii <eliz@gnu.org>:

> Then what is NOFLSH good for?

Your guess is as good as mine. After all, this is the Unix TTY layer we are talking about.

bug#36879: 26.2; OSC 52 paste in term/xterm.el not working

[Daniel Eklöf]

> Emacs uses BEL (C-g) as INTR char, which means that not
> only is special effort required to avoid having it quit the current
> elisp code -- this could have been done using inhibit-quit -- but when
> the pty receives the BEL from XTerm, it immediately discards unread
> characters and raises SIGINT.

I did figure Emacs, at the very least, handled BEL differently, but I
wasn't aware the PTY would discard unread characters. Thanks for
clarifying.

> Thus, it's very much a race: the only way it could ever work would be
> if Emacs has been able to read the entire reply except the BEL, and be
> sitting inside (read-char) when the BEL reaches the pty. Needless to
> say, this is rather unlikely.

Never happened during my tests :)

> Since XTerm parrots our choice of string terminator (BEL or ST), this
> suggests a simpler solution: just use ST, and the trouble with BEL is
> no more. Unfortunately the code has provisions for screen/tmux, where
> the entire request is wrapped in a DCS request:
>
>  ESC P ... ESC \
>
> which means that we cannot use ST as terminator in that case.

I came to the same conclusion, and wrote a proof-of-concept patch that
replaced BEL with ST, and verified that yes, that does indeed work. In
XTerm as well as in my not-fancy terminal. (I didn't bother removing
support for screen though, making it both a broken patch, and cruder
than yours :) )

(Förresten, hej Mattias! Det var lite oväntat...)

--
Daniel

bug#36879: 26.2; OSC 52 paste in term/xterm.el not working

[Daniel Eklöf]

> Perhaps it's fine to drop screen support from this
> particular function? I attached another, alternative patch that 
> does
> that instead.

My use case is bare metal terminal, no screen. I.e. I'm fine with 
this. (And yes, I realize this question wasn't primarily for me.)

--
Daniel

bug#36879: 26.2; OSC 52 paste in term/xterm.el not working

[Stefan Monnier]

> The question is rather, how did this code ever work in the first
> place?

Here's what I remember from it:
- I did test the code with the xterm that comes with Debian.
  I definitely remember making it work it in one direction (paste or
  yank, can't remember), not sure if I did get it to work in both
  directions at the time.
- The feature is fundamentally dodgy from a security perspective.
- This is the first case I hear of someone actually using that feature
  (or trying to) since it was added to Emacs.

So I'm not sure it's worth the trouble supporting this, really:
In many/most cases `xclip-mode` can be used instead, and is much
more straightforward.


        Stefan

bug#36879: 26.2; OSC 52 paste in term/xterm.el not working

[Daniel Eklöf]

> - I did test the code with the xterm that comes with Debian.
>   I definitely remember making it work it in one direction 
>   (paste or
>   yank, can't remember), not sure if I did get it to work in 
>   both
>   directions at the time.

set-selection has always worked, at least for me. That one is also 
enabled by default in xterm.el (when an xterm supporting it is 
detected, I assume).

> - The feature is fundamentally dodgy from a security 
> perspective.

I'm probably missing something obvious, but how is talking to 
xclip more secure than talking to the terminal emulator? Or is the 
"security perspective" somewhere else?

> So I'm not sure it's worth the trouble supporting this, really:
> In many/most cases `xclip-mode` can be used instead, and is much
> more straightforward.

Except that xclip assumes x11. Would it not make sense to support 
a window protocol agnostic method? By supporting OSC 52, you 
support whatever clipboard mechanism the terminal emulator 
supports.

Perhaps one could use the heavy weight solution (change quit char) 
when 'screen' is detected, but simply use ST in the non-screen 
case?

(before I tried this, OSC 52, I was using 
https://github.com/bugaevc/wl-clipboard with my own Emacs 
"bindings" - yes, I'm on Wayland)

bug#36879: 26.2; OSC 52 paste in term/xterm.el not working

[Mattias Engdegård]

4 aug. 2019 kl. 10.19 skrev Daniel Eklöf <daniel@ekloef.se>:
> 
> set-selection has always worked, at least for me. That one is also enabled by default in xterm.el (when an xterm supporting it is detected, I assume).

Right, it lacks the technical problems of the copy-from-clipboard direction, since no reply from the terminal is involved.

> I'm probably missing something obvious, but how is talking to xclip more secure than talking to the terminal emulator? Or is the "security perspective" somewhere else?

It's not a problem in Emacs, but by enabling OSC 52 in your terminal, an adversary might arrange for a crafted string to be sent to it which would surreptitiously inject malicious data into the clipboard, or extract secrets from it. The OSC 52 reply itself could cause damage under some circumstances, or the attacker could just hope for the victim to paste a command into a shell prompt.

> Except that xclip assumes x11. Would it not make sense to support a window protocol agnostic method? By supporting OSC 52, you support whatever clipboard mechanism the terminal emulator supports.

I can definitely see how OSC 52 can be useful when there is only a terminal connection to the machine running Emacs, and no out-of-band conduit for the clipboard. The user needs to enable it actively both in the terminal and in Emacs; it cannot be used by accident.

> Perhaps one could use the heavy weight solution (change quit char) when 'screen' is detected, but simply use ST in the non-screen case?

The thought did cross my mind, but I thought I'd first enquire about the screen usage, given that I only got it to work with screen, not tmux, and then only after explicitly setting TERM.

Perhaps Philipp Stephani who originally wrote the code could help us here (sorry about dragging you into the discussion, Philipp). Under what circumstances did you run it? (It was 4 years ago; it's understandable if you don't remember much of it.)

bug#36879: 26.2; OSC 52 paste in term/xterm.el not working

[Daniel Eklöf]

> It's not a problem in Emacs, but by enabling OSC 52 in your 
> terminal, an adversary might arrange for a crafted string to be 
> sent to it which would surreptitiously inject malicious data 
> into the clipboard, or extract secrets from it. The OSC 52 reply 
> itself could cause damage under some circumstances, or the 
> attacker could just hope for the victim to paste a command into 
> a shell prompt.

Right. I interpreted Stefan's statement as concerning Emacs, not 
OSC 52 itself.

bug#36879: 26.2; OSC 52 paste in term/xterm.el not working

[Stefan Monnier]

> Right. I interpreted Stefan's statement as concerning Emacs, not OSC
> 52 itself.

No, the security issues have to do with OSC-52 itself, IIRC.


        Stefan

bug#36879: 26.2; OSC 52 paste in term/xterm.el not working

[Philipp Stephani]

Am So., 4. Aug. 2019 um 11:45 Uhr schrieb Mattias Engdegård <mattiase@acm.org>:

> > I'm probably missing something obvious, but how is talking to xclip more secure than talking to the terminal emulator? Or is the "security perspective" somewhere else?
>
> It's not a problem in Emacs, but by enabling OSC 52 in your terminal, an adversary might arrange for a crafted string to be sent to it which would surreptitiously inject malicious data into the clipboard, or extract secrets from it. The OSC 52 reply itself could cause damage under some circumstances, or the attacker could just hope for the victim to paste a command into a shell prompt.
>
> > Except that xclip assumes x11. Would it not make sense to support a window protocol agnostic method? By supporting OSC 52, you support whatever clipboard mechanism the terminal emulator supports.
>
> I can definitely see how OSC 52 can be useful when there is only a terminal connection to the machine running Emacs, and no out-of-band conduit for the clipboard. The user needs to enable it actively both in the terminal and in Emacs; it cannot be used by accident.
>
> > Perhaps one could use the heavy weight solution (change quit char) when 'screen' is detected, but simply use ST in the non-screen case?
>
> The thought did cross my mind, but I thought I'd first enquire about the screen usage, given that I only got it to work with screen, not tmux, and then only after explicitly setting TERM.
>
> Perhaps Philipp Stephani who originally wrote the code could help us here (sorry about dragging you into the discussion, Philipp). Under what circumstances did you run it? (It was 4 years ago; it's understandable if you don't remember much of it.)
>


I added OSC-52 support primarily to support HTerm/Chrome Secure Shell.
HTerm supports copying via OSC-52, but not pasting due to the
aforementioned security issues, cf.
https://chromium.googlesource.com/apps/libapps/+/master/nassh/doc/FAQ.md#Is-OSC-52-aka-clipboard-operations_supported.
I don't use HTerm that much any more, but OSC-52 support for copying
was definitely quite useful. Copying is not a security issue (at least
for the SSH use case) as the clipboard is always ephemeral anyway.

bug#36879: 26.2; OSC 52 paste in term/xterm.el not working

[Mattias Engdegård]

15 aug. 2019 kl. 21.32 skrev Philipp Stephani <p.stephani2@gmail.com>:
> 
> I added OSC-52 support primarily to support HTerm/Chrome Secure Shell.
> HTerm supports copying via OSC-52, but not pasting due to the
> aforementioned security issues, cf.
> https://chromium.googlesource.com/apps/libapps/+/master/nassh/doc/FAQ.md#Is-OSC-52-aka-clipboard-operations_supported.
> I don't use HTerm that much any more, but OSC-52 support for copying
> was definitely quite useful. Copying is not a security issue (at least
> for the SSH use case) as the clipboard is always ephemeral anyway.

Thank you! It confirms that the paste code was never used much if at all.
The limitations of the chosen patch is then probably nothing to worry about.

bug#36879: 26.2; OSC 52 paste in term/xterm.el not working

[Stefan Monnier]

>> - The feature is fundamentally dodgy from a security perspective.
> I'm probably missing something obvious, but how is talking to xclip more
> secure than talking to the terminal emulator? Or is the "security
> perspective" somewhere else?

I remember various attack vectors.  They're not necessarily trivial
to exploit, but it's still problematic (which is likely why it's
disabled by default in Debian's build).

>> So I'm not sure it's worth the trouble supporting this, really:
>> In many/most cases `xclip-mode` can be used instead, and is much
>> more straightforward.
> Except that xclip assumes x11.

I'm talking about xclip-mode, not xclip.

> (before I tried this, OSC 52, I was using
> https://github.com/bugaevc/wl-clipboard with my own Emacs "bindings" - yes,
> I'm on Wayland)

xclip-mode also has code to support wl-clipboard (I never tested it, tho).

> Would it not make sense to support a window protocol agnostic method?
> By supporting OSC 52, you support whatever clipboard mechanism the
> terminal emulator supports.

It has its advantages, of course.  But also its downsides.
The overlap with xclip-mode is fairly large, tho.


        Stefan

bug#36879: 26.2; OSC 52 paste in term/xterm.el not working

[Daniel Eklöf]

> I'm talking about xclip-mode, not xclip.
> xclip-mode also has code to support wl-clipboard (I never tested 
> it, tho).

I know you where, and I see it does, now. It didn't when I looked 
at it before. Thanks for the update, and thanks for adding support 
for it.

Anyway, I think it's beyond clear _what_ the problem is. As to 
how, if at all, to fix it is not for me to say so I wont try. I'll 
be using some version of the patch(es) provided here and move on.

Everybody, thanks for the help.

bug#36879: 26.2; OSC 52 paste in term/xterm.el not working

[Mattias Engdegård]

4 aug. 2019 kl. 17.59 skrev Daniel Eklöf <daniel@ekloef.se>:
> 
> Anyway, I think it's beyond clear _what_ the problem is. As to how, if at all, to fix it is not for me to say so I wont try. I'll be using some version of the patch(es) provided here and move on.

Fine. How about we push the 'light' patch posted earlier, ditching screen support for get-selection since it was painful to use anyway? Unless anyone objects, of course.

bug#36879: 26.2; OSC 52 paste in term/xterm.el not working

[Daniel Eklöf]

> Fine. How about we push the 'light' patch posted earlier, 
> ditching screen support for get-selection since it was painful 
> to use anyway? Unless anyone objects, of course.

I'm obviously fine with that.

bug#36879: 26.2; OSC 52 paste in term/xterm.el not working

[Mattias Engdegård]

5 aug. 2019 kl. 18.57 skrev Daniel Eklöf <daniel@ekloef.se>:
> 
> I'm obviously fine with that.

All right, the 'light' patch has now been pushed to master.