bug#33524: 27.0.50; Suspicious code in flymake-proc around temporary directories

[João Távora]

No, it looks great, for all I know... Which is very little :-)

Flymake-proc is the "legacy" backend that I packed up in a file. It probably has many such bugs.

Thanks very much Michael and Philipp,

João

On Fri, Nov 30, 2018 at 12:59 PM Michael Albinus <michael.albinus@gmx.de> wrote:

Philipp Stephani <p.stephani2@gmail.com> writes:

Hi Philipp,

> Sorry for being imprecise. Yes, I mean quoted filenames. (It's
> possible that the issue also arises for true remote filenames, but I
> haven't checked.)

"True remote filename" is still a wrong phrase. Quoted file names and
remote file names play different games.

> You can easily reproduce user-facing problems. Assuming you have some
> binary installed that would check Java files using the legacy backend:
>
> $ touch /tmp/{a,b}.java
> $ emacs -Q -eval '(progn (add-hook (quote prog-mode-hook) (quote
> flymake-mode)) (ediff "/:/tmp/a.java" "/:/tmp/b.java"))'
>
> And Emacs immediately hangs because it runs into the endless loop.
> Stacktrace in this case is
>
> Debugger entered--Lisp error: (quit)
>   display-warning((flymake flymake-proc) "Failed to delete dir /,
> error ignored" :error "*Flymake log*")
>   flymake--log-1(1 flymake-proc "Failed to delete dir %s, error
> ignored" "/")
>   flymake-proc--safe-delete-directory("/")
>   flymake-proc--delete-temp-directory("/tmp/:/tmp/")
>   flymake-proc-simple-java-cleanup()
>   #f(compiled-function () #<bytecode 0x11a4ea9>)()
>   flymake-proc-legacy-flymake(#f(compiled-function (&rest args)
> #<bytecode 0x11a4e51>))
>   flymake--run-backend(flymake-proc-legacy-flymake)
>   #f(compiled-function (backend) #<bytecode 0x117b195>)
> (flymake-proc-legacy-flymake)
>   run-hook-wrapped(#f(compiled-function (backend) #<bytecode
> 0x117b195>) flymake-proc-legacy-flymake)
>   flymake-start((post-command) nil)
>   #f(compiled-function () #<bytecode 0x1180771>)()
>   set-window-buffer(nil #<buffer a.java>)
>   switch-to-buffer(#<buffer a.java>)
>   ediff-setup-windows-multiframe-compare(#<buffer a.java> #<buffer
> b.java> nil #<buffer *Ediff Control Panel*>)
>   ediff-setup-windows-multiframe(#<buffer a.java> #<buffer b.java> nil
> #<buffer *Ediff Control Panel*>)
>   ediff-setup-windows-default(#<buffer a.java> #<buffer b.java> nil
> #<buffer *Ediff Control Panel*>)
>   ediff-setup-windows(#<buffer a.java> #<buffer b.java> nil #<buffer
> *Ediff Control Panel*>)
>   ediff-setup(#<buffer a.java> "/:/tmp/a.java" #<buffer b.java>
> "/:/tmp/b.java" nil nil nil ((ediff-job-name . ediff-files)) nil)
>   ediff-files-internal("/:/tmp/a.java" "/:/tmp/b.java" nil nil
> ediff-files)
>   ediff("/:/tmp/a.java" "/:/tmp/b.java")
>   (progn (add-hook (quote prog-mode-hook) (quote flymake-mode)) (ediff
> "/:/tmp/a.java" "/:/tmp/b.java"))
>   eval((progn (add-hook (quote prog-mode-hook) (quote flymake-mode))
> (ediff "/:/tmp/a.java" "/:/tmp/b.java")))
>   command-line-1(("-f" "toggle-debug-on-quit" "-eval" "(progn
> (add-hook (quote prog-mode-hook) (quote flymake-mode)) (ediff
> \"/:/tmp/a.java\" \"/:/tmp/b.java\"))"))
>   command-line()
>   normal-top-level()

The following patch fixes this problem:

diff --git a/lisp/progmodes/flymake-proc.el b/lisp/progmodes/flymake-proc.el
index 8600be9b97..e969c5d992 100644
--- a/lisp/progmodes/flymake-proc.el
+++ b/lisp/progmodes/flymake-proc.el
@@ -874,7 +874,7 @@ flymake-proc-create-temp-with-folder-structure
   (unless (stringp file-name)
     (error "Invalid file-name"))

-  (let* ((dir       (file-name-directory file-name))
+  (let* ((dir       (file-name-directory (file-name-unquote file-name)))
          ;; Not sure what this slash-pos is all about, but I guess it's just
          ;; trying to remove the leading / of absolute file names.
         (slash-pos (string-match "/" dir))

However, I don't know whether this is sufficient, because I don't know
why you use quoted file names in your example. Is this something which
shall be preserved in the temporary directory, created by flymake?

João, do you have further remarks?

Best regards, Michael.

--

João Távora