bug#21454: 25.0.50; `parse-colon-path' fails with paths containing consecutive directory separators

bug#21454: 25.0.50; `parse-colon-path' fails with paths containing consecutive directory separators

[Tino Calancha]

[-- Attachment #1: Type: text/plain, Size: 5413 bytes --]



Implementation use `substitute-in-file-name' which substitute environment variables but 
also drop all in front of consecutive directory separators (up to the last 
of those dir. separators). The documentation of `parse-colon-path' dont 
mention about that behaviour.

In unix systems there is no difference between:

I)  /foo//bar/baz
II) /foo/bar/baz

`substitute-in-file-name' would translate I) into:
/bar/baz

In case path to parse contains an environment variable:
FOO=/foo/bar
BAZ=/foo/bar/

A) path="$FOO/bin" ; works (FOO is a directory file name)
B) path="$BAZ/bin" ; fails (FOO is not a directory file name)


Observed same behaviour on this fuction in emacs versions: 19, 20, 21, 22, 
23, 24 and 25.
Attached patch calls `substitute-env-in-file-name' instead of 
`substitute-in-file-name' and it returns same values for 
inputs I) and II) or A) and B).

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
In *scratch* buffer after started session with:
emacs -Q

(let* ((foo "/foo/bar//baz/:/qux/quux/corge//")) ; '//'
   (setenv "FOO" foo)
   (parse-colon-path (getenv "FOO")))
("/baz/" "/")

(let ((foo "/foo/bar/baz/qux/:/quux/corge/:/grault/graply/waldo")) ; `ok'
   (setenv "FOO" foo)
   (parse-colon-path "$FOO"))
("/foo/bar/baz/qux/:/quux/corge/:/grault/graply/waldo/")

(let ((foo "/foo/bar/baz/qux/:/quux/corge/:/grault/graply/waldo/")) ; end with '/' and `parse-colon-path' add another
   (setenv "FOO" foo)
   (parse-colon-path "$FOO"))
("/")

(let ((foo "/foo/bar/baz/qux/:/quux/corge/:/grault/graply/waldo/")) ; `getenv' makes it work somehow
   (setenv "FOO" foo)
   (parse-colon-path (getenv "FOO")))
("/foo/bar/baz/qux/" "/quux/corge/" "/grault/graply/waldo/")

(let ((foo "/foo/bar/baz//qux/:/quux/corge/:/grault/garply/waldo/")) ; `getenv' dont help with '//'
   (setenv "FOO" foo)
   (parse-colon-path (getenv "FOO")))
("/qux/" "/quux/corge/" "/grault/garply/waldo/")

(let ((foo "/foo/bar/baz//qux/:/quux/corge/:/grault/garply/waldo")) ; '//'
   (setenv "FOO" foo)
   (parse-colon-path "$FOO"))
("/qux/:/quux/corge/:/grault/garply/waldo/")

(let* ((foo "/foo/bar")                    ; `ok' foo not end with '/'
        (bar "$FOO/baz/qux/:/quux/corge/"))
   (setenv "FOO" foo)
   (setenv "BAR" bar)
   (parse-colon-path (getenv "BAR")))
("/foo/bar/baz/qux/" "/quux/corge/")

(let* ((foo "/foo/bar/")                   ; foo ends with '/'
 	   (bar "$FOO/baz/qux/:/quux/corge/"))
   (setenv "FOO" foo)
   (setenv "BAR" bar)
   (parse-colon-path (getenv "BAR")))
("/baz/qux/" "/quux/corge/")


;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

In GNU Emacs 25.0.50.1 (x86_64-unknown-linux-gnu, GTK+ Version 2.24.23)
  of 2015-09-10
Repository revision: f962c5d6472804f788cdd54631bcc46aab5f59ce
Windowing system distributor 'Scientific Linux', version 11.0.11500000
System Description:	Scientific Linux release 6.7 (Carbon)

Configured using:
  'configure --without-makeinfo --with-gif=no'

Configured features:
XPM JPEG TIFF PNG IMAGEMAGICK SOUND DBUS GCONF NOTIFY LIBSELINUX LIBXML2
FREETYPE XFT ZLIB TOOLKIT_SCROLL_BARS GTK2 X11

Important settings:
   value of $LANG: en_US.utf8
   value of $XMODIFIERS: @im=none
   locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
   tooltip-mode: t
   global-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
   transient-mark-mode: t

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
<backtab> is undefined
Quit
completing-read-default: Command attempted to use minibuffer while in minibuffer
user-error: Beginning of history; no preceding item
Quit [2 times]

Load-path shadows:
None found.

Features:
(shadow sort gnus-util mail-extr emacsbug message dired format-spec
rfc822 mml mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231
mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums
mm-util help-fns help-mode easymenu cl-loaddefs pcase cl-lib mail-prsvr
mail-utils time-date mule-util tooltip eldoc electric uniquify
ediff-hook vc-hooks lisp-float-type mwheel x-win term/common-win x-dnd
tool-bar dnd fontset image regexp-opt fringe tabulated-list newcomment
elisp-mode lisp-mode prog-mode register page menu-bar rfn-eshadow timer
select scroll-bar mouse jit-lock font-lock syntax facemenu font-core
frame 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 charscript
case-table epa-hook jka-cmpr-hook help simple abbrev minibuffer
cl-preloaded nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote dbusbind inotify
dynamic-setting system-font-setting font-render-setting move-toolbar gtk
x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 80421 12421)
  (symbols 48 18940 0)
  (miscs 40 39 110)
  (strings 32 12495 3997)
  (string-bytes 1 361986)
  (vectors 16 11158)
  (vector-slots 8 408984 7901)
  (floats 8 131 123)
  (intervals 56 192 0)
  (buffers 976 12)
  (heap 1024 26696 621))

[-- Attachment #2: Type: text/plain, Size: 1382 bytes --]

--- files.el	2015-09-10 19:31:06.569650962 +0900
+++ files_patched.el	2015-09-10 20:01:00.303650748 +0900
@@ -667,7 +667,8 @@
   "Value of the CDPATH environment variable, as a list.
 Not actually set up until the first time you use it.")
 
-(defun parse-colon-path (search-path)
+(require 'env)
+(defun myparse-colon-path2 (search-path)
   "Explode a search path into a list of directory names.
 Directories are separated by `path-separator' (which is colon in
 GNU and Unix systems).  Substitute environment variables into the
@@ -675,10 +676,12 @@
 a leading or trailing separator, or two adjacent separators), return
 nil (meaning `default-directory') as the associated list element."
   (when (stringp search-path)
-    (mapcar (lambda (f)
-	      (if (equal "" f) nil
-		(substitute-in-file-name (file-name-as-directory f))))
-	    (split-string search-path path-separator))))
+    (let* ((path-list        (mapcar 'substitute-env-vars (split-string search-path path-separator)))
+           (search-path-sub  (mapconcat 'identity path-list path-separator)))
+      (mapcar (lambda (f)
+                (if (equal "" f) nil
+                  (expand-file-name (file-name-as-directory f))))
+              (split-string search-path-sub path-separator)))))
 
 (defun cd-absolute (dir)
   "Change current directory to given absolute file name DIR."

bug#21454: (no subject)

[Tino Calancha]

[-- Attachment #1: Type: text/plain, Size: 107 bytes --]


Previous patch renamed `parse-colon-path' with a different name. 
Attached the patch using the right name.

[-- Attachment #2: Type: text/plain, Size: 1271 bytes --]

--- files.el	2015-09-10 19:31:06.569650962 +0900
+++ files_patched.el	2015-09-10 21:42:02.045650024 +0900
@@ -667,6 +667,7 @@
   "Value of the CDPATH environment variable, as a list.
 Not actually set up until the first time you use it.")
 
+(require 'env)
 (defun parse-colon-path (search-path)
   "Explode a search path into a list of directory names.
 Directories are separated by `path-separator' (which is colon in
@@ -675,10 +676,12 @@
 a leading or trailing separator, or two adjacent separators), return
 nil (meaning `default-directory') as the associated list element."
   (when (stringp search-path)
-    (mapcar (lambda (f)
-	      (if (equal "" f) nil
-		(substitute-in-file-name (file-name-as-directory f))))
-	    (split-string search-path path-separator))))
+    (let* ((path-list        (mapcar 'substitute-env-vars (split-string search-path path-separator)))
+           (search-path-sub  (mapconcat 'identity path-list path-separator)))
+      (mapcar (lambda (f)
+                (if (equal "" f) nil
+                  (expand-file-name (file-name-as-directory f))))
+              (split-string search-path-sub path-separator)))))
 
 (defun cd-absolute (dir)
   "Change current directory to given absolute file name DIR."

bug#21454: 25.0.93; `parse-colon-path' over dirs with 2 consecutive dir sep

[Tino Calancha]

[-- Attachment #1: Type: text/plain, Size: 1078 bytes --]


There are 2 typos in previous report:

>B) path="$BAZ/bin" ; fails (FOO is not a directory file name)
Should be:
B) path="$BAZ/bin" ; fails (BAZ is not a directory file name)

>let ((foo "/foo/bar/baz/qux/:/quux/corge/:/grault/graply/waldo")) ; `ok'
>  (setenv "FOO" foo)
>  (parse-colon-path "$FOO"))
>("/foo/bar/baz/qux/:/quux/corge/:/grault/graply/waldo/")
It's _not_ ok: `parse-colon-path' returns list with 1 element (don't split on ':').


New patch applied over previous tests returns:

("/foo/bar/baz/" "/qux/quux/corge/")
("/foo/bar/baz/qux/" "/quux/corge/" "/grault/graply/waldo/")
("/foo/bar/baz/qux/" "/quux/corge/" "/grault/graply/waldo/")
("/foo/bar/baz/qux/" "/quux/corge/" "/grault/graply/waldo/")
("/foo/bar/baz/qux/" "/quux/corge/" "/grault/garply/waldo/")
("/foo/bar/baz/qux/" "/quux/corge/" "/grault/garply/waldo/")
("/foo/bar/baz/qux/" "/quux/corge/")
("/foo/bar/baz/qux/" "/quux/corge/")


In GNU Emacs 25.0.93.1 (x86_64-pc-linux-gnu, GTK+ Version 2.24.30)
  of 2016-04-25 built on calancha-pc
Repository revision: 0cd2e923dba8d8c7128b0c084ce6af22069e8db5

[-- Attachment #2: Type: text/plain, Size: 1004 bytes --]

diff --git a/lisp/files.el b/lisp/files.el
index 5cdd384..7d92535 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -675,10 +675,14 @@ parse-colon-path
 a leading or trailing separator, or two adjacent separators), return
 nil (meaning `default-directory') as the associated list element."
   (when (stringp search-path)
-    (mapcar (lambda (f)
-	      (if (equal "" f) nil
-		(substitute-in-file-name (file-name-as-directory f))))
-	    (split-string search-path path-separator))))
+    (let ((path (mapconcat 'identity
+                           (mapcar 'substitute-env-vars
+                                   (split-string search-path path-separator))
+                           path-separator)))
+      (mapcar (lambda (f)
+                (if (equal "" f) nil
+                  (file-name-as-directory (expand-file-name f))))
+              (split-string path path-separator)))))
 
 (defun cd-absolute (dir)
   "Change current directory to given absolute file name DIR."

bug#21454: 25.1.50; `parse-colon-path' fails with file names containing multiple consecutive "/"

[Tino Calancha]

A new patch handling multiple "/" in front of a dir as it was
done before: collapse multiple "/" in front of the directory.

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
From c363eee16029566be565ef37bec7952fdb630bb1 Mon Sep 17 00:00:00 2001
From: Tino Calancha <tino.calancha@gmail.com>
Date: Tue, 13 Sep 2016 17:40:55 +0900
Subject: [PATCH] parse-colon-path:  Handle a dir with 2 consecutive dir
  separator

Do not truncate /foo//bar to /bar/
* lisp/files.el (parse-colon-path): Use substitute-env-vars and
expand-file-name instead of substitute-in-file-name (Bug#21454).
---
  lisp/files.el | 15 +++++++++++----
  1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/lisp/files.el b/lisp/files.el
index 4bd708d..79e5abf 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -672,10 +672,17 @@ parse-colon-path
  a leading or trailing separator, or two adjacent separators), return
  nil (meaning `default-directory') as the associated list element."
    (when (stringp search-path)
-    (mapcar (lambda (f)
-	      (if (equal "" f) nil
-		(substitute-in-file-name (file-name-as-directory f))))
-	    (split-string search-path path-separator))))
+    (let ((spath (substitute-env-vars search-path)))
+      (mapcar (lambda (f)
+                (if (equal "" f) nil
+                  (let ((dir (expand-file-name (file-name-as-directory 
f))))
+                    ;; Previous implementation used 
`substitute-in-file-name'
+                    ;; which collapse multiple "/" in front.  Do the same 
for
+                    ;; backward compatibility.
+                    (while (and (char-equal ?/ (aref dir 0))
+                                (char-equal ?/ (aref dir 1)))
+                      (setq dir (substring dir 1))) dir)))
+              (split-string spath path-separator)))))

  (defun cd-absolute (dir)
    "Change current directory to given absolute file name DIR."
-- 
2.9.3

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
In GNU Emacs 25.1.50.1 (x86_64-pc-linux-gnu, GTK+ Version 3.21.5)
  of 2016-09-13
Repository revision: ead76c4603f2c4a1761ab8a7dd5cf6f56e782fb2

bug#21454: 25.1.50; `parse-colon-path' fails with file names containing multiple consecutive "/"

[Andreas Schwab]

On Sep 13 2016, Tino Calancha <tino.calancha@gmail.com> wrote:

> diff --git a/lisp/files.el b/lisp/files.el
> index 4bd708d..79e5abf 100644
> --- a/lisp/files.el
> +++ b/lisp/files.el
> @@ -672,10 +672,17 @@ parse-colon-path
>   a leading or trailing separator, or two adjacent separators), return
>   nil (meaning `default-directory') as the associated list element."
>     (when (stringp search-path)
> -    (mapcar (lambda (f)
> -	      (if (equal "" f) nil
> -		(substitute-in-file-name (file-name-as-directory f))))
> -	    (split-string search-path path-separator))))
> +    (let ((spath (substitute-env-vars search-path)))
> +      (mapcar (lambda (f)
> +                (if (equal "" f) nil
> +                  (let ((dir (expand-file-name (file-name-as-directory 
> f))))
> +                    ;; Previous implementation used 
> `substitute-in-file-name'
> +                    ;; which collapse multiple "/" in front.  Do the same 
> for
> +                    ;; backward compatibility.
> +                    (while (and (char-equal ?/ (aref dir 0))
> +                                (char-equal ?/ (aref dir 1)))
> +                      (setq dir (substring dir 1))) dir)))

aref signals an error if dir is too short.  You can also avoid the loop
if you write it as (if (string-match "\\`/+" dir) (substring
dir (1- (match-end 0))) dir).

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

bug#21454: 25.1.50; `parse-colon-path' fails with file names containing multiple consecutive "/"

[Tino Calancha]

On Tue, 13 Sep 2016, Andreas Schwab wrote:

> On Sep 13 2016, Tino Calancha <tino.calancha@gmail.com> wrote:
>
>> +                    ;; backward compatibility.
>> +                    (while (and (char-equal ?/ (aref dir 0))
>> +                                (char-equal ?/ (aref dir 1)))
>> +                      (setq dir (substring dir 1))) dir)))
> aref signals an error if dir is too short.  You can also avoid the loop
> if you write it as (if (string-match "\\`/+" dir) (substring
> dir (1- (match-end 0))) dir).

Indeed.  Thank you very much!
Here is the new patch:

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
From 9adbc52a3b7a744fddc6f6692d53bc6617d5892c Mon Sep 17 00:00:00 2001
From: Tino Calancha <tino.calancha@gmail.com>
Date: Tue, 13 Sep 2016 18:34:29 +0900
Subject: [PATCH] parse-colon-path:  Handle a dir with 2 consecutive dir
  separator

Do not truncate /foo//bar to /bar/
* lisp/files.el (parse-colon-path): Use substitute-env-vars and
expand-file-name instead of substitute-in-file-name (Bug#21454).
---
  lisp/files.el | 14 ++++++++++----
  1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/lisp/files.el b/lisp/files.el
index 4bd708d..7b84ae5 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -672,10 +672,16 @@ parse-colon-path
  a leading or trailing separator, or two adjacent separators), return
  nil (meaning `default-directory') as the associated list element."
    (when (stringp search-path)
-    (mapcar (lambda (f)
-	      (if (equal "" f) nil
-		(substitute-in-file-name (file-name-as-directory f))))
-	    (split-string search-path path-separator))))
+    (let ((spath (substitute-env-vars search-path)))
+      (mapcar (lambda (f)
+                (if (equal "" f) nil
+                  (let ((dir (expand-file-name (file-name-as-directory 
f))))
+                    ;; Previous implementation used 
`substitute-in-file-name'
+                    ;; which collapse multiple "/" in front.  Do the same 
for
+                    ;; backward compatibility.
+                    (if (string-match "\\`/+" dir)
+                        (substring dir (1- (match-end 0))) dir))))
+              (split-string spath path-separator)))))

  (defun cd-absolute (dir)
    "Change current directory to given absolute file name DIR."
-- 
2.9.3

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
In GNU Emacs 25.1.50.1 (x86_64-pc-linux-gnu, GTK+ Version 3.21.5)
  of 2016-09-13
Repository revision: ead76c4603f2c4a1761ab8a7dd5cf6f56e782fb2

bug#21454: 25.1.50; `parse-colon-path' fails with file names containing multiple consecutive "/"

[npostavs]

Tino Calancha <tino.calancha@gmail.com> writes:

> Indeed.  Thank you very much!
> Here is the new patch:

I guess this could be pushed?

bug#21454: 25.1.50; `parse-colon-path' fails with file names containing multiple consecutive "/"

[Lars Ingebrigtsen]

Tino Calancha <tino.calancha@gmail.com> writes:

> Do not truncate /foo//bar to /bar/
> * lisp/files.el (parse-colon-path): Use substitute-env-vars and
> expand-file-name instead of substitute-in-file-name (Bug#21454).

The bug report was slightly unclear, but I think the taste case was

(parse-colon-path "/foo//bar/baz")
=> ("/bar/baz/")

and that being a mistake?  But I'm not sure it is -- In Emacs, if you do
that in, say, find-file, you'll end up in /bar/baz, and that's by
design.

But I may have misread what this was about...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#21454: 25.1.50; `parse-colon-path' fails with file names containing multiple consecutive "/"

[Andreas Schwab]

On Jun 25 2019, Lars Ingebrigtsen <larsi@gnus.org> wrote:

> Tino Calancha <tino.calancha@gmail.com> writes:
>
>> Do not truncate /foo//bar to /bar/
>> * lisp/files.el (parse-colon-path): Use substitute-env-vars and
>> expand-file-name instead of substitute-in-file-name (Bug#21454).
>
> The bug report was slightly unclear, but I think the taste case was
>
> (parse-colon-path "/foo//bar/baz")
> => ("/bar/baz/")
>
> and that being a mistake?  But I'm not sure it is -- In Emacs, if you do
> that in, say, find-file, you'll end up in /bar/baz, and that's by
> design.

For file names coming from outside the double slash should not be
special.  For example, `emacs foo//bar' visits the file `foo/bar', not
`/bar'.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

bug#21454: 25.1.50; `parse-colon-path' fails with file names containing multiple consecutive "/"

[Lars Ingebrigtsen]

Andreas Schwab <schwab@suse.de> writes:

> On Jun 25 2019, Lars Ingebrigtsen <larsi@gnus.org> wrote:
>
>> Tino Calancha <tino.calancha@gmail.com> writes:
>>
>>> Do not truncate /foo//bar to /bar/
>>> * lisp/files.el (parse-colon-path): Use substitute-env-vars and
>>> expand-file-name instead of substitute-in-file-name (Bug#21454).
>>
>> The bug report was slightly unclear, but I think the taste case was
>>
>> (parse-colon-path "/foo//bar/baz")
>> => ("/bar/baz/")
>>
>> and that being a mistake?  But I'm not sure it is -- In Emacs, if you do
>> that in, say, find-file, you'll end up in /bar/baz, and that's by
>> design.
>
> For file names coming from outside the double slash should not be
> special.  For example, `emacs foo//bar' visits the file `foo/bar', not
> `/bar'.

Makes sense.  I've now applied this patch to Emacs 28 (and adjusted the
test that expected :failure).

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no