bug#51549: 29.0.50; flymake error when filename contain '%'

bug#51549: 29.0.50; flymake error when filename contain '%'

[Ola x Nilsson]

Flymake includes the buffer name in `warning-type-format' (see
`flymake--log-1').
When the file name - and the buffer name - contain a % character it is
very probable that the `format' call in `display-warning' either fails
or produce unexpected results.

In my case I used a file name arm-trusted-firmware-2.4_%.bbappend with a
local flymake backend running a local linter.
The local backend uses a process sentinel to process errors and generate
flymake diagnostic objects with

   (flymake-make-diagnostic source beg end type msg)

where
source is the buffer visiting arm-trusted-firmware-2.4_%.bbappend,
bed and end set from `flymake-diag-region'
type is :warning
and msg is a string with the error message from the linter.
   


In GNU Emacs 29.0.50 (build 3, x86_64-pc-linux-gnu, GTK+ Version 3.24.5, cairo version 1.16.0)
 of 2021-10-28 built on lnxolani1
Repository revision: cb949963570ad0dbe272109f0245ca21029e2e5c
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12004000
System Description: Debian GNU/Linux 10 (buster)

Configured using:
 'configure --with-native-compilation'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBSELINUX LIBSYSTEMD LIBXML2 MODULES NATIVE_COMP NOTIFY
INOTIFY PDUMPER PNG RSVG SECCOMP SOUND THREADS TIFF TOOLKIT_SCROLL_BARS
X11 XDBE XIM XPM GTK3 ZLIB

Important settings:
  value of $LC_MONETARY: en_GB.UTF-8
  value of $LC_NUMERIC: en_GB.UTF-8
  value of $LC_TIME: en_GB.UTF-8
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix

Major mode: ELisp/l

Minor modes in effect:
  magit-auto-revert-mode: t
  checkdoc-minor-mode: t
  delete-trailing-whitespace-mode: t
  hes-mode: t
  desktop-save-mode: t
  flycheck-mode: t
  global-git-commit-mode: t
  which-function-mode: t
  display-time-mode: t
  global-auto-revert-mode: t
  override-global-mode: t
  shell-dirtrack-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  buffer-read-only: t
  column-number-mode: t
  line-number-mode: t
  indent-tabs-mode: t

Load-path shadows:
/home/olani/.emacs.d/elpa-lnxolani1-29.0.50/dpkg-dev-el-20190824.2314/debian-autoloads hides /home/olani/.emacs.d/elpa-lnxolani1-29.0.50/debian-el-20201011.1543/debian-autoloads
/home/olani/.emacs.d/elpa-lnxolani1-29.0.50/magit-20210928.1325/magit-section-pkg hides /home/olani/.emacs.d/elpa-lnxolani1-29.0.50/magit-section-20210829.1849/magit-section-pkg
/home/olani/.emacs.d/elpa-lnxolani1-29.0.50/transient-20210920.1038/transient hides /usr/local/share/emacs/29.0.50/lisp/transient

Features:
(shadow emacsbug dabbrev edebug cus-edit loadhist vc-dir ewoc
vc-filewise cl-print debug backtrace cus-start cus-load url-http-ntlm
ntlm hmac-md5 hex-util md4 url-http url-gw url-cache eww xdg url-queue
mm-url gnus nnheader rect magit-bundle magit-patch magit-subtree
magit-gitignore magit-imerge ispell macrostep-c cmacexp macrostep
eieio-opt speedbar ezimage dframe shortdoc coan-c c-style-axis
vc-clearcase trace tq coan-lilypond coan-semc coan-todochiku incdec
help-fns hideif hideshow coan-cc cc-mode cc-fonts cc-guess cc-menus
cc-cmds cc-styles cc-align cc-engine cc-vars cc-defs coan-gmake
make-mode magit-ediff ediff ediff-merg ediff-mult ediff-wind ediff-diff
ediff-help ediff-init ediff-util rng-xsd xsd-regexp rng-cmpct rng-nxml
rng-valid rng-loc rng-uri rng-parse nxml-parse rng-match rng-dt rng-util
rng-pttrn nxml-ns nxml-mode nxml-outln nxml-rap sgml-mode facemenu
nxml-util nxml-enc xmltok disp-table view pp descr-text
coan-magit-gerrit magit-gerrit log-view vc-annotate bitbake-build-dir
wid-edit coan-magit-functions mmm-cmds dired-aux coan-dired goto-addr
vc-mtn vc-hg vc-bzr vc-src vc-sccs vc-svn vc-cvs vc-rcs gerrit-buttons
magit-extras magit-imenu git-rebase coan-magit magit-submodule
magit-obsolete magit-popup magit-blame magit-stash magit-reflog
magit-bisect magit-push magit-pull magit-fetch magit-clone magit-remote
magit-commit magit-sequence magit-notes magit-worktree magit-tag
magit-merge magit-branch magit-reset magit-files magit-refs magit-status
magit magit-repos magit-apply magit-wip magit-log magit-diff smerge-mode
diff magit-core magit-autorevert magit-margin magit-transient
magit-process magit-mode vc display-fill-column-indicator shr-color
color shr kinsoku svg xml dom misearch multi-isearch gnutls
network-stream nsm mailalias qp sort face-remap mail-extr coan cl
gee-email gee-gerrit-rest mu4e mu4e-org org ob ob-tangle ob-ref ob-lob
ob-table ob-exp org-macro org-footnote org-src ob-comint org-pcomplete
org-list org-faces org-entities noutline outline org-version
ob-emacs-lisp ob-core ob-eval org-table oc-basic bibtex ol org-keys oc
org-compat org-macs org-loaddefs mu4e-main mu4e-view cal-menu calendar
cal-loaddefs mu4e-headers mu4e-compose mu4e-context mu4e-draft
mu4e-actions ido rfc2368 smtpmail sendmail mu4e-mark mu4e-message
flow-fill mu4e-proc mu4e-utils doc-view image-mode exif mu4e-lists
mule-util hl-line mu4e-vars mu4e-meta bblint let-alist bitbake generic
conf-bitbake-mode bitbake-ff grep find-file bitbake-mmm
flycheck-checkbashisms sh-script smie executable mmm-sh-here-doc
mmm-mode mmm-univ mmm-class mmm-region bitbake-functions bitbake-compat
checkdoc coan-elisp skeleton delete-trailing-whitespace-mode init
highlight-escape-sequences woman man pcase anaphora dired-rainbow
dired-hacks-utils autoinsert mmm-sample mmm-auto mmm-vars mmm-utils
mmm-compat t32-auto coan-list midnight desktop frameset
flycheck-flawfinder flycheck-cstyle flycheck find-func python-black
reformatter python tramp-sh bug-reference find-dired git-commit
with-editor transient server log-edit message rmc puny dired
dired-loaddefs rfc822 mml mml-sec epa derived epg rfc6068 epg-config
gnus-util rmail rmail-loaddefs mm-decode mm-bodies mm-encode mailabbrev
mail-utils gmm-utils mailheader pcvs-util add-log magit-git
magit-section magit-utils crm dash edmacro kmacro gitignore-mode
conf-mode flymake-git-whitespace flymake-proc flymake project compile
text-property-search thingatpt vc-git diff-mode vc-dispatcher advice
coan-29.0.50-autoloads autoload radix-tree lisp-mnt mail-parse rfc2231
rfc2047 rfc2045 mm-util ietf-drums mail-prsvr whitespace which-func
imenu time avoid autorevert filenotify jka-compr use-package
use-package-ensure use-package-delight use-package-diminish
use-package-bind-key bind-key easy-mmode use-package-core finder-inf
coan-system tramp tramp-loaddefs trampver tramp-integration files-x
tramp-compat shell pcomplete comint ansi-color ring parse-time iso8601
time-date ls-lisp format-spec dpkg-dev-el debian-el url-auth info
package browse-url url url-proxy url-privacy url-expand url-methods
url-history url-cookie url-domsuf url-util mailcap url-handlers
url-parse auth-source eieio eieio-core eieio-loaddefs password-cache
json map url-vars comp comp-cstr warnings rx cl-seq cl-macs cl-extra
help-mode seq gv subr-x byte-opt cl-loaddefs cl-lib bytecomp
byte-compile cconv early-init iso-transl tooltip eldoc paren electric
uniquify ediff-hook vc-hooks lisp-float-type elisp-mode mwheel
term/x-win x-win term/common-win x-dnd tool-bar dnd fontset image
regexp-opt fringe tabulated-list replace newcomment text-mode lisp-mode
prog-mode register page tab-bar menu-bar rfn-eshadow isearch easymenu
timer select scroll-bar mouse jit-lock font-lock syntax font-core
term/tty-colors frame minibuffer 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 emoji-zwj charscript charprop case-table
epa-hook jka-cmpr-hook help simple abbrev obarray cl-preloaded nadvice
button loaddefs faces cus-face macroexp files window text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote threads dbusbind inotify lcms2
dynamic-setting system-font-setting font-render-setting cairo
move-toolbar gtk x-toolkit x multi-tty make-network-process
native-compile emacs)

Memory information:
((conses 16 2351826 196419)
 (symbols 48 60287 194)
 (strings 32 373582 28488)
 (string-bytes 1 11152651)
 (vectors 16 149160)
 (vector-slots 8 3218814 467321)
 (floats 8 645 2556)
 (intervals 56 169534 4504)
 (buffers 992 151))

-- 
Ola x Nilsson

bug#51549: 29.0.50; flymake error when filename contain '%'

[Stefan Kangas]

tags 51549 + confirmed
thanks

Ola x Nilsson <ola.x.nilsson@axis.com> writes:

> Flymake includes the buffer name in `warning-type-format' (see
> `flymake--log-1').
> When the file name - and the buffer name - contain a % character it is
> very probable that the `format' call in `display-warning' either fails
> or produce unexpected results.
>
> In my case I used a file name arm-trusted-firmware-2.4_%.bbappend with a
> local flymake backend running a local linter.
> The local backend uses a process sentinel to process errors and generate
> flymake diagnostic objects with
>
>    (flymake-make-diagnostic source beg end type msg)
>
> where
> source is the buffer visiting arm-trusted-firmware-2.4_%.bbappend,
> bed and end set from `flymake-diag-region'
> type is :warning
> and msg is a string with the error message from the linter.

Thanks for the bug report.  I can reproduce this using this contrived
example:

./src/emacs -Q --eval "(progn (require 'flymake) (find-file
\"/tmp/foo%sbar.c\") \
    (flymake--log-1 :emergency nil \"foo\"))"

I now see in the *Warnings* buffer:

    Emergency [flymake fooflymakebar.c]: foo

The expected output is:

    Emergency [flymake foo%sbar.c]: foo

Your analysis of why the problem happens seems correct to me, but I'm
not sure how to best fix it.  João, what do you think?

bug#51549: 29.0.50; flymake error when filename contain '%'

[João Távora]

Hi Stefan, Ola

Thanks for the report.  I don't have much time to look into this but
this looks like the typical problem where a user-provided
string is being passed as the first argument of format/message.

In these cases, one normally makes it so that the first
argument is simply the string "%s" and then the second
argument is that user-provided thing.

Thanks very much for looking into this,
João

On Sat, Nov 6, 2021 at 8:30 AM Stefan Kangas <stefan@marxist.se> wrote:
>
> tags 51549 + confirmed
> thanks
>
> Ola x Nilsson <ola.x.nilsson@axis.com> writes:
>
> > Flymake includes the buffer name in `warning-type-format' (see
> > `flymake--log-1').
> > When the file name - and the buffer name - contain a % character it is
> > very probable that the `format' call in `display-warning' either fails
> > or produce unexpected results.
> >
> > In my case I used a file name arm-trusted-firmware-2.4_%.bbappend with a
> > local flymake backend running a local linter.
> > The local backend uses a process sentinel to process errors and generate
> > flymake diagnostic objects with
> >
> >    (flymake-make-diagnostic source beg end type msg)
> >
> > where
> > source is the buffer visiting arm-trusted-firmware-2.4_%.bbappend,
> > bed and end set from `flymake-diag-region'
> > type is :warning
> > and msg is a string with the error message from the linter.
>
> Thanks for the bug report.  I can reproduce this using this contrived
> example:
>
> ./src/emacs -Q --eval "(progn (require 'flymake) (find-file
> \"/tmp/foo%sbar.c\") \
>     (flymake--log-1 :emergency nil \"foo\"))"
>
> I now see in the *Warnings* buffer:
>
>     Emergency [flymake fooflymakebar.c]: foo
>
> The expected output is:
>
>     Emergency [flymake foo%sbar.c]: foo
>
> Your analysis of why the problem happens seems correct to me, but I'm
> not sure how to best fix it.  João, what do you think?



-- 
João Távora

bug#51549: 29.0.50; flymake error when filename contain '%'

[Stefan Kangas]

João Távora <joaotavora@gmail.com> writes:

> Thanks for the report.  I don't have much time to look into this but
> this looks like the typical problem where a user-provided
> string is being passed as the first argument of format/message.
>
> In these cases, one normally makes it so that the first
> argument is simply the string "%s" and then the second
> argument is that user-provided thing.

That's what I thought too, until I looked at the code.  Unfortunately,
the fix we both thought of isn't that trivial as we set
`warning-type-format' like this:

    (warning-type-format
             (format " [%s %s]"
                     (or sublog 'flymake)
                     (current-buffer)))

And then we run `display-warning', which uses this variable as the first
argument to format.

Is there a strong need to have this format for flymake warnings?

    Emergency [flymake foo%sbar.c]: foo

If not, my best idea for fixing this is to remove the filename from the
`warning-type-format', and put it in the message instead.  That would be
a backwards-incompatible change, however.

bug#51549: 29.0.50; flymake error when filename contain '%'

[João Távora]

On Sat, Nov 6, 2021 at 11:08 AM Stefan Kangas <stefan@marxist.se> wrote:

> That's what I thought too, until I looked at the code.  Unfortunately,
> the fix we both thought of isn't that trivial as we set
> `warning-type-format' like this:
>
>     (warning-type-format
>              (format " [%s %s]"
>                      (or sublog 'flymake)
>                      (current-buffer)))

Ah I see.  Then maybe the problem is how that is used forward.

> And then we run `display-warning', which uses this variable as the first
> argument to format.
>
> Is there a strong need to have this format for flymake warnings?

Absolutely not.  This is probably one of the most over-engineered
things I put in Flymake.  It's occasionally useful, but not terribly so.

>     Emergency [flymake foo%sbar.c]: foo
>
> If not, my best idea for fixing this is to remove the filename from the
> `warning-type-format', and put it in the message instead.  That would be
> a backwards-incompatible change, however.

Yes, do what you think is best, and thanks in advance.
It's good to have the buffer name where the warning came from,
but it doesn't have to be in that exact place.  Maybe just
escaping the "%" in the return value of `current-buffer`
would work though?  Doubling the "%" into "%%", perhaps.

João

bug#51549: 29.0.50; flymake error when filename contain '%'

[Stefan Kangas]

close 51549 28.1
thanks

João Távora <joaotavora@gmail.com> writes:

> Yes, do what you think is best, and thanks in advance.
> It's good to have the buffer name where the warning came from,
> but it doesn't have to be in that exact place.  Maybe just
> escaping the "%" in the return value of `current-buffer`
> would work though?  Doubling the "%" into "%%", perhaps.

Doh!  Of course, that's the other obvious thing to do, thanks for
reminding me.  Now fixed on emacs-28 in commit d85cf33e49.

Ola, thanks again for the bug report.