From 2ce1846488544fe9afe6ce24c502185fab6ddff0 Mon Sep 17 00:00:00 2001 From: Dan Kessler Date: Mon, 31 Jan 2022 13:50:02 -0500 Subject: [PATCH 1/9] use octal rather than literals for control characters in R string This avoids a bug when the file is "injected" where the control characters are potentially intercepted and consumed by the shell on their way to R. This should fix the original bug reported in #1163, although it doesn't fully address the issues with MPI support for sessions initiated by `ess-remote` (which require some additional work as outlined [here](https://github.com/emacs-ess/ESS/issues/1163#issuecomment-1024967733) --- etc/ESSR/R/mpi.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/etc/ESSR/R/mpi.R b/etc/ESSR/R/mpi.R index 53d7e470f..e5e769db1 100644 --- a/etc/ESSR/R/mpi.R +++ b/etc/ESSR/R/mpi.R @@ -7,7 +7,7 @@ else as.character(el) }) payload <- paste(dots, collapse = "") - cat(sprintf("_%s%s\\", head, payload)) + cat(sprintf("\033_%s036%s\033\\", head, payload)) } .ess_mpi_message <- function(msg){ From d9443009347caaa779420986191dd1a830db4893 Mon Sep 17 00:00:00 2001 From: Dan Kessler Date: Mon, 31 Jan 2022 14:20:17 -0500 Subject: [PATCH 2/9] use different control codes as MPI start/end delimiters Literal ESC is tricky because it can be consumed by the shell or otherwise misinterpreted. Use FS to mark the beginning, GS to mark the end, and RS (which was previously used) to mark the end of the header. Hopefully the separator control codes are unlikely to cause conflicts. Also simplify the delimiters to use *solely* the control code and not rely on other adjacent ASCII characters. Make corresponding updates to elisp variables ess-mpi-message-{start,end}-delimiter and update the docstring for `ess-mpi-handle-messages` (which curiously already used GS as the end separator). --- etc/ESSR/R/mpi.R | 2 +- lisp/ess-tracebug.el | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/etc/ESSR/R/mpi.R b/etc/ESSR/R/mpi.R index e5e769db1..6f803549e 100644 --- a/etc/ESSR/R/mpi.R +++ b/etc/ESSR/R/mpi.R @@ -7,7 +7,7 @@ else as.character(el) }) payload <- paste(dots, collapse = "") - cat(sprintf("\033_%s036%s\033\\", head, payload)) + cat(sprintf("\034%s\036%s\035", head, payload)) } .ess_mpi_message <- function(msg){ diff --git a/lisp/ess-tracebug.el b/lisp/ess-tracebug.el index 234697e1a..694e6d6a2 100644 --- a/lisp/ess-tracebug.el +++ b/lisp/ess-tracebug.el @@ -1183,9 +1183,9 @@ Kill the *ess.dbg.[R_name]* buffer." ;; http://jkorpela.fi/chars/c0.html ;; https://en.wikipedia.org/wiki/ANSI_escape_code#Escape_sequences -(defvar ess-mpi-message-start-delimiter "_") +(defvar ess-mpi-message-start-delimiter "") (defvar ess-mpi-message-field-separator "") -(defvar ess-mpi-message-end-delimiter "\\") +(defvar ess-mpi-message-end-delimiter "") (define-obsolete-variable-alias 'ess-mpi-alist 'ess-mpi-handlers "ESS 19.04") (defvar ess-mpi-handlers @@ -1229,7 +1229,7 @@ value from EXPR and then sent to the subprocess." (defun ess-mpi-handle-messages (buf) "Handle all mpi messages in BUF and delete them. -The MPI message has the form TYPEFIELD... where TYPE is the +The MPI message has the form TYPEFIELD... where TYPE is the type of the messages on which handlers in `ess-mpi-handlers' are dispatched. And FIELDs are strings. Return :incomplete if BUF ends with an incomplete message." From a5451413b939aebd1e629ca189510b7b27e9d2d8 Mon Sep 17 00:00:00 2001 From: Dan Kessler Date: Mon, 31 Jan 2022 14:35:38 -0500 Subject: [PATCH 3/9] override read-only when handling MPI messages in `*shell*` buffers, the MPI message we are trying to extract may likely have the special `read-only` text property, which prevents us from deleting it after we handle it. Overcome this by wrapping the deletion command in a let form that binds `inhibit-read-only` to `t`, as suggested by the elisp manual's "Special Properties" page --- lisp/ess-tracebug.el | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lisp/ess-tracebug.el b/lisp/ess-tracebug.el index 694e6d6a2..887308705 100644 --- a/lisp/ess-tracebug.el +++ b/lisp/ess-tracebug.el @@ -1257,7 +1257,8 @@ ends with an incomplete message." (apply handler payload)) (error "No handler defined for MPI message '%s" head)) (goto-char mbeg0) - (delete-region mbeg0 mend0))) + (let ((inhibit-read-only t)) + (delete-region mbeg0 mend0)))) (setq out :incomplete)))) out))) From f5f54f27238d0058b5346cbe2b76751030ee45a1 Mon Sep 17 00:00:00 2001 From: Dan Kessler Date: Mon, 31 Jan 2022 15:00:22 -0500 Subject: [PATCH 4/9] setup ess-tracebug when invoked via ess-remote This is necessary to configure the MPI handler to run regularly --- lisp/essd-els.el | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lisp/essd-els.el b/lisp/essd-els.el index c72805726..a5487c95f 100644 --- a/lisp/essd-els.el +++ b/lisp/essd-els.el @@ -154,7 +154,8 @@ DIALECT is the desired ess-dialect. If nil, ask for dialect" (ess-eval-linewise (format "options(pager='%s')\n" (or inferior-ess-remote-pager inferior-ess-pager)) nil nil nil 'wait) - (ess-r-load-ESSR)) + (ess-r-load-ESSR) + (when ess-use-tracebug (ess-tracebug 1))) (when (equal ess-dialect "S+") (ess-command ess-S+--injected-code)) From a9784f9f3b14aac9676f7cd9fcc4937bb62a707d Mon Sep 17 00:00:00 2001 From: Dan Kessler Date: Tue, 1 Feb 2022 09:28:42 -0500 Subject: [PATCH 5/9] Change control codes in MPI protocol As suggested in discussion of #1182, adopt the following protocol for MPI `SOH header STX payload ETX` also use octal codes in elisp for better readability --- etc/ESSR/R/mpi.R | 2 +- lisp/ess-tracebug.el | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/etc/ESSR/R/mpi.R b/etc/ESSR/R/mpi.R index 6f803549e..09a5ec825 100644 --- a/etc/ESSR/R/mpi.R +++ b/etc/ESSR/R/mpi.R @@ -7,7 +7,7 @@ else as.character(el) }) payload <- paste(dots, collapse = "") - cat(sprintf("\034%s\036%s\035", head, payload)) + cat(sprintf("\001%s\002%s\003", head, payload)) } .ess_mpi_message <- function(msg){ diff --git a/lisp/ess-tracebug.el b/lisp/ess-tracebug.el index 887308705..87a55b981 100644 --- a/lisp/ess-tracebug.el +++ b/lisp/ess-tracebug.el @@ -1183,9 +1183,9 @@ Kill the *ess.dbg.[R_name]* buffer." ;; http://jkorpela.fi/chars/c0.html ;; https://en.wikipedia.org/wiki/ANSI_escape_code#Escape_sequences -(defvar ess-mpi-message-start-delimiter "") -(defvar ess-mpi-message-field-separator "") -(defvar ess-mpi-message-end-delimiter "") +(defvar ess-mpi-message-start-delimiter "\001") +(defvar ess-mpi-message-field-separator "\002") +(defvar ess-mpi-message-end-delimiter "\003") (define-obsolete-variable-alias 'ess-mpi-alist 'ess-mpi-handlers "ESS 19.04") (defvar ess-mpi-handlers @@ -1229,7 +1229,7 @@ value from EXPR and then sent to the subprocess." (defun ess-mpi-handle-messages (buf) "Handle all mpi messages in BUF and delete them. -The MPI message has the form TYPEFIELD... where TYPE is the +The MPI message has the form TYPEFIELD... where TYPE is the type of the messages on which handlers in `ess-mpi-handlers' are dispatched. And FIELDs are strings. Return :incomplete if BUF ends with an incomplete message." From 3e84b0a04b027398a34373997879fd50befe3bf9 Mon Sep 17 00:00:00 2001 From: Dan Kessler Date: Wed, 2 Feb 2022 14:40:29 -0500 Subject: [PATCH 6/9] rename mpi-related vars and funs to use internal convention As requested in #1182, rename any variables/functions that I've touched to begin with `ess--mpi` in order to follow the convention for "internal" features --- lisp/ess-tracebug.el | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/lisp/ess-tracebug.el b/lisp/ess-tracebug.el index 87a55b981..a08b67391 100644 --- a/lisp/ess-tracebug.el +++ b/lisp/ess-tracebug.el @@ -1183,9 +1183,9 @@ Kill the *ess.dbg.[R_name]* buffer." ;; http://jkorpela.fi/chars/c0.html ;; https://en.wikipedia.org/wiki/ANSI_escape_code#Escape_sequences -(defvar ess-mpi-message-start-delimiter "\001") -(defvar ess-mpi-message-field-separator "\002") -(defvar ess-mpi-message-end-delimiter "\003") +(defvar ess--mpi-message-start-delimiter "\001") +(defvar ess--mpi-message-field-separator "\002") +(defvar ess--mpi-message-end-delimiter "\003") (define-obsolete-variable-alias 'ess-mpi-alist 'ess-mpi-handlers "ESS 19.04") (defvar ess-mpi-handlers @@ -1227,7 +1227,7 @@ value from EXPR and then sent to the subprocess." ((string= el "t") t) (t el))) -(defun ess-mpi-handle-messages (buf) +(defun ess--mpi-handle-messages (buf) "Handle all mpi messages in BUF and delete them. The MPI message has the form TYPEFIELD... where TYPE is the type of the messages on which handlers in `ess-mpi-handlers' are @@ -1240,15 +1240,15 @@ ends with an incomplete message." ;; This should be smarter because Emacs might cut it in the middle of the ;; message. In practice this almost never happen because we are ;; accumulating output into the cache buffer. - (while (search-forward ess-mpi-message-start-delimiter nil t) + (while (search-forward ess--mpi-message-start-delimiter nil t) (let ((mbeg0 (match-beginning 0)) (mbeg (match-end 0))) - (if (search-forward ess-mpi-message-end-delimiter nil t) + (if (search-forward ess--mpi-message-end-delimiter nil t) (let* ((mend (match-beginning 0)) (mend0 (match-end 0)) (msg (buffer-substring mbeg mend)) (payload (mapcar #'ess-mpi-convert - (split-string msg ess-mpi-message-field-separator))) + (split-string msg ess--mpi-message-field-separator))) (head (pop payload)) (handler (cdr (assoc head ess-mpi-handlers)))) (unwind-protect @@ -1325,7 +1325,7 @@ prompts." ;; Incomplete mpi should hardly happen. Only on those rare occasions ;; when an mpi is issued after a long task and split by the Emacs input ;; handler, or mpi printing itself takes very long. - (unless (eq :incomplete (ess-mpi-handle-messages abuf)) + (unless (eq :incomplete (ess--mpi-handle-messages abuf)) (with-current-buffer abuf ;; Uncomment this line when debugging. This pops up the ;; accumulation buffer and causes point to follow From 775e541ccda200686e6ef0a10472b1a0db4eb3d6 Mon Sep 17 00:00:00 2001 From: Dan Kessler Date: Wed, 2 Feb 2022 15:45:48 -0500 Subject: [PATCH 7/9] add some documentation regarding MPI protocol remove link to ESC sequence documentation (since we no longer use literal ESC for delimiters) and replace with link to control codes on wikipedia (slightly redundant with existing link but with more detail). Update docstring for ess--mpi-handle-messages to 1. Use octal's for literals for better readability on github 2. add a note calling attention to the fact that the message contains literal ASCII control codes which hopefully gives something search-able if a user is confused by the seemingly strange characters printed in the docstring --- lisp/ess-tracebug.el | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/lisp/ess-tracebug.el b/lisp/ess-tracebug.el index a08b67391..dbfa282ff 100644 --- a/lisp/ess-tracebug.el +++ b/lisp/ess-tracebug.el @@ -1182,10 +1182,10 @@ Kill the *ess.dbg.[R_name]* buffer." ;;; MPI ;; http://jkorpela.fi/chars/c0.html -;; https://en.wikipedia.org/wiki/ANSI_escape_code#Escape_sequences -(defvar ess--mpi-message-start-delimiter "\001") -(defvar ess--mpi-message-field-separator "\002") -(defvar ess--mpi-message-end-delimiter "\003") +;; https://en.wikipedia.org/wiki/C0_and_C1_control_codes +(defvar ess--mpi-message-start-delimiter "\001") ; SOH control code +(defvar ess--mpi-message-field-separator "\002") ; STX control code +(defvar ess--mpi-message-end-delimiter "\003") ; ETX control code (define-obsolete-variable-alias 'ess-mpi-alist 'ess-mpi-handlers "ESS 19.04") (defvar ess-mpi-handlers @@ -1229,10 +1229,12 @@ value from EXPR and then sent to the subprocess." (defun ess--mpi-handle-messages (buf) "Handle all mpi messages in BUF and delete them. -The MPI message has the form TYPEFIELD... where TYPE is the -type of the messages on which handlers in `ess-mpi-handlers' are -dispatched. And FIELDs are strings. Return :incomplete if BUF -ends with an incomplete message." +The MPI message has the form \001TYPE\002FIELD...\003 where TYPE +is the type of the messages on which handlers in +`ess-mpi-handlers' are dispatched. And FIELDs are strings. Note +that the MPI message contains literal ASCII control codes as +delimiters. Return :incomplete if BUF ends with an incomplete +message." (let ((obuf (current-buffer)) (out nil)) (with-current-buffer buf From 7f1e9846932b1e7ff7416cd8036892260edd4678 Mon Sep 17 00:00:00 2001 From: Dan Kessler Date: Thu, 3 Feb 2022 11:24:56 -0500 Subject: [PATCH 8/9] revert function name to `ess-mpi-handle-messages` as requested by @vspinu, but for now, leave the mpi delimiter variables names double-dashed as explicitly requested by @lionel- --- lisp/ess-tracebug.el | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lisp/ess-tracebug.el b/lisp/ess-tracebug.el index dbfa282ff..b6aaccf90 100644 --- a/lisp/ess-tracebug.el +++ b/lisp/ess-tracebug.el @@ -1227,7 +1227,7 @@ value from EXPR and then sent to the subprocess." ((string= el "t") t) (t el))) -(defun ess--mpi-handle-messages (buf) +(defun ess-mpi-handle-messages (buf) "Handle all mpi messages in BUF and delete them. The MPI message has the form \001TYPE\002FIELD...\003 where TYPE is the type of the messages on which handlers in @@ -1327,7 +1327,7 @@ prompts." ;; Incomplete mpi should hardly happen. Only on those rare occasions ;; when an mpi is issued after a long task and split by the Emacs input ;; handler, or mpi printing itself takes very long. - (unless (eq :incomplete (ess--mpi-handle-messages abuf)) + (unless (eq :incomplete (ess-mpi-handle-messages abuf)) (with-current-buffer abuf ;; Uncomment this line when debugging. This pops up the ;; accumulation buffer and causes point to follow From 930897df007e26863ae17ef0ecada928cc701237 Mon Sep 17 00:00:00 2001 From: Dan Kessler Date: Thu, 29 Jun 2023 15:04:36 -0400 Subject: [PATCH 9/9] update protocol and documentation for mpi new protocol is GS RS head US payload RS GS See further [discussion on github](https://github.com/emacs-ess/ESS/pull/1182#issuecomment-1613633197) --- etc/ESSR/R/mpi.R | 3 ++- lisp/ess-tracebug.el | 10 +++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/etc/ESSR/R/mpi.R b/etc/ESSR/R/mpi.R index 09a5ec825..c29bfb48b 100644 --- a/etc/ESSR/R/mpi.R +++ b/etc/ESSR/R/mpi.R @@ -7,7 +7,8 @@ else as.character(el) }) payload <- paste(dots, collapse = "") - cat(sprintf("\001%s\002%s\003", head, payload)) + ## see docstring for ess-mpi-handle-messages + cat(sprintf("\035\036%s\037%s\036\035", head, payload)) } .ess_mpi_message <- function(msg){ diff --git a/lisp/ess-tracebug.el b/lisp/ess-tracebug.el index b6aaccf90..8bab28f94 100644 --- a/lisp/ess-tracebug.el +++ b/lisp/ess-tracebug.el @@ -1183,9 +1183,9 @@ Kill the *ess.dbg.[R_name]* buffer." ;; http://jkorpela.fi/chars/c0.html ;; https://en.wikipedia.org/wiki/C0_and_C1_control_codes -(defvar ess--mpi-message-start-delimiter "\001") ; SOH control code -(defvar ess--mpi-message-field-separator "\002") ; STX control code -(defvar ess--mpi-message-end-delimiter "\003") ; ETX control code +(defvar ess--mpi-message-start-delimiter "\035\036") ; GS RS +(defvar ess--mpi-message-field-separator "\037") ; US +(defvar ess--mpi-message-end-delimiter "\036\035") ; RS GS (define-obsolete-variable-alias 'ess-mpi-alist 'ess-mpi-handlers "ESS 19.04") (defvar ess-mpi-handlers @@ -1229,9 +1229,9 @@ value from EXPR and then sent to the subprocess." (defun ess-mpi-handle-messages (buf) "Handle all mpi messages in BUF and delete them. -The MPI message has the form \001TYPE\002FIELD...\003 where TYPE +The MPI message has the form \035\036TYPE\037FIELD\036\035 where TYPE is the type of the messages on which handlers in -`ess-mpi-handlers' are dispatched. And FIELDs are strings. Note +`ess-mpi-handlers' are dispatched, and FIELDs are strings. Note that the MPI message contains literal ASCII control codes as delimiters. Return :incomplete if BUF ends with an incomplete message."