* bug#49723: 28.0.50; Test in coding.c for NUL bytes in filenames is not reliable
@ 2021-07-24 17:39 Eli Zaretskii
2021-09-14 19:01 ` Federico Tedin
0 siblings, 1 reply; 18+ messages in thread
From: Eli Zaretskii @ 2021-07-24 17:39 UTC (permalink / raw)
To: 49723; +Cc: Philipp Stephani
From: eliz@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))
^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#49723: 28.0.50; Test in coding.c for NUL bytes in filenames is not reliable
2021-07-24 17:39 bug#49723: 28.0.50; Test in coding.c for NUL bytes in filenames is not reliable Eli Zaretskii
@ 2021-09-14 19:01 ` Federico Tedin
2021-09-14 19:24 ` Eli Zaretskii
0 siblings, 1 reply; 18+ messages in thread
From: Federico Tedin @ 2021-09-14 19:01 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: Philipp Stephani, 49723
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@gnu.org> writes:
> From: eliz@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))
^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#49723: 28.0.50; Test in coding.c for NUL bytes in filenames is not reliable
2021-09-14 19:01 ` Federico Tedin
@ 2021-09-14 19:24 ` Eli Zaretskii
2021-09-14 22:17 ` Federico Tedin
0 siblings, 1 reply; 18+ messages in thread
From: Eli Zaretskii @ 2021-09-14 19:24 UTC (permalink / raw)
To: Federico Tedin; +Cc: phst, 49723
> From: Federico Tedin <federicotedin@gmail.com>
> Cc: 49723@debbugs.gnu.org, Philipp Stephani <phst@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.
^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#49723: 28.0.50; Test in coding.c for NUL bytes in filenames is not reliable
2021-09-14 19:24 ` Eli Zaretskii
@ 2021-09-14 22:17 ` Federico Tedin
2021-09-16 12:25 ` Eli Zaretskii
0 siblings, 1 reply; 18+ messages in thread
From: Federico Tedin @ 2021-09-14 22:17 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: phst, 49723
[-- Attachment #1: Type: text/plain, Size: 254 bytes --]
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.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 4251 bytes --]
From 0f960aeab1c4b3182d597ac0ce80526f09e97452 Mon Sep 17 00:00:00 2001
From: Federico Tedin <federicotedin@gmail.com>
Date: Wed, 15 Sep 2021 00:15:16 +0200
Subject: [PATCH] Add null byte checks to filenames in expand-file-name
(bug#49723)
* src/fileio.c (expand-file-name): Check for null bytes for both NAME
and DEFAULT-DIRECTORY arguments. Also check for null bytes in
buffer-local default-directory, assuming it is used.
* src/coding.c (encode_file_name): Use CHECK_STRING_NULL_BYTES.
* src/lisp.h (CHECK_STRING_NULL_BYTES): Add function for checking for
null bytes in Lisp strings.
* test/src/fileio-tests.el (fileio-test--expand-file-name-null-bytes):
Add test for new changes to expand-file-name.
* etc/NEWS: Announce changes.
---
etc/NEWS | 6 ++++++
src/coding.c | 3 +--
src/fileio.c | 6 +++++-
src/lisp.h | 7 +++++++
test/src/fileio-tests.el | 9 +++++++++
5 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/etc/NEWS b/etc/NEWS
index 5809716868..8d96ceff79 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -278,6 +278,12 @@ personalize the uniquified buffer name.
---
** 'remove-hook' is now an interactive command.
+** '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.
+
+---
** Frames
+++
diff --git a/src/coding.c b/src/coding.c
index d027c7d539..7030a53869 100644
--- a/src/coding.c
+++ b/src/coding.c
@@ -10430,8 +10430,7 @@ encode_file_name (Lisp_Object fname)
cause subtle bugs because the system would silently use a
different filename than expected. Perform this check after
encoding to not miss NUL bytes introduced through encoding. */
- CHECK_TYPE (memchr (SSDATA (encoded), '\0', SBYTES (encoded)) == NULL,
- Qfilenamep, fname);
+ CHECK_STRING_NULL_BYTES (encoded);
return encoded;
}
diff --git a/src/fileio.c b/src/fileio.c
index 0db8ed793b..3c13d3fe41 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -945,6 +945,7 @@ DEFUN ("expand-file-name", Fexpand_file_name, Sexpand_file_name, 1, 2, 0,
USE_SAFE_ALLOCA;
CHECK_STRING (name);
+ CHECK_STRING_NULL_BYTES (name);
/* If the file name has special constructs in it,
call the corresponding file name handler. */
@@ -993,7 +994,10 @@ DEFUN ("expand-file-name", Fexpand_file_name, Sexpand_file_name, 1, 2, 0,
if (STRINGP (dir))
{
if (file_name_absolute_no_tilde_p (dir))
- default_directory = dir;
+ {
+ CHECK_STRING_NULL_BYTES (dir);
+ default_directory = dir;
+ }
else
{
Lisp_Object absdir
diff --git a/src/lisp.h b/src/lisp.h
index 7bfc69b647..9716b34bae 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -1615,6 +1615,13 @@ STRING_SET_CHARS (Lisp_Object string, ptrdiff_t newsize)
XSTRING (string)->u.s.size = newsize;
}
+INLINE void
+CHECK_STRING_NULL_BYTES (Lisp_Object string)
+{
+ CHECK_TYPE (memchr (SSDATA (string), '\0', SBYTES (string)) == NULL,
+ Qfilenamep, string);
+}
+
/* A regular vector is just a header plus an array of Lisp_Objects. */
struct Lisp_Vector
diff --git a/test/src/fileio-tests.el b/test/src/fileio-tests.el
index f4d123b426..438ebebb76 100644
--- a/test/src/fileio-tests.el
+++ b/test/src/fileio-tests.el
@@ -136,6 +136,15 @@ fileio-tests--relative-default-directory
(should (and (file-name-absolute-p name)
(not (eq (aref name 0) ?~))))))
+(ert-deftest fileio-test--expand-file-name-null-bytes ()
+ "Test that expand-file-name checks for null bytes in filenames."
+ (should-error (expand-file-name (concat "file" (char-to-string ?\0) ".txt"))
+ :type 'wrong-type-argument)
+ (should-error (expand-file-name "file.txt" (concat "dir" (char-to-string ?\0)))
+ :type 'wrong-type-argument)
+ (let ((default-directory (concat "dir" (char-to-string ?\0))))
+ (should-error (expand-file-name "file.txt") :type 'wrong-type-argument)))
+
(ert-deftest fileio-tests--file-name-absolute-p ()
"Test file-name-absolute-p."
(dolist (suffix '("" "/" "//" "/foo" "/foo/" "/foo//" "/foo/bar"))
--
2.25.1
[-- Attachment #3: Type: text/plain, Size: 986 bytes --]
Eli Zaretskii <eliz@gnu.org> writes:
>> From: Federico Tedin <federicotedin@gmail.com>
>> Cc: 49723@debbugs.gnu.org, Philipp Stephani <phst@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.
^ permalink raw reply related [flat|nested] 18+ messages in thread
* bug#49723: 28.0.50; Test in coding.c for NUL bytes in filenames is not reliable
2021-09-14 22:17 ` Federico Tedin
@ 2021-09-16 12:25 ` Eli Zaretskii
2021-09-16 16:58 ` Federico Tedin
0 siblings, 1 reply; 18+ messages in thread
From: Eli Zaretskii @ 2021-09-16 12:25 UTC (permalink / raw)
To: Federico Tedin; +Cc: phst, 49723
> From: Federico Tedin <federicotedin@gmail.com>
> Cc: phst@google.com, 49723@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.
^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#49723: 28.0.50; Test in coding.c for NUL bytes in filenames is not reliable
2021-09-16 12:25 ` Eli Zaretskii
@ 2021-09-16 16:58 ` Federico Tedin
2021-09-16 18:12 ` Michael Albinus
2021-09-17 7:49 ` Eli Zaretskii
0 siblings, 2 replies; 18+ messages in thread
From: Federico Tedin @ 2021-09-16 16:58 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: phst, 49723
[-- Attachment #1: Type: text/plain, Size: 433 bytes --]
Eli Zaretskii <eliz@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.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 4320 bytes --]
From 6faba0f1107290d44a83d396a61747dea67f0f39 Mon Sep 17 00:00:00 2001
From: Federico Tedin <federicotedin@gmail.com>
Date: Wed, 15 Sep 2021 00:15:16 +0200
Subject: [PATCH] Add null byte checks to filenames in expand-file-name
(bug#49723)
* src/fileio.c (expand-file-name): Check for null bytes for both NAME
and DEFAULT-DIRECTORY arguments. Also check for null bytes in
buffer-local default-directory, assuming it is used.
* src/coding.c (encode_file_name): Use CHECK_STRING_NULL_BYTES.
* src/lisp.h (CHECK_STRING_NULL_BYTES): Add function for checking for
null bytes in Lisp strings.
* test/src/fileio-tests.el (fileio-test--expand-file-name-null-bytes):
Add test for new changes to expand-file-name.
* etc/NEWS: Announce changes.
---
etc/NEWS | 7 +++++++
src/coding.c | 3 +--
src/fileio.c | 6 +++++-
src/lisp.h | 7 +++++++
test/src/fileio-tests.el | 9 +++++++++
5 files changed, 29 insertions(+), 3 deletions(-)
diff --git a/etc/NEWS b/etc/NEWS
index 17c42ce104..3484ef3f3f 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -281,6 +281,13 @@ personalize the uniquified buffer name.
---
** 'remove-hook' is now an interactive command.
+** '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.
+
+---
** Frames
+++
diff --git a/src/coding.c b/src/coding.c
index d027c7d539..7030a53869 100644
--- a/src/coding.c
+++ b/src/coding.c
@@ -10430,8 +10430,7 @@ encode_file_name (Lisp_Object fname)
cause subtle bugs because the system would silently use a
different filename than expected. Perform this check after
encoding to not miss NUL bytes introduced through encoding. */
- CHECK_TYPE (memchr (SSDATA (encoded), '\0', SBYTES (encoded)) == NULL,
- Qfilenamep, fname);
+ CHECK_STRING_NULL_BYTES (encoded);
return encoded;
}
diff --git a/src/fileio.c b/src/fileio.c
index 0db8ed793b..3c13d3fe41 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -945,6 +945,7 @@ DEFUN ("expand-file-name", Fexpand_file_name, Sexpand_file_name, 1, 2, 0,
USE_SAFE_ALLOCA;
CHECK_STRING (name);
+ CHECK_STRING_NULL_BYTES (name);
/* If the file name has special constructs in it,
call the corresponding file name handler. */
@@ -993,7 +994,10 @@ DEFUN ("expand-file-name", Fexpand_file_name, Sexpand_file_name, 1, 2, 0,
if (STRINGP (dir))
{
if (file_name_absolute_no_tilde_p (dir))
- default_directory = dir;
+ {
+ CHECK_STRING_NULL_BYTES (dir);
+ default_directory = dir;
+ }
else
{
Lisp_Object absdir
diff --git a/src/lisp.h b/src/lisp.h
index 7bfc69b647..9716b34bae 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -1615,6 +1615,13 @@ STRING_SET_CHARS (Lisp_Object string, ptrdiff_t newsize)
XSTRING (string)->u.s.size = newsize;
}
+INLINE void
+CHECK_STRING_NULL_BYTES (Lisp_Object string)
+{
+ CHECK_TYPE (memchr (SSDATA (string), '\0', SBYTES (string)) == NULL,
+ Qfilenamep, string);
+}
+
/* A regular vector is just a header plus an array of Lisp_Objects. */
struct Lisp_Vector
diff --git a/test/src/fileio-tests.el b/test/src/fileio-tests.el
index f4d123b426..438ebebb76 100644
--- a/test/src/fileio-tests.el
+++ b/test/src/fileio-tests.el
@@ -136,6 +136,15 @@ fileio-tests--relative-default-directory
(should (and (file-name-absolute-p name)
(not (eq (aref name 0) ?~))))))
+(ert-deftest fileio-test--expand-file-name-null-bytes ()
+ "Test that expand-file-name checks for null bytes in filenames."
+ (should-error (expand-file-name (concat "file" (char-to-string ?\0) ".txt"))
+ :type 'wrong-type-argument)
+ (should-error (expand-file-name "file.txt" (concat "dir" (char-to-string ?\0)))
+ :type 'wrong-type-argument)
+ (let ((default-directory (concat "dir" (char-to-string ?\0))))
+ (should-error (expand-file-name "file.txt") :type 'wrong-type-argument)))
+
(ert-deftest fileio-tests--file-name-absolute-p ()
"Test file-name-absolute-p."
(dolist (suffix '("" "/" "//" "/foo" "/foo/" "/foo//" "/foo/bar"))
--
2.25.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* bug#49723: 28.0.50; Test in coding.c for NUL bytes in filenames is not reliable
2021-09-16 16:58 ` Federico Tedin
@ 2021-09-16 18:12 ` Michael Albinus
2021-09-16 18:30 ` Eli Zaretskii
2021-09-16 18:38 ` Federico Tedin
2021-09-17 7:49 ` Eli Zaretskii
1 sibling, 2 replies; 18+ messages in thread
From: Michael Albinus @ 2021-09-16 18:12 UTC (permalink / raw)
To: Federico Tedin; +Cc: phst, 49723
Federico Tedin <federicotedin@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.
^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#49723: 28.0.50; Test in coding.c for NUL bytes in filenames is not reliable
2021-09-16 18:12 ` Michael Albinus
@ 2021-09-16 18:30 ` Eli Zaretskii
2021-09-16 18:44 ` Michael Albinus
2021-09-16 18:38 ` Federico Tedin
1 sibling, 1 reply; 18+ messages in thread
From: Eli Zaretskii @ 2021-09-16 18:30 UTC (permalink / raw)
To: Michael Albinus; +Cc: phst, 49723, federicotedin
> From: Michael Albinus <michael.albinus@gmx.de>
> Cc: Eli Zaretskii <eliz@gnu.org>, phst@google.com, 49723@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?
^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#49723: 28.0.50; Test in coding.c for NUL bytes in filenames is not reliable
2021-09-16 18:12 ` Michael Albinus
2021-09-16 18:30 ` Eli Zaretskii
@ 2021-09-16 18:38 ` Federico Tedin
2021-09-16 18:51 ` Michael Albinus
1 sibling, 1 reply; 18+ messages in thread
From: Federico Tedin @ 2021-09-16 18:38 UTC (permalink / raw)
To: Michael Albinus; +Cc: phst, 49723
Michael Albinus <michael.albinus@gmx.de> writes:
> Federico Tedin <federicotedin@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.
^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#49723: 28.0.50; Test in coding.c for NUL bytes in filenames is not reliable
2021-09-16 18:30 ` Eli Zaretskii
@ 2021-09-16 18:44 ` Michael Albinus
2021-09-16 19:07 ` Eli Zaretskii
0 siblings, 1 reply; 18+ messages in thread
From: Michael Albinus @ 2021-09-16 18:44 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: phst, 49723, federicotedin
Eli Zaretskii <eliz@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.
^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#49723: 28.0.50; Test in coding.c for NUL bytes in filenames is not reliable
2021-09-16 18:38 ` Federico Tedin
@ 2021-09-16 18:51 ` Michael Albinus
2021-09-16 19:13 ` Federico Tedin
0 siblings, 1 reply; 18+ messages in thread
From: Michael Albinus @ 2021-09-16 18:51 UTC (permalink / raw)
To: Federico Tedin; +Cc: phst, 49723
Federico Tedin <federicotedin@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.
^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#49723: 28.0.50; Test in coding.c for NUL bytes in filenames is not reliable
2021-09-16 18:44 ` Michael Albinus
@ 2021-09-16 19:07 ` Eli Zaretskii
0 siblings, 0 replies; 18+ messages in thread
From: Eli Zaretskii @ 2021-09-16 19:07 UTC (permalink / raw)
To: Michael Albinus; +Cc: phst, 49723, federicotedin
> From: Michael Albinus <michael.albinus@gmx.de>
> Cc: federicotedin@gmail.com, phst@google.com, 49723@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.
^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#49723: 28.0.50; Test in coding.c for NUL bytes in filenames is not reliable
2021-09-16 18:51 ` Michael Albinus
@ 2021-09-16 19:13 ` Federico Tedin
2021-09-20 11:59 ` Michael Albinus
0 siblings, 1 reply; 18+ messages in thread
From: Federico Tedin @ 2021-09-16 19:13 UTC (permalink / raw)
To: Michael Albinus; +Cc: phst, 49723
Michael Albinus <michael.albinus@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`.
^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#49723: 28.0.50; Test in coding.c for NUL bytes in filenames is not reliable
2021-09-16 16:58 ` Federico Tedin
2021-09-16 18:12 ` Michael Albinus
@ 2021-09-17 7:49 ` Eli Zaretskii
2021-09-17 19:00 ` Federico Tedin
1 sibling, 1 reply; 18+ messages in thread
From: Eli Zaretskii @ 2021-09-17 7:49 UTC (permalink / raw)
To: Federico Tedin; +Cc: phst, 49723
> From: Federico Tedin <federicotedin@gmail.com>
> Cc: phst@google.com, 49723@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.
^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#49723: 28.0.50; Test in coding.c for NUL bytes in filenames is not reliable
2021-09-17 7:49 ` Eli Zaretskii
@ 2021-09-17 19:00 ` Federico Tedin
2021-09-18 6:51 ` Eli Zaretskii
0 siblings, 1 reply; 18+ messages in thread
From: Federico Tedin @ 2021-09-17 19:00 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: phst, 49723
[-- Attachment #1: Type: text/plain, Size: 760 bytes --]
Eli Zaretskii <eliz@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.
[-- Attachment #2: log --]
[-- Type: text/plain, Size: 10534 bytes --]
Running 135 tests (2021-09-17 20:46:15+0200, selector `(not (or (tag :expensive-test) (tag :unstable) (tag :nativecomp)))')
passed 1/135 formatz-%-10z-hhmm (0.000985 sec)
passed 2/135 formatz-%-10z-seconds (0.001173 sec)
passed 3/135 formatz-%-10z-threedigit (0.000526 sec)
passed 4/135 formatz-%-12z-hhmm (0.000991 sec)
passed 5/135 formatz-%-12z-seconds (0.001199 sec)
passed 6/135 formatz-%-12z-threedigit (0.000558 sec)
passed 7/135 formatz-%-3z-hhmm (0.000924 sec)
passed 8/135 formatz-%-3z-seconds (0.001093 sec)
passed 9/135 formatz-%-3z-threedigit (0.000506 sec)
passed 10/135 formatz-%-z-hhmm (0.000854 sec)
passed 11/135 formatz-%-z-seconds (0.001047 sec)
passed 12/135 formatz-%-z-threedigit (0.000491 sec)
passed 13/135 formatz-%012:::z-hhmm (0.001046 sec)
passed 14/135 formatz-%012:::z-seconds (0.001133 sec)
passed 15/135 formatz-%012:::z-threedigit (0.000583 sec)
passed 16/135 formatz-%012::z-hhmm (0.001066 sec)
passed 17/135 formatz-%012::z-seconds (0.001096 sec)
passed 18/135 formatz-%012::z-threedigit (0.000584 sec)
passed 19/135 formatz-%012:z-hhmm (0.001047 sec)
passed 20/135 formatz-%012:z-seconds (0.001066 sec)
passed 21/135 formatz-%012:z-threedigit (0.000558 sec)
passed 22/135 formatz-%012z-hhmm (0.001053 sec)
passed 23/135 formatz-%012z-seconds (0.001166 sec)
passed 24/135 formatz-%012z-threedigit (0.095949 sec)
passed 25/135 formatz-%03:::z-hhmm (0.000898 sec)
passed 26/135 formatz-%03:::z-seconds (0.000912 sec)
passed 27/135 formatz-%03:::z-threedigit (0.000488 sec)
passed 28/135 formatz-%05z-hhmm (0.000870 sec)
passed 29/135 formatz-%05z-seconds (0.001055 sec)
passed 30/135 formatz-%05z-threedigit (0.000480 sec)
passed 31/135 formatz-%06:::z-hhmm (0.000852 sec)
passed 32/135 formatz-%06:::z-seconds (0.000949 sec)
passed 33/135 formatz-%06:::z-threedigit (0.000495 sec)
passed 34/135 formatz-%06:z-hhmm (0.000839 sec)
passed 35/135 formatz-%06:z-seconds (0.000926 sec)
passed 36/135 formatz-%06:z-threedigit (0.000489 sec)
passed 37/135 formatz-%06z-hhmm (0.000954 sec)
passed 38/135 formatz-%06z-seconds (0.001083 sec)
passed 39/135 formatz-%06z-threedigit (0.000492 sec)
passed 40/135 formatz-%07:::z-hhmm (0.000957 sec)
passed 41/135 formatz-%07:::z-seconds (0.001001 sec)
passed 42/135 formatz-%07:::z-threedigit (0.000532 sec)
passed 43/135 formatz-%07:z-hhmm (0.000938 sec)
passed 44/135 formatz-%07:z-seconds (0.000979 sec)
passed 45/135 formatz-%07:z-threedigit (0.000506 sec)
passed 46/135 formatz-%09::z-hhmm (0.000963 sec)
passed 47/135 formatz-%09::z-seconds (0.000970 sec)
passed 48/135 formatz-%09::z-threedigit (0.000526 sec)
passed 49/135 formatz-%0:::z-hhmm (0.000857 sec)
passed 50/135 formatz-%0:::z-seconds (0.000980 sec)
passed 51/135 formatz-%0:::z-threedigit (0.000534 sec)
passed 52/135 formatz-%0::z-hhmm (0.000964 sec)
passed 53/135 formatz-%0::z-seconds (0.001002 sec)
passed 54/135 formatz-%0::z-threedigit (0.000543 sec)
passed 55/135 formatz-%0:z-hhmm (0.000887 sec)
passed 56/135 formatz-%0:z-seconds (0.000970 sec)
passed 57/135 formatz-%0:z-threedigit (0.094870 sec)
passed 58/135 formatz-%0z-hhmm (0.000828 sec)
passed 59/135 formatz-%0z-seconds (0.000983 sec)
passed 60/135 formatz-%0z-threedigit (0.000470 sec)
passed 61/135 formatz-%10:::z-hhmm (0.001001 sec)
passed 62/135 formatz-%10:::z-seconds (0.001000 sec)
passed 63/135 formatz-%10:::z-threedigit (0.000493 sec)
passed 64/135 formatz-%12:::z-hhmm (0.000864 sec)
passed 65/135 formatz-%12:::z-seconds (0.001001 sec)
passed 66/135 formatz-%12:::z-threedigit (0.000515 sec)
passed 67/135 formatz-%12::z-hhmm (0.000959 sec)
passed 68/135 formatz-%12::z-seconds (0.000998 sec)
passed 69/135 formatz-%12::z-threedigit (0.000530 sec)
passed 70/135 formatz-%12:z-hhmm (0.000921 sec)
passed 71/135 formatz-%12:z-seconds (0.001011 sec)
passed 72/135 formatz-%12:z-threedigit (0.000559 sec)
passed 73/135 formatz-%12z-hhmm (0.000956 sec)
passed 74/135 formatz-%12z-seconds (0.001112 sec)
passed 75/135 formatz-%12z-threedigit (0.000514 sec)
passed 76/135 formatz-%3:::z-hhmm (0.000850 sec)
passed 77/135 formatz-%3:::z-seconds (0.000958 sec)
passed 78/135 formatz-%3:::z-threedigit (0.000514 sec)
passed 79/135 formatz-%5z-hhmm (0.000912 sec)
passed 80/135 formatz-%5z-seconds (0.001076 sec)
passed 81/135 formatz-%5z-threedigit (0.000507 sec)
passed 82/135 formatz-%6:z-hhmm (0.000887 sec)
passed 83/135 formatz-%6:z-seconds (0.000965 sec)
passed 84/135 formatz-%6:z-threedigit (0.000530 sec)
passed 85/135 formatz-%9::z-hhmm (0.000967 sec)
passed 86/135 formatz-%9::z-seconds (0.001021 sec)
passed 87/135 formatz-%9::z-threedigit (0.000549 sec)
passed 88/135 formatz-%:::z-hhmm (0.096159 sec)
passed 89/135 formatz-%:::z-seconds (0.000838 sec)
passed 90/135 formatz-%:::z-threedigit (0.000442 sec)
passed 91/135 formatz-%::z-hhmm (0.000822 sec)
passed 92/135 formatz-%::z-seconds (0.000866 sec)
passed 93/135 formatz-%::z-threedigit (0.000466 sec)
passed 94/135 formatz-%:z-hhmm (0.000757 sec)
passed 95/135 formatz-%:z-seconds (0.000848 sec)
passed 96/135 formatz-%:z-threedigit (0.000466 sec)
passed 97/135 formatz-%_12z-hhmm (0.001074 sec)
passed 98/135 formatz-%_12z-seconds (0.001125 sec)
passed 99/135 formatz-%_12z-threedigit (0.000527 sec)
passed 100/135 formatz-%_7z-hhmm (0.000929 sec)
passed 101/135 formatz-%_7z-seconds (0.001055 sec)
passed 102/135 formatz-%_7z-threedigit (0.000501 sec)
passed 103/135 formatz-%_z-hhmm (0.000903 sec)
passed 104/135 formatz-%_z-seconds (0.001031 sec)
passed 105/135 formatz-%_z-threedigit (0.000492 sec)
passed 106/135 formatz-%z-spotcheck (0.000326 sec)
passed 107/135 formatz-illegal-options (0.000445 sec)
passed 108/135 time-stamp-custom-count (0.000538 sec)
passed 109/135 time-stamp-custom-end (0.000311 sec)
passed 110/135 time-stamp-custom-format-tabs-expand (0.000654 sec)
passed 111/135 time-stamp-custom-inserts-lines (0.000468 sec)
passed 112/135 time-stamp-custom-pattern (0.005087 sec)
passed 113/135 time-stamp-custom-time-zone (0.000375 sec)
passed 114/135 time-stamp-format-am-pm (0.000284 sec)
passed 115/135 time-stamp-format-day-number-in-week (0.000231 sec)
passed 116/135 time-stamp-format-day-of-month (0.000470 sec)
Test time-stamp-format-day-of-week backtrace:
signal(ert-test-failed (((should (equal (time-stamp-string "%3a" ref
ert-fail(((should (equal (time-stamp-string "%3a" ref-time1) Mon)) :
#f(compiled-function () #<bytecode 0xbda32adea3e6e3b>)()
ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test
ert-run-test(#s(ert-test :name time-stamp-format-day-of-week :docume
ert-run-or-rerun-test(#s(ert--stats :selector ... :tests ... :test-m
ert-run-tests((not (or (tag :expensive-test) (tag :unstable) (tag :n
ert-run-tests-batch((not (or (tag :expensive-test) (tag :unstable) (
ert-run-tests-batch-and-exit((not (or (tag :expensive-test) (tag :un
eval((ert-run-tests-batch-and-exit '(not (or (tag :expensive-test) (
command-line-1(("-L" ":." "-l" "ert" "-l" "lisp/time-stamp-tests" "-
command-line()
normal-top-level()
Test time-stamp-format-day-of-week condition:
(ert-test-failed
((should
(equal
(time-stamp-string "%3a" ref-time1)
Mon))
:form
(equal " Mo" "Mo")
:value nil :explanation
(arrays-of-different-length 3 2 " Mo" "Mo" first-mismatch-at 0)))
FAILED 117/135 time-stamp-format-day-of-week (0.000415 sec)
passed 118/135 time-stamp-format-hours-12 (0.000579 sec)
passed 119/135 time-stamp-format-hours-24 (0.000581 sec)
passed 120/135 time-stamp-format-ignored-modifiers (0.000306 sec)
passed 121/135 time-stamp-format-minute (0.000419 sec)
passed 122/135 time-stamp-format-month-name (0.000326 sec)
passed 123/135 time-stamp-format-month-number (0.000418 sec)
passed 124/135 time-stamp-format-multiple-conversions (0.000698 sec)
passed 125/135 time-stamp-format-non-conversions (0.000174 sec)
passed 126/135 time-stamp-format-non-date-conversions (0.000432 sec)
passed 127/135 time-stamp-format-second (0.000424 sec)
Test time-stamp-format-string-width backtrace:
signal(ert-test-failed (((should (equal (time-stamp-string "%#3a" re
ert-fail(((should (equal (time-stamp-string "%#3a" ref-time3) SUN))
#f(compiled-function () #<bytecode -0xd4dc00a92f30942>)()
ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test
ert-run-test(#s(ert-test :name time-stamp-format-string-width :docum
ert-run-or-rerun-test(#s(ert--stats :selector ... :tests ... :test-m
ert-run-tests((not (or (tag :expensive-test) (tag :unstable) (tag :n
ert-run-tests-batch((not (or (tag :expensive-test) (tag :unstable) (
ert-run-tests-batch-and-exit((not (or (tag :expensive-test) (tag :un
eval((ert-run-tests-batch-and-exit '(not (or (tag :expensive-test) (
command-line-1(("-L" ":." "-l" "ert" "-l" "lisp/time-stamp-tests" "-
command-line()
normal-top-level()
Test time-stamp-format-string-width condition:
(ert-test-failed
((should
(equal
(time-stamp-string "%#3a" ref-time3)
SUN))
:form
(equal " SO" "SO")
:value nil :explanation
(arrays-of-different-length 3 2 " SO" "SO" first-mismatch-at 0)))
FAILED 128/135 time-stamp-format-string-width (0.000539 sec)
passed 129/135 time-stamp-format-time-zone-name (0.000231 sec)
passed 130/135 time-stamp-format-time-zone-offset (0.000485 sec)
passed 131/135 time-stamp-format-year-2digit (0.000393 sec)
passed 132/135 time-stamp-format-year-4digit (0.000225 sec)
passed 133/135 time-stamp-helper-safe-locals (0.000230 sec)
passed 134/135 time-stamp-helper-string-defaults (0.000352 sec)
passed 135/135 time-stamp-helper-zone-type-p (0.000208 sec)
Ran 135 tests, 133 results as expected, 2 unexpected (2021-09-17 20:46:21+0200, 5.091627 sec)
2 unexpected results:
FAILED time-stamp-format-day-of-week
FAILED time-stamp-format-string-width
^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#49723: 28.0.50; Test in coding.c for NUL bytes in filenames is not reliable
2021-09-17 19:00 ` Federico Tedin
@ 2021-09-18 6:51 ` Eli Zaretskii
2021-09-18 17:57 ` Federico Tedin
0 siblings, 1 reply; 18+ messages in thread
From: Eli Zaretskii @ 2021-09-18 6:51 UTC (permalink / raw)
To: Federico Tedin; +Cc: phst, 49723-done
> From: Federico Tedin <federicotedin@gmail.com>
> Cc: phst@google.com, 49723@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.
^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#49723: 28.0.50; Test in coding.c for NUL bytes in filenames is not reliable
2021-09-18 6:51 ` Eli Zaretskii
@ 2021-09-18 17:57 ` Federico Tedin
0 siblings, 0 replies; 18+ messages in thread
From: Federico Tedin @ 2021-09-18 17:57 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: phst, 49723-done
[-- Attachment #1: Type: text/plain, Size: 1931 bytes --]
Noted! Thank you.
Eli Zaretskii <eliz@gnu.org> schrieb am Sa. 18. Sept. 2021 um 08:51:
> > From: Federico Tedin <federicotedin@gmail.com>
> > Cc: phst@google.com, 49723@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.
>
[-- Attachment #2: Type: text/html, Size: 2694 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#49723: 28.0.50; Test in coding.c for NUL bytes in filenames is not reliable
2021-09-16 19:13 ` Federico Tedin
@ 2021-09-20 11:59 ` Michael Albinus
0 siblings, 0 replies; 18+ messages in thread
From: Michael Albinus @ 2021-09-20 11:59 UTC (permalink / raw)
To: Federico Tedin; +Cc: phst, 49723
Federico Tedin <federicotedin@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.
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2021-09-20 11:59 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-07-24 17:39 bug#49723: 28.0.50; Test in coding.c for NUL bytes in filenames is not reliable Eli Zaretskii
2021-09-14 19:01 ` Federico Tedin
2021-09-14 19:24 ` Eli Zaretskii
2021-09-14 22:17 ` Federico Tedin
2021-09-16 12:25 ` Eli Zaretskii
2021-09-16 16:58 ` Federico Tedin
2021-09-16 18:12 ` Michael Albinus
2021-09-16 18:30 ` Eli Zaretskii
2021-09-16 18:44 ` Michael Albinus
2021-09-16 19:07 ` Eli Zaretskii
2021-09-16 18:38 ` Federico Tedin
2021-09-16 18:51 ` Michael Albinus
2021-09-16 19:13 ` Federico Tedin
2021-09-20 11:59 ` Michael Albinus
2021-09-17 7:49 ` Eli Zaretskii
2021-09-17 19:00 ` Federico Tedin
2021-09-18 6:51 ` Eli Zaretskii
2021-09-18 17:57 ` Federico Tedin
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.