GNU bug report logs - #49723
28.0.50; Test in coding.c for NUL bytes in filenames is not reliable

Previous Next

Package: emacs;

Reported by: Eli Zaretskii <eliz <at> gnu.org>

Date: Sat, 24 Jul 2021 17:40:01 UTC

Severity: normal

Found in version 28.0.50

Done: Eli Zaretskii <eliz <at> gnu.org>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 49723 in the body.
You can then email your comments to 49723 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#49723; Package emacs. (Sat, 24 Jul 2021 17:40:01 GMT) Full text and rfc822 format available.

Acknowledgement sent to Eli Zaretskii <eliz <at> gnu.org>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Sat, 24 Jul 2021 17:40:01 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: bug-gnu-emacs <at> gnu.org
Cc: Philipp Stephani <phst <at> google.com>
Subject: 28.0.50; Test in coding.c for NUL bytes in filenames is not reliable
Date: Sat, 24 Jul 2021 20:39:22 +0300
From: eliz <at> HOME-C4E4A596F7.i-did-not-set--mail-host-address--so-tickle-me
--text follows this line--
Emacs currently tries to catch null bytes in file names in
encode_file_name, after encoding the file name.  But that is not
reliable enough, because it assumes that the call to expand-file-name
at the beginning of encode_file_name leaves those null bytes intact.
That is not guaranteed, and in fact doesn't happen at least on
MS-Windows: the file name gets chopped after the first null byte, and
doesn't trigger the test in encode_file_name.

So for a more reliable test we need to include such a check inside
expand-file-name (and then we could probably drop the test in
encode_file_name, because Emacs always runs file names through
expand-file-name before using them).

In GNU Emacs 28.0.50 (build 1573, i686-pc-mingw32)
 of 2021-07-24 built on HOME-C4E4A596F7
Repository revision: 7c83e605ab84e8b62254c55f347abc8aa9c6057b
Repository branch: master
Windowing system distributor 'Microsoft Corp.', version 5.1.2600
System Description: Microsoft Windows XP Service Pack 3 (v5.1.0.2600)

Configured using:
 'configure -C --prefix=/d/usr --with-wide-int --with-modules
 --enable-checking=yes,glyphs 'CFLAGS=-O0 -gdwarf-4 -g3''

Configured features:
ACL GIF GMP GNUTLS HARFBUZZ JPEG JSON LCMS2 LIBXML2 MODULES NOTIFY
W32NOTIFY PDUMPER PNG RSVG SOUND THREADS TIFF TOOLKIT_SCROLL_BARS XPM
ZLIB

Important settings:
  value of $LANG: ENU
  locale-coding-system: cp1255

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-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
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
rfc822 mml mml-sec epa derived epg epg-config gnus-util rmail
rmail-loaddefs auth-source cl-seq eieio eieio-core cl-macs
eieio-loaddefs password-cache json map text-property-search time-date
subr-x seq byte-opt gv bytecomp byte-compile cconv mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader cl-loaddefs
cl-lib sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils
iso-transl tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type mwheel dos-w32 ls-lisp disp-table term/w32-win w32-win
w32-vars term/common-win tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode elisp-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 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 w32notify w32 lcms2 multi-tty
make-network-process emacs)

Memory information:
((conses 16 56888 7103)
 (symbols 48 7785 1)
 (strings 16 21560 2884)
 (string-bytes 1 630636)
 (vectors 16 13320)
 (vector-slots 8 173301 9720)
 (floats 8 23 47)
 (intervals 40 265 116)
 (buffers 888 10))




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49723; Package emacs. (Tue, 14 Sep 2021 19:02:02 GMT) Full text and rfc822 format available.

Message #8 received at 49723 <at> debbugs.gnu.org (full text, mbox):

From: Federico Tedin <federicotedin <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Philipp Stephani <phst <at> google.com>, 49723 <at> debbugs.gnu.org
Subject: Re: bug#49723: 28.0.50; Test in coding.c for NUL bytes in filenames
 is not reliable
Date: Tue, 14 Sep 2021 21:01:16 +0200
I'm interested in looking into this one since I want to learn more about
the C side of the codebase. However, I wasn't able to find a call to
expand-file-name in encode_file_name or encode_file_name_1. I did find
the null byte check though (CHECK_TYPE + memchr). Maybe I am missing
something out.

I assume that a similar check on expand-file-name should be applied to
both input arguments, NAME and DEFAULT-DIRECTORY?

Thanks!

Eli Zaretskii <eliz <at> gnu.org> writes:

> From: eliz <at> HOME-C4E4A596F7.i-did-not-set--mail-host-address--so-tickle-me
> --text follows this line--
> Emacs currently tries to catch null bytes in file names in
> encode_file_name, after encoding the file name.  But that is not
> reliable enough, because it assumes that the call to expand-file-name
> at the beginning of encode_file_name leaves those null bytes intact.
> That is not guaranteed, and in fact doesn't happen at least on
> MS-Windows: the file name gets chopped after the first null byte, and
> doesn't trigger the test in encode_file_name.
>
> So for a more reliable test we need to include such a check inside
> expand-file-name (and then we could probably drop the test in
> encode_file_name, because Emacs always runs file names through
> expand-file-name before using them).
>
> In GNU Emacs 28.0.50 (build 1573, i686-pc-mingw32)
>  of 2021-07-24 built on HOME-C4E4A596F7
> Repository revision: 7c83e605ab84e8b62254c55f347abc8aa9c6057b
> Repository branch: master
> Windowing system distributor 'Microsoft Corp.', version 5.1.2600
> System Description: Microsoft Windows XP Service Pack 3 (v5.1.0.2600)
>
> Configured using:
>  'configure -C --prefix=/d/usr --with-wide-int --with-modules
>  --enable-checking=yes,glyphs 'CFLAGS=-O0 -gdwarf-4 -g3''
>
> Configured features:
> ACL GIF GMP GNUTLS HARFBUZZ JPEG JSON LCMS2 LIBXML2 MODULES NOTIFY
> W32NOTIFY PDUMPER PNG RSVG SOUND THREADS TIFF TOOLKIT_SCROLL_BARS XPM
> ZLIB
>
> Important settings:
>   value of $LANG: ENU
>   locale-coding-system: cp1255
>
> Major mode: Lisp Interaction
>
> Minor modes in effect:
>   tooltip-mode: t
>   global-eldoc-mode: t
>   eldoc-mode: t
>   electric-indent-mode: t
>   mouse-wheel-mode: t
>   tool-bar-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
>   line-number-mode: t
>   indent-tabs-mode: t
>   transient-mark-mode: t
>
> Load-path shadows:
> None found.
>
> Features:
> (shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
> rfc822 mml mml-sec epa derived epg epg-config gnus-util rmail
> rmail-loaddefs auth-source cl-seq eieio eieio-core cl-macs
> eieio-loaddefs password-cache json map text-property-search time-date
> subr-x seq byte-opt gv bytecomp byte-compile cconv mm-decode mm-bodies
> mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader cl-loaddefs
> cl-lib sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils
> iso-transl tooltip eldoc electric uniquify ediff-hook vc-hooks
> lisp-float-type mwheel dos-w32 ls-lisp disp-table term/w32-win w32-win
> w32-vars term/common-win tool-bar dnd fontset image regexp-opt fringe
> tabulated-list replace newcomment text-mode elisp-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 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 w32notify w32 lcms2 multi-tty
> make-network-process emacs)
>
> Memory information:
> ((conses 16 56888 7103)
>  (symbols 48 7785 1)
>  (strings 16 21560 2884)
>  (string-bytes 1 630636)
>  (vectors 16 13320)
>  (vector-slots 8 173301 9720)
>  (floats 8 23 47)
>  (intervals 40 265 116)
>  (buffers 888 10))




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49723; Package emacs. (Tue, 14 Sep 2021 19:25:01 GMT) Full text and rfc822 format available.

Message #11 received at 49723 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Federico Tedin <federicotedin <at> gmail.com>
Cc: phst <at> google.com, 49723 <at> debbugs.gnu.org
Subject: Re: bug#49723: 28.0.50; Test in coding.c for NUL bytes in filenames
 is not reliable
Date: Tue, 14 Sep 2021 22:24:22 +0300
> From: Federico Tedin <federicotedin <at> gmail.com>
> Cc: 49723 <at> debbugs.gnu.org,  Philipp Stephani <phst <at> google.com>
> Date: Tue, 14 Sep 2021 21:01:16 +0200
> 
> I'm interested in looking into this one since I want to learn more about
> the C side of the codebase. However, I wasn't able to find a call to
> expand-file-name in encode_file_name or encode_file_name_1. I did find
> the null byte check though (CHECK_TYPE + memchr). Maybe I am missing
> something out.

My description was inaccurate: the expand-file-name call usually
precedes the call to ENCODE_FILE, it is not part of encode_file_name.

> I assume that a similar check on expand-file-name should be applied to
> both input arguments, NAME and DEFAULT-DIRECTORY?

I don't think we need that because expand-file-name calls itself on
DEFAULT-DIRECTORY internally.  But we may need to perform the check on
default-directory, if we use it inside expand-file-name.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49723; Package emacs. (Tue, 14 Sep 2021 22:18:01 GMT) Full text and rfc822 format available.

Message #14 received at 49723 <at> debbugs.gnu.org (full text, mbox):

From: Federico Tedin <federicotedin <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: phst <at> google.com, 49723 <at> debbugs.gnu.org
Subject: Re: bug#49723: 28.0.50; Test in coding.c for NUL bytes in filenames
 is not reliable
Date: Wed, 15 Sep 2021 00:17:33 +0200
[Message part 1 (text/plain, inline)]
Ok! Here's my first try at this. I ended up skipping the check on
DEFAULT-DIRECTORY since as you mentioned, its value is used with
expand-file-name itself. In the other case, if default-directory is
picked up, then I checked the value of that variable.

[expand-file-name.patch (text/x-diff, attachment)]
[Message part 3 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Federico Tedin <federicotedin <at> gmail.com>
>> Cc: 49723 <at> debbugs.gnu.org,  Philipp Stephani <phst <at> google.com>
>> Date: Tue, 14 Sep 2021 21:01:16 +0200
>> 
>> I'm interested in looking into this one since I want to learn more about
>> the C side of the codebase. However, I wasn't able to find a call to
>> expand-file-name in encode_file_name or encode_file_name_1. I did find
>> the null byte check though (CHECK_TYPE + memchr). Maybe I am missing
>> something out.
>
> My description was inaccurate: the expand-file-name call usually
> precedes the call to ENCODE_FILE, it is not part of encode_file_name.
>
>> I assume that a similar check on expand-file-name should be applied to
>> both input arguments, NAME and DEFAULT-DIRECTORY?
>
> I don't think we need that because expand-file-name calls itself on
> DEFAULT-DIRECTORY internally.  But we may need to perform the check on
> default-directory, if we use it inside expand-file-name.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49723; Package emacs. (Thu, 16 Sep 2021 12:26:01 GMT) Full text and rfc822 format available.

Message #17 received at 49723 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Federico Tedin <federicotedin <at> gmail.com>
Cc: phst <at> google.com, 49723 <at> debbugs.gnu.org
Subject: Re: bug#49723: 28.0.50; Test in coding.c for NUL bytes in filenames
 is not reliable
Date: Thu, 16 Sep 2021 15:25:24 +0300
> From: Federico Tedin <federicotedin <at> gmail.com>
> Cc: phst <at> google.com,  49723 <at> debbugs.gnu.org
> Date: Wed, 15 Sep 2021 00:17:33 +0200
> 
> Ok! Here's my first try at this. I ended up skipping the check on
> DEFAULT-DIRECTORY since as you mentioned, its value is used with
> expand-file-name itself. In the other case, if default-directory is
> picked up, then I checked the value of that variable.

Thanks, this LGTM.

> +** 'expand-file-name' now checks for null bytes in filenames.
> +The function will now check for null bytes in both NAME and
> +DEFAULT-DIRECTORY arguments, as well as in the 'default-directory'
> +buffer-local variable, assuming its value is used.

This should say that if null bytes are found, expand-file-name will
signal an error.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49723; Package emacs. (Thu, 16 Sep 2021 16:59:01 GMT) Full text and rfc822 format available.

Message #20 received at 49723 <at> debbugs.gnu.org (full text, mbox):

From: Federico Tedin <federicotedin <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: phst <at> google.com, 49723 <at> debbugs.gnu.org
Subject: Re: bug#49723: 28.0.50; Test in coding.c for NUL bytes in filenames
 is not reliable
Date: Thu, 16 Sep 2021 18:58:02 +0200
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes:

>> +** 'expand-file-name' now checks for null bytes in filenames.
>> +The function will now check for null bytes in both NAME and
>> +DEFAULT-DIRECTORY arguments, as well as in the 'default-directory'
>> +buffer-local variable, assuming its value is used.
>
> This should say that if null bytes are found, expand-file-name will
> signal an error.

Makes sense! I'm attaching a revised version.

[expand-file-name.patch (text/x-diff, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49723; Package emacs. (Thu, 16 Sep 2021 18:13:02 GMT) Full text and rfc822 format available.

Message #23 received at 49723 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Federico Tedin <federicotedin <at> gmail.com>
Cc: phst <at> google.com, Eli Zaretskii <eliz <at> gnu.org>, 49723 <at> debbugs.gnu.org
Subject: Re: bug#49723: 28.0.50; Test in coding.c for NUL bytes in filenames
 is not reliable
Date: Thu, 16 Sep 2021 20:12:09 +0200
Federico Tedin <federicotedin <at> gmail.com> writes:

Hi,

Sorry for being late, I have seen this just now only.

> +** 'expand-file-name' now checks for null bytes in filenames.
> +The function will now check for null bytes in both NAME and
> +DEFAULT-DIRECTORY arguments, as well as in the 'default-directory'
> +buffer-local variable, assuming its value is used.  If null bytes are
> +found, 'expand-file-name' will signal an error.

Should this be implemented also in remote file names?

Best regards, Michael.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49723; Package emacs. (Thu, 16 Sep 2021 18:31:02 GMT) Full text and rfc822 format available.

Message #26 received at 49723 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: phst <at> google.com, 49723 <at> debbugs.gnu.org, federicotedin <at> gmail.com
Subject: Re: bug#49723: 28.0.50; Test in coding.c for NUL bytes in filenames
 is not reliable
Date: Thu, 16 Sep 2021 21:30:18 +0300
> From: Michael Albinus <michael.albinus <at> gmx.de>
> Cc: Eli Zaretskii <eliz <at> gnu.org>,  phst <at> google.com,  49723 <at> debbugs.gnu.org
> Date: Thu, 16 Sep 2021 20:12:09 +0200
> 
> > +** 'expand-file-name' now checks for null bytes in filenames.
> > +The function will now check for null bytes in both NAME and
> > +DEFAULT-DIRECTORY arguments, as well as in the 'default-directory'
> > +buffer-local variable, assuming its value is used.  If null bytes are
> > +found, 'expand-file-name' will signal an error.
> 
> Should this be implemented also in remote file names?

Are we sure remote file names cannot include null bytes?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49723; Package emacs. (Thu, 16 Sep 2021 18:39:02 GMT) Full text and rfc822 format available.

Message #29 received at 49723 <at> debbugs.gnu.org (full text, mbox):

From: Federico Tedin <federicotedin <at> gmail.com>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: phst <at> google.com, Eli Zaretskii <eliz <at> gnu.org>, 49723 <at> debbugs.gnu.org
Subject: Re: bug#49723: 28.0.50; Test in coding.c for NUL bytes in filenames
 is not reliable
Date: Thu, 16 Sep 2021 20:38:03 +0200
Michael Albinus <michael.albinus <at> gmx.de> writes:

> Federico Tedin <federicotedin <at> gmail.com> writes:
>
> Hi,
>
> Sorry for being late, I have seen this just now only.
>
>> +** 'expand-file-name' now checks for null bytes in filenames.
>> +The function will now check for null bytes in both NAME and
>> +DEFAULT-DIRECTORY arguments, as well as in the 'default-directory'
>> +buffer-local variable, assuming its value is used.  If null bytes are
>> +found, 'expand-file-name' will signal an error.
>
> Should this be implemented also in remote file names?
>
> Best regards, Michael.

Would this imply only doing it in `tramp-handle-expand-file-name`, or
other functions as well?

I had thought of doing it for at least `tramp-handle-expand-file-name`,
but then I noticed that the function eventually calls `expand-file-name`
with a combination of name + dir, or only with name. So in that case I
figured that the null bytes check would be performed by
`expand-file-name` anyways.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49723; Package emacs. (Thu, 16 Sep 2021 18:45:02 GMT) Full text and rfc822 format available.

Message #32 received at 49723 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: phst <at> google.com, 49723 <at> debbugs.gnu.org, federicotedin <at> gmail.com
Subject: Re: bug#49723: 28.0.50; Test in coding.c for NUL bytes in filenames
 is not reliable
Date: Thu, 16 Sep 2021 20:44:32 +0200
Eli Zaretskii <eliz <at> gnu.org> writes:

Hi Eli,

>> > +** 'expand-file-name' now checks for null bytes in filenames.
>> > +The function will now check for null bytes in both NAME and
>> > +DEFAULT-DIRECTORY arguments, as well as in the 'default-directory'
>> > +buffer-local variable, assuming its value is used.  If null bytes are
>> > +found, 'expand-file-name' will signal an error.
>>
>> Should this be implemented also in remote file names?
>
> Are we sure remote file names cannot include null bytes?

Likely not. I have added "foo\0bar" as file name in tramp-test.el, and
then I get

--8<---------------cut here---------------start------------->8---
Test tramp-test41-special-characters condition:
    (ert-test-failed
     ((should
       (file-exists-p file1))
      :form
      (file-exists-p "/mock:gandalf:/tmp/tramp-testkLeKOx/foo\0bar")
      :value nil))
   FAILED  1/1  tramp-test41-special-characters (0.484141 sec)

--8<---------------cut here---------------end--------------->8---

Well, perhaps Tramp can be improved to handle null bytes on (remote)
shell level, but do we need this?

Best regards, Michael.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49723; Package emacs. (Thu, 16 Sep 2021 18:52:02 GMT) Full text and rfc822 format available.

Message #35 received at 49723 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Federico Tedin <federicotedin <at> gmail.com>
Cc: phst <at> google.com, Eli Zaretskii <eliz <at> gnu.org>, 49723 <at> debbugs.gnu.org
Subject: Re: bug#49723: 28.0.50; Test in coding.c for NUL bytes in filenames
 is not reliable
Date: Thu, 16 Sep 2021 20:51:47 +0200
Federico Tedin <federicotedin <at> gmail.com> writes:

Hi Federico,

> Would this imply only doing it in `tramp-handle-expand-file-name`, or
> other functions as well?
>
> I had thought of doing it for at least `tramp-handle-expand-file-name`,
> but then I noticed that the function eventually calls `expand-file-name`
> with a combination of name + dir, or only with name. So in that case I
> figured that the null bytes check would be performed by
> `expand-file-name` anyways.

I haven't checked yet what's needed. However, there are several
incarnations of `expand-file-name' implementation in Tramp, namely

--8<---------------cut here---------------start------------->8---
grep --color=auto -nH --null -E 'defun.+handle-expand-file-name' *.el
tramp.el3393:(defun tramp-handle-expand-file-name (name &optional dir)
tramp-gvfs.el1137:(defun tramp-gvfs-handle-expand-file-name (name &optional dir)
tramp-sh.el2683:(defun tramp-sh-handle-expand-file-name (name &optional dir)
tramp-smb.el736:(defun tramp-smb-handle-expand-file-name (name &optional dir)
tramp-sudoedit.el346:(defun tramp-sudoedit-handle-expand-file-name (name &optional dir)
--8<---------------cut here---------------end--------------->8---

Best regards, Michael.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49723; Package emacs. (Thu, 16 Sep 2021 19:09:01 GMT) Full text and rfc822 format available.

Message #38 received at 49723 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: phst <at> google.com, 49723 <at> debbugs.gnu.org, federicotedin <at> gmail.com
Subject: Re: bug#49723: 28.0.50; Test in coding.c for NUL bytes in filenames
 is not reliable
Date: Thu, 16 Sep 2021 22:07:56 +0300
> From: Michael Albinus <michael.albinus <at> gmx.de>
> Cc: federicotedin <at> gmail.com,  phst <at> google.com,  49723 <at> debbugs.gnu.org
> Date: Thu, 16 Sep 2021 20:44:32 +0200
> 
> >> Should this be implemented also in remote file names?
> >
> > Are we sure remote file names cannot include null bytes?
> 
> Likely not. I have added "foo\0bar" as file name in tramp-test.el, and
> then I get
> 
> --8<---------------cut here---------------start------------->8---
> Test tramp-test41-special-characters condition:
>     (ert-test-failed
>      ((should
>        (file-exists-p file1))
>       :form
>       (file-exists-p "/mock:gandalf:/tmp/tramp-testkLeKOx/foo\0bar")
>       :value nil))
>    FAILED  1/1  tramp-test41-special-characters (0.484141 sec)
> 
> --8<---------------cut here---------------end--------------->8---
> 
> Well, perhaps Tramp can be improved to handle null bytes on (remote)
> shell level, but do we need this?

I'm not sure.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49723; Package emacs. (Thu, 16 Sep 2021 19:14:01 GMT) Full text and rfc822 format available.

Message #41 received at 49723 <at> debbugs.gnu.org (full text, mbox):

From: Federico Tedin <federicotedin <at> gmail.com>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: phst <at> google.com, Eli Zaretskii <eliz <at> gnu.org>, 49723 <at> debbugs.gnu.org
Subject: Re: bug#49723: 28.0.50; Test in coding.c for NUL bytes in filenames
 is not reliable
Date: Thu, 16 Sep 2021 21:13:04 +0200
Michael Albinus <michael.albinus <at> gmx.de> writes:

> I haven't checked yet what's needed. However, there are several
> incarnations of `expand-file-name' implementation in Tramp, namely
>
> grep --color=auto -nH --null -E 'defun.+handle-expand-file-name' *.el
> tramp.el3393:(defun tramp-handle-expand-file-name (name &optional dir)
> tramp-gvfs.el1137:(defun tramp-gvfs-handle-expand-file-name (name &optional dir)
> tramp-sh.el2683:(defun tramp-sh-handle-expand-file-name (name &optional dir)
> tramp-smb.el736:(defun tramp-smb-handle-expand-file-name (name &optional dir)
> tramp-sudoedit.el346:(defun tramp-sudoedit-handle-expand-file-name (name &optional dir)
>
> Best regards, Michael.

Ouch, hadn't noticed those. I gave a look at the four I hadn't seen (gvfs,
sh, smb and sudoedit) and it seems like all of them return expressions
in the shape of:

  (expand-file-name xyz)

or:

  (tramp-run-real-handler #'expand-file-name xyz)

with xyz being a value derived from NAME or NAME and DIR. So in that
case it seems like we would be covered by the changes in `expand-file-mode`.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49723; Package emacs. (Fri, 17 Sep 2021 07:51:01 GMT) Full text and rfc822 format available.

Message #44 received at 49723 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Federico Tedin <federicotedin <at> gmail.com>
Cc: phst <at> google.com, 49723 <at> debbugs.gnu.org
Subject: Re: bug#49723: 28.0.50; Test in coding.c for NUL bytes in filenames
 is not reliable
Date: Fri, 17 Sep 2021 10:49:59 +0300
> From: Federico Tedin <federicotedin <at> gmail.com>
> Cc: phst <at> google.com,  49723 <at> debbugs.gnu.org
> Date: Thu, 16 Sep 2021 18:58:02 +0200
> 
> >> +** 'expand-file-name' now checks for null bytes in filenames.
> >> +The function will now check for null bytes in both NAME and
> >> +DEFAULT-DIRECTORY arguments, as well as in the 'default-directory'
> >> +buffer-local variable, assuming its value is used.
> >
> > This should say that if null bytes are found, expand-file-name will
> > signal an error.
> 
> Makes sense! I'm attaching a revised version.

Thanks.  Did you run the test suite after applying the changes, and
did you see no regressions?  If you didn't yet run the test suite,
please be sure to run all of it, as the use of expand-file-name is
universal.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49723; Package emacs. (Fri, 17 Sep 2021 19:01:02 GMT) Full text and rfc822 format available.

Message #47 received at 49723 <at> debbugs.gnu.org (full text, mbox):

From: Federico Tedin <federicotedin <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: phst <at> google.com, 49723 <at> debbugs.gnu.org
Subject: Re: bug#49723: 28.0.50; Test in coding.c for NUL bytes in filenames
 is not reliable
Date: Fri, 17 Sep 2021 21:00:08 +0200
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes:

> Thanks.  Did you run the test suite after applying the changes, and
> did you see no regressions?  If you didn't yet run the test suite,
> please be sure to run all of it, as the use of expand-file-name is
> universal.

I hadn't, so I checked out master aa59d38c59 and applied my patch on top
of it. I then ran "make check" and waited for a bit. There appears to be
only two tests failing in test/lisp/time-stamp-tests.el
('time-stamp-format-day-of-week' and
'time-stamp-format-string-width'). Both seem to be unrelated to my
change; maybe it's my system's strange combination of
Spanish/English/German locale-related configurations (I'm attaching the
log just in case). All other test files were run without problems.

[time-stamp-tests.log (text/plain, attachment)]

Reply sent to Eli Zaretskii <eliz <at> gnu.org>:
You have taken responsibility. (Sat, 18 Sep 2021 06:52:02 GMT) Full text and rfc822 format available.

Notification sent to Eli Zaretskii <eliz <at> gnu.org>:
bug acknowledged by developer. (Sat, 18 Sep 2021 06:52:02 GMT) Full text and rfc822 format available.

Message #52 received at 49723-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Federico Tedin <federicotedin <at> gmail.com>
Cc: phst <at> google.com, 49723-done <at> debbugs.gnu.org
Subject: Re: bug#49723: 28.0.50; Test in coding.c for NUL bytes in filenames
 is not reliable
Date: Sat, 18 Sep 2021 09:51:33 +0300
> From: Federico Tedin <federicotedin <at> gmail.com>
> Cc: phst <at> google.com,  49723 <at> debbugs.gnu.org
> Date: Fri, 17 Sep 2021 21:00:08 +0200
> 
> > Thanks.  Did you run the test suite after applying the changes, and
> > did you see no regressions?  If you didn't yet run the test suite,
> > please be sure to run all of it, as the use of expand-file-name is
> > universal.
> 
> I hadn't, so I checked out master aa59d38c59 and applied my patch on top
> of it. I then ran "make check" and waited for a bit. There appears to be
> only two tests failing in test/lisp/time-stamp-tests.el
> ('time-stamp-format-day-of-week' and
> 'time-stamp-format-string-width'). Both seem to be unrelated to my
> change; maybe it's my system's strange combination of
> Spanish/English/German locale-related configurations (I'm attaching the
> log just in case). All other test files were run without problems.

Thanks.  I installed your changes, and I'm therefore closing this bug.

A few minor stylistic comments, for the future

 . the lines in the commit log message are too wide, they should be at
   most 66 characters, because we produce ChangeLog files from Girt
   logs, and ChangeLog files have the fill-column set to 74, which
   includes 9-column TAB (perhaps this means the fill-column setting
   in .dire-locals.el should be amended?)
 . please quote symbols in commit log messages rather than leaving
   them unquoted (this doesn't apply to symbols in parentheses that
   state the functions which were changed)
 . please try to establish whether the changes need to be described in
   the manual(s), and mark the NEWS entries accordingly (if you decide
   there's a need to describe in the manual, please also include a
   suitable change for that)

Thanks again for working on this.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49723; Package emacs. (Sat, 18 Sep 2021 17:59:01 GMT) Full text and rfc822 format available.

Message #55 received at 49723-done <at> debbugs.gnu.org (full text, mbox):

From: Federico Tedin <federicotedin <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: phst <at> google.com, 49723-done <at> debbugs.gnu.org
Subject: Re: bug#49723: 28.0.50; Test in coding.c for NUL bytes in filenames
 is not reliable
Date: Sat, 18 Sep 2021 19:57:56 +0200
[Message part 1 (text/plain, inline)]
Noted! Thank you.

Eli Zaretskii <eliz <at> gnu.org> schrieb am Sa. 18. Sept. 2021 um 08:51:

> > From: Federico Tedin <federicotedin <at> gmail.com>
> > Cc: phst <at> google.com,  49723 <at> debbugs.gnu.org
> > Date: Fri, 17 Sep 2021 21:00:08 +0200
> >
> > > Thanks.  Did you run the test suite after applying the changes, and
> > > did you see no regressions?  If you didn't yet run the test suite,
> > > please be sure to run all of it, as the use of expand-file-name is
> > > universal.
> >
> > I hadn't, so I checked out master aa59d38c59 and applied my patch on top
> > of it. I then ran "make check" and waited for a bit. There appears to be
> > only two tests failing in test/lisp/time-stamp-tests.el
> > ('time-stamp-format-day-of-week' and
> > 'time-stamp-format-string-width'). Both seem to be unrelated to my
> > change; maybe it's my system's strange combination of
> > Spanish/English/German locale-related configurations (I'm attaching the
> > log just in case). All other test files were run without problems.
>
> Thanks.  I installed your changes, and I'm therefore closing this bug.
>
> A few minor stylistic comments, for the future
>
>  . the lines in the commit log message are too wide, they should be at
>    most 66 characters, because we produce ChangeLog files from Girt
>    logs, and ChangeLog files have the fill-column set to 74, which
>    includes 9-column TAB (perhaps this means the fill-column setting
>    in .dire-locals.el should be amended?)
>  . please quote symbols in commit log messages rather than leaving
>    them unquoted (this doesn't apply to symbols in parentheses that
>    state the functions which were changed)
>  . please try to establish whether the changes need to be described in
>    the manual(s), and mark the NEWS entries accordingly (if you decide
>    there's a need to describe in the manual, please also include a
>    suitable change for that)
>
> Thanks again for working on this.
>
[Message part 2 (text/html, inline)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49723; Package emacs. (Mon, 20 Sep 2021 12:00:01 GMT) Full text and rfc822 format available.

Message #58 received at 49723 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Federico Tedin <federicotedin <at> gmail.com>
Cc: phst <at> google.com, Eli Zaretskii <eliz <at> gnu.org>, 49723 <at> debbugs.gnu.org
Subject: Re: bug#49723: 28.0.50; Test in coding.c for NUL bytes in filenames
 is not reliable
Date: Mon, 20 Sep 2021 13:59:36 +0200
Federico Tedin <federicotedin <at> gmail.com> writes:

Hi Federico,

> Ouch, hadn't noticed those. I gave a look at the four I hadn't seen (gvfs,
> sh, smb and sudoedit) and it seems like all of them return expressions
> in the shape of:
>
>   (expand-file-name xyz)
>
> or:
>
>   (tramp-run-real-handler #'expand-file-name xyz)
>
> with xyz being a value derived from NAME or NAME and DIR. So in that
> case it seems like we would be covered by the changes in `expand-file-mode`.

Indeed. So there's no need for Tramp to do anything special.

Best regards, Michael.




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Tue, 19 Oct 2021 11:24:06 GMT) Full text and rfc822 format available.

This bug report was last modified 2 years and 204 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.