Fixed add_man_subdirs to correctly add PATH elements with trailing paths

4 files changed, 51 insertions, 5 deletions

diff --git a/NEWS.md b/NEWS.md
index 554b4bb3..d4ef874d 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -7,6 +7,8 @@ Fixes:
  * Fix test failures when a working `iconv` is not available.
  * Ensure that timestamps read from the database can go past the year 2038,
    even on systems where this is not the default.
+ * Fix `manpath` not parsing `PATH` entries with trailing slash correctly
+   for guessing `MANPATH` entries.
 
 Improvements:
 
diff --git a/src/manp.c b/src/manp.c
index 174b2926..bcb919ca 100644
--- a/src/manp.c
+++ b/src/manp.c
@@ -1056,35 +1056,50 @@ static void add_dir_to_list (gl_list_t list, const char *dir)
 static void add_man_subdirs (gl_list_t list, const char *path)
 {
 	char *newpath;
+        char *trimmed_path = xstrdup (path);
 
 	/* don't assume anything about path, especially that it ends in
 	   "bin" or even has a '/' in it! */
 
-	char *subdir = strrchr (path, '/');
+	char *subdir = strrchr (trimmed_path, '/');
+
+	/* Trailing slash or root directory. Remove the trailing slash and
+	   try again. If root directory, subdir will be null, so we don't
+	   cause a segfault. If a path element is '/', we will correctly add
+	   /man and /share/man manpaths. */
+	if (subdir && strncmp (subdir, "/", 2) == 0) {
+		subdir[0] = '\0';
+		subdir = strrchr (trimmed_path, '/');
+	}
 	if (subdir) {
-		newpath = xasprintf ("%.*s/man", (int) (subdir - path), path);
+		newpath = xasprintf ("%.*s/man",
+				     (int) (subdir - trimmed_path),
+				     trimmed_path);
 		if (is_directory (newpath) == 1)
 			add_dir_to_list (list, newpath);
 		free (newpath);
 	}
 
-	newpath = xasprintf ("%s/man", path);
+	newpath = xasprintf ("%s/man", trimmed_path);
 	if (is_directory (newpath) == 1)
 		add_dir_to_list (list, newpath);
 	free (newpath);
 
 	if (subdir) {
 		newpath = xasprintf ("%.*s/share/man",
-				     (int) (subdir - path), path);
+				     (int) (subdir - trimmed_path),
+				     trimmed_path);
 		if (is_directory (newpath) == 1)
 			add_dir_to_list (list, newpath);
 		free (newpath);
 	}
 
-	newpath = xasprintf ("%s/share/man", path);
+	newpath = xasprintf ("%s/share/man", trimmed_path);
 	if (is_directory (newpath) == 1)
 		add_dir_to_list (list, newpath);
 	free (newpath);
+
+        free (trimmed_path);
 }
 
 struct canonicalized_path {
diff --git a/src/tests/Makefile.am b/src/tests/Makefile.am
index 4f0b872e..7cdad4ac 100644
--- a/src/tests/Makefile.am
+++ b/src/tests/Makefile.am
@@ -57,6 +57,7 @@ ALL_TESTS = \
 	mandb-symlink-beats-whatis-ref \
 	mandb-symlink-target-timestamp \
 	mandb-whatis-broken-link-changes \
+	manpath-slash \
 	whatis-path-to-executable \
 	zsoelim-so-includes
 if !CROSS_COMPILING
diff --git a/src/tests/manpath-slash b/src/tests/manpath-slash
new file mode 100755
index 00000000..181bd92b
--- /dev/null
+++ b/src/tests/manpath-slash
@@ -0,0 +1,28 @@
+#! /bin/sh
+
+: "${srcdir=.}"
+# shellcheck source-path=SCRIPTDIR
+. "$srcdir/testlib.sh"
+
+: "${MANPATH=manpath}"
+
+# Check whether manpath correctly parses PATH elements with trailing slash.
+
+init
+fake_config /usr/share/man
+mkdir -p "$tmpdir/bin"
+mkdir -p "$tmpdir/share/man"
+OLDPATH="$PATH"
+export PATH="$tmpdir/bin/:$OLDPATH"
+output=$(run $MANPATH)
+case "$output" in
+	"$tmpdir/share/man:"*) report "manpath with trailing slash" 0 ;;
+	*) report "manpath with trailing slash" 1 ;;
+esac
+export PATH="$tmpdir/bin:$OLDPATH"
+output=$(run $MANPATH)
+case "$output" in
+	"$tmpdir/share/man:"*) report "manpath without trailing slash" 0 ;;
+	*) report "manpath without trailing slash" 1 ;;
+esac
+finish