45583/0004: Fix segfault on resolving symlink loops

4 files changed, 13 insertions, 10 deletions

diff --git a/ChangeLog b/ChangeLog
index c6478935c..3083552c3 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,8 @@
 2020-03-25  Daniel Shahaf  <d.s@daniel.shahaf.name>
 
+	* 45583/0004: Etc/BUGS, Src/utils.c, Test/D02glob.ztst: Fix
+	segfault on resolving symlink loops
+
 	* 45583/0003: Src/hist.c, Src/subst.c: chrealpath: Let caller
 	decide how the return value should be allocated.
 
diff --git a/Etc/BUGS b/Etc/BUGS
index 99a0d9753..2501d59a7 100644
--- a/Etc/BUGS
+++ b/Etc/BUGS
@@ -29,5 +29,6 @@ skipped when STTY=... is set for that command
 44007 - Martijn - exit in trap executes rest of function
 See test case in Test/C03traps.ztst.
 ------------------------------------------------------------------------
-45282: ${${:-foo}:P} where foo is a symlink that points to itself segfaults
+45282: xsymlinks() segfaults on symlink loops
+Fixed for some cases; need to audit remaining callers
 ------------------------------------------------------------------------
diff --git a/Src/utils.c b/Src/utils.c
index 4d16de591..b0f6820fb 100644
--- a/Src/utils.c
+++ b/Src/utils.c
@@ -1038,11 +1038,11 @@ xsymlink(char *s, int heap)
     if (*s != '/')
 	return NULL;
     *xbuf = '\0';
-    if (xsymlinks(s + 1, 1) < 0)
+    if (!chrealpath(&s, 'P', heap)) {
 	zwarn("path expansion failed, using root directory");
-    if (!*xbuf)
 	return heap ? dupstring("/") : ztrdup("/");
-    return heap ? dupstring(xbuf) : ztrdup(xbuf);
+    }
+    return s;
 }
 
 /**/
diff --git a/Test/D02glob.ztst b/Test/D02glob.ztst
index 248cc7ff5..041784310 100644
--- a/Test/D02glob.ztst
+++ b/Test/D02glob.ztst
@@ -690,10 +690,9 @@
  # This is a bit brittle as it depends on PATH_MAX.
  # We could use sysconf..
  bad_pwd="/${(l:16000:: :):-}"
- print ${bad_pwd:P}
+ print ${bad_pwd:P} | wc -c
 0:modifier ':P' with path too long
-?(eval):4: path expansion failed, using root directory
->/
+>16002
 
  foo=a
  value="ac"
@@ -765,7 +764,7 @@
   } always {
     rm -f glob.tmp/trap glob.tmp/loop
   }
--f:the ':P' modifier handles symlink loops in the last path component
+0:the ':P' modifier handles symlink loops in the last path component
 *>*/(trap|loop)
 *>*/(trap|loop)
 
@@ -777,7 +776,7 @@
   } always {
     rm -f glob.tmp/trap glob.tmp/loop
   }
--f:the ':P' modifier handles symlink loops before the last path component
+0:the ':P' modifier handles symlink loops before the last path component
 *>*/glob.tmp/loop/trailing/components
 *>*/glob.tmp/(loop|trap)/trailing/components
 
@@ -789,7 +788,7 @@
   } always {
     rm -f glob.tmp/flip glob.tmp/flop
   }
--f:the ':P' modifier handles symlink loops other than the trivial case
+0:the ':P' modifier handles symlink loops other than the trivial case
 *>*/glob.tmp/(flip|flop)
 *>*/glob.tmp/(flip|flop)/trailing/components