* 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: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: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: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: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: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 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
* 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
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.