Subject: Re: bug#78184: 30.0.93; ansi-osc-apply-on-region should use end marker

From: Eli Zaretskii <[email-masked]>
Date: Sat, 10 May 2025 13:52:12 +0300

> From: Matthias Meulien <[email-masked]>
> Date: Thu, 8 May 2025 17:12:50 +0200
> Cc: Troy Brown <[email-masked]>, [email-masked]
> 
> Troy analysis is right.  I was able to reproduce.  Please have a look to the attached patch.

Thanks.  Troy, could you please try the patch and report?

> Le lun. 5 mai 2025 à 21:41, Matthias Meulien <[email-masked]> a écrit :
> 
>  I'll have a look at the end of the week.
> 
>  Le sam. 3 mai 2025 à 11:46, Eli Zaretskii <[email-masked]> a écrit :
> 
>  > Date: Thu, 1 May 2025 07:47:56 -0400
>  > From:  Troy Brown via "Bug reports for GNU Emacs,
>  >  the Swiss army knife of text editors" <[email-masked]>
>  > 
>  > I've been running into an issue when using the ANSI OSC compilation
>  > filters, where part way through parsing input, I'll get an "args out
>  > of range" error on the call to `put-text-property`.  I believe this is
>  > because `ansi-osc-apply-on-region` uses `delete-region` and therefore
>  > the buffer can shrink from its original size.  Due to this shrinking,
>  > the use of "end", in the call to `put-text-property` can reside
>  > outside the current bounds of the buffer, causing the error.
>  > 
>  > Looking at similar functionality in `ansi-color-apply-on-region`, I
>  > noticed that it creates a marker for "end" at the beginning of the
>  > function, avoiding this problem.  I believe the ANSI OSC package
>  > should use a similar approach and create a marker for "end".  It's
>  > also likely that a marker should be used in other functions within the
>  > package, such as `ansi-osc-filter-region`, although
>  > `re-search-forward` doesn't seem to be susceptible to the range issue
>  > that `put-text-property` is.
>  > 
>  > Currently, I use the following advice to workaround the problem by
>  > ensuring that "end" is a marker before calling the original function:
>  > 
>  > ```emacs
>  > (defun init.el/advice/ansi-osc-apply-on-region (oldfun &rest r)
>  >   (let ((start (car r))
>  >         (end (cadr r)))
>  >     (funcall oldfun start (copy-marker end))))
>  > (advice-add 'ansi-osc-apply-on-region :around
>  > #'init.el/advice/ansi-osc-apply-on-region)
>  > ```
> 
>  Matthias, any comments on this?
> 
>  -- 
>  Matthias
> 
> -- 
> Matthias
> 
> 
> From 7838eaf084456388d129139c84c556c14fe2be5a Mon Sep 17 00:00:00 2001
> From: Matthias Meulien <[email-masked]>
> Date: Thu, 8 May 2025 16:51:46 +0200
> Subject: [PATCH] ansi-osc.el: Use marker (bug#78184)
> 
> * lisp/ansi-osc.el (ansi-osc-apply-on-region): Use marker to fix
> possible out of range argument when hiding unfinished escape
> sequence.
> 
> * test/lisp/ansi-osc-tests.el (ansi-osc-tests--strings)
> (ansi-osc-tests-apply-region-no-handlers)
> (ansi-osc-tests-filter-region): Cover bug#78184.
> ---
>  lisp/ansi-osc.el            | 38 +++++++++++++++++++------------------
>  test/lisp/ansi-osc-tests.el | 15 +++++++++++++++
>  2 files changed, 35 insertions(+), 18 deletions(-)
> 
> diff --git a/lisp/ansi-osc.el b/lisp/ansi-osc.el
> index 97d6f6c8754..b002b89c924 100644
> --- a/lisp/ansi-osc.el
> +++ b/lisp/ansi-osc.el
> @@ -70,24 +70,26 @@ ansi-osc-apply-on-region
>  value of the local variable `ansi-osc-handlers', that key's
>  value, which should be a function, is called with `command' and
>  `text' as arguments, with point where the escape sequence was
> -located."
> -  (save-excursion
> -    (goto-char (or ansi-osc--marker begin))
> -    (when (eq (char-before) ?\e) (backward-char))
> -    (while (re-search-forward "\e]" end t)
> -      (let ((pos0 (match-beginning 0))
> -            (code (and (re-search-forward "\\=\\([0-9A-Za-z]*\\);" end t)
> -                       (match-string 1)))
> -            (pos1 (point)))
> -        (if (re-search-forward "\a\\|\e\\\\" end t)
> -            (let ((text (buffer-substring-no-properties
> -                         pos1 (match-beginning 0))))
> -              (setq ansi-osc--marker nil)
> -              (delete-region pos0 (point))
> -              (when-let* ((fun (cdr (assoc-string code ansi-osc-handlers))))
> -                (funcall fun code text)))
> -          (put-text-property pos0 end 'invisible t)
> -          (setq ansi-osc--marker (copy-marker pos0)))))))
> +located.  Unfinished escape sequences are hidden."
> +  (let ((end-marker (copy-marker end)))
> +    (save-excursion
> +      (goto-char (or ansi-osc--marker begin))
> +      (when (eq (char-before) ?\e) (backward-char))
> +      (while (re-search-forward "\e]" end-marker t)
> +        (let ((pos0 (match-beginning 0))
> +              (code (and
> +                     (re-search-forward "\\=\\([0-9A-Za-z]*\\);" end-marker t)
> +                     (match-string 1)))
> +              (pos1 (point)))
> +          (if (re-search-forward "\a\\|\e\\\\" end-marker t)
> +              (let ((text (buffer-substring-no-properties
> +                           pos1 (match-beginning 0))))
> +                (setq ansi-osc--marker nil)
> +                (delete-region pos0 (point))
> +                (when-let* ((fun (cdr (assoc-string code ansi-osc-handlers))))
> +                  (funcall fun code text)))
> +            (put-text-property pos0 end-marker 'invisible t)
> +            (setq ansi-osc--marker (copy-marker pos0))))))))
>  
>  ;; Window title handling (OSC 2)
>  
> diff --git a/test/lisp/ansi-osc-tests.el b/test/lisp/ansi-osc-tests.el
> index d2fb130e518..28cf8f41d59 100644
> --- a/test/lisp/ansi-osc-tests.el
> +++ b/test/lisp/ansi-osc-tests.el
> @@ -44,6 +44,10 @@ ansi-osc-tests--strings
>  
>      ;; hyperlink
>      ("\e]8;;http://example.com\e\\This is a link\e]8;;\e\\" "This is a link")
> +
> +    ;; multiple sequences
> +    ("Escape \e]2;A window title\e\\sequence followed by \e]2;unfinished sequence"
> +     "Escape sequence followed by \e]2;unfinished sequence")
>      ))
>  ;; Don't output those strings to stdout since they may have
>  ;; side-effects on the environment
> @@ -54,4 +58,15 @@ ansi-osc-tests-apply-region-no-handlers
>        (with-temp-buffer
>          (insert input)
>          (ansi-osc-apply-on-region (point-min) (point-max))
> +        (should (equal
> +                 (buffer-substring-no-properties
> +                  (point-min) (point-max))
> +                 text))))))
> +
> +(ert-deftest ansi-osc-tests-filter-region ()
> +  (let ((ansi-osc-handlers nil))
> +    (pcase-dolist (`(,input ,text) ansi-osc-tests--strings)
> +      (with-temp-buffer
> +        (insert input)
> +        (ansi-osc-filter-region (point-min) (point-max))
>          (should (equal (buffer-string) text))))))
> -- 
> 2.39.5
>