unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: miha--- via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
To: Ioannis Kappas <ioannis.kappas@gmail.com>,
	Lars Ingebrigtsen <larsi@gnus.org>
Cc: 53808@debbugs.gnu.org
Subject: bug#53808: 29.0.50; ansi colorization process could block indefinetly on stray ESC char
Date: Sun, 06 Feb 2022 21:36:59 +0100	[thread overview]
Message-ID: <87ee4fwx3o.fsf@miha-pc> (raw)
In-Reply-To: <CAMRHuGB=UdG+zeRiKoL3DzsgdJPXc_FX6XkLpDvU=mY5c8aVzQ@mail.gmail.com>


[-- Attachment #1.1: Type: text/plain, Size: 996 bytes --]

Ioannis Kappas <ioannis.kappas@gmail.com> writes:

> It is handled correctly as expected if the concatenated sequence is an
> SGR, it is output as such, i.e. all
> test/lisp/ansi-color-tests.el:ansi-color-incomplete-sequences-test
> pass still pass.
>
> Here is the list of unit tests showing of what I consider correct
> handling of non SGR sequences I have came up with thus far
>
> (ert-deftest ansi-color-context-non-sgr ()
>
> [...]
>
>   (with-temp-buffer
>     (let ((pretext (ansi-color-apply "\e[33;"))
>           (text (ansi-color-apply "1mHello World\e[0m")))
>       (should (string= "Hello World" text))
>       (should (equal (get-char-property 2 'font-lock-face text)
>                      '(ansi-color-bold (:foreground "yellow3"))))
>       ))
>   )

Thanks. I took the liberty of working on your patch, adding support for
ansi-color-apply-on-region, ansi-color-filter-region,
ansi-color-filter-apply. I also added some tests as you suggested and
made a minor simplification.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-ansi-color-don-t-get-stuck-on-e.patch --]
[-- Type: text/x-patch, Size: 5364 bytes --]

From 162045f83154d3df7b482871b05076a92efd02f9 Mon Sep 17 00:00:00 2001
From: Ioannis Kappas <ioannis.kappas@gmail.com>
Date: Sun, 6 Feb 2022 21:25:56 +0100
Subject: [PATCH] ansi-color: don't get stuck on \e

* lisp/ansi-color.el (ansi-color--control-seq-fragment-regexp): New
constant.

(ansi-color-filter-apply):
(ansi-color-apply):
(ansi-color-filter-region):
(ansi-color-apply-on-region): Don't get stuck on \e if it is
determined that it cannot start a valid ANSI escape
sequence (Bug#53808).

* test/lisp/ansi-color-tests.el (ansi-color-incomplete-sequences-test):
Test for \e that doesn't start a valid ANSI escape sequence.
---
 lisp/ansi-color.el            | 26 ++++++++++++++++++++------
 test/lisp/ansi-color-tests.el | 20 +++++++++++++++++++-
 2 files changed, 39 insertions(+), 7 deletions(-)

diff --git a/lisp/ansi-color.el b/lisp/ansi-color.el
index 3973d9db08..e5d2e2c4ac 100644
--- a/lisp/ansi-color.el
+++ b/lisp/ansi-color.el
@@ -347,6 +347,10 @@ ansi-color-control-seq-regexp
   "\e\\[[\x30-\x3F]*[\x20-\x2F]*[\x40-\x7E]"
   "Regexp matching an ANSI control sequence.")
 
+(defconst ansi-color--control-seq-fragment-regexp
+  "\e\\[[\x30-\x3F]*[\x20-\x2F]*\\|\e"
+  "Regexp matching a partial ANSI control sequence.")
+
 (defconst ansi-color-parameter-regexp "\\([0-9]*\\)[m;]"
   "Regexp that matches SGR control sequence parameters.")
 
@@ -492,7 +496,9 @@ ansi-color-filter-apply
     ;; save context, add the remainder of the string to the result
     (let ((fragment ""))
       (push (substring string start
-                       (if (string-match "\033" string start)
+                       (if (string-match
+                            (concat "\\(?:" ansi-color--control-seq-fragment-regexp "\\)\\'")
+                            string start)
                            (let ((pos (match-beginning 0)))
                              (setq fragment (substring string pos))
                              pos)
@@ -549,7 +555,9 @@ ansi-color-apply
       (put-text-property start (length string)
                          'font-lock-face face string))
     ;; save context, add the remainder of the string to the result
-    (if (string-match "\033" string start)
+    (if (string-match
+         (concat "\\(?:" ansi-color--control-seq-fragment-regexp "\\)\\'")
+         string start)
         (let ((pos (match-beginning 0)))
           (setcar (cdr context) (substring string pos))
           (push (substring string start pos) result))
@@ -685,7 +693,11 @@ ansi-color-filter-region
       (while (re-search-forward ansi-color-control-seq-regexp end-marker t)
         (delete-region (match-beginning 0) (match-end 0)))
       ;; save context, add the remainder of the string to the result
-      (if (re-search-forward "\033" end-marker t)
+      (set-marker start (point))
+      (while (re-search-forward ansi-color--control-seq-fragment-regexp
+                                end-marker t))
+      (if (and (/= (point) start)
+               (= (point) end-marker))
 	  (set-marker start (match-beginning 0))
         (set-marker start nil)))))
 
@@ -742,10 +754,12 @@ ansi-color-apply-on-region
             ;; Otherwise, strip.
             (delete-region esc-beg esc-end))))
       ;; search for the possible start of a new escape sequence
-      (if (re-search-forward "\033" end-marker t)
+      (while (re-search-forward ansi-color--control-seq-fragment-regexp
+                                end-marker t))
+      (if (and (/= (point) start-marker)
+               (= (point) end-marker))
           (progn
-            (while (re-search-forward "\033" end-marker t))
-            (backward-char)
+            (goto-char (match-beginning 0))
             (funcall ansi-color-apply-face-function
                      start-marker (point)
                      (ansi-color--face-vec-face face-vec))
diff --git a/test/lisp/ansi-color-tests.el b/test/lisp/ansi-color-tests.el
index 71b706c763..2ff7fc6aaf 100644
--- a/test/lisp/ansi-color-tests.el
+++ b/test/lisp/ansi-color-tests.el
@@ -171,7 +171,25 @@ ansi-color-incomplete-sequences-test
           (insert str)
           (ansi-color-apply-on-region opoint (point))))
       (should (ansi-color-tests-equal-props
-               propertized-str (buffer-string))))))
+               propertized-str (buffer-string))))
+
+    ;; \e not followed by '[' and invalid ANSI escape seqences
+    (dolist (fun (list ansi-filt ansi-app))
+      (with-temp-buffer
+        (should (equal (funcall fun "\e") ""))
+        (should (equal (funcall fun "\e[33m test \e[0m")
+                       (with-temp-buffer
+                         (concat "\e" (funcall fun "\e[33m test \e[0m"))))))
+      (with-temp-buffer
+        (should (equal (funcall fun "\e[") ""))
+        (should (equal (funcall fun "\e[33m Z \e[0m")
+                       (with-temp-buffer
+                         (concat "\e[" (funcall fun "\e[33m Z \e[0m"))))))
+      (with-temp-buffer
+        (should (equal (funcall fun "\e a \e\e[\e[") "\e a \e\e["))
+        (should (equal (funcall fun "\e[33m Z \e[0m")
+                       (with-temp-buffer
+                         (concat "\e[" (funcall fun "\e[33m Z \e[0m")))))))))
 
 (provide 'ansi-color-tests)
 
-- 
2.34.1


[-- Attachment #1.3: Type: text/plain, Size: 32 bytes --]


Thanks again and best regards.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]

  reply	other threads:[~2022-02-06 20:36 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-05 20:52 bug#53808: 29.0.50; ansi colorization process could block indefinetly on stray ESC char Ioannis Kappas
2022-02-05 21:00 ` Ioannis Kappas
2022-02-05 21:47   ` Ioannis Kappas
2022-02-05 21:56   ` Lars Ingebrigtsen
2022-02-05 22:05     ` Ioannis Kappas
2022-02-06 20:36       ` miha--- via Bug reports for GNU Emacs, the Swiss army knife of text editors [this message]
2022-02-06 22:55         ` Lars Ingebrigtsen
2022-02-07  7:51         ` Ioannis Kappas
2022-02-07 11:42           ` miha--- via Bug reports for GNU Emacs, the Swiss army knife of text editors

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87ee4fwx3o.fsf@miha-pc \
    --to=bug-gnu-emacs@gnu.org \
    --cc=53808@debbugs.gnu.org \
    --cc=ioannis.kappas@gmail.com \
    --cc=larsi@gnus.org \
    --cc=miha@kamnitnik.top \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).