From c0b8e81b023bf1f118311a060662bea1ad565a19 Mon Sep 17 00:00:00 2001
From: ridiculousfish <corydoras@ridiculousfish.com>
Date: Sun, 12 Oct 2014 15:01:35 -0700
Subject: [PATCH] Change how arrays and environment variables interact.

Prior to this change, inherited environment variables
would be split on colons, becoming an array. This change
eliminates that behavior. Now environment variables are
always split on the record separator character (ASCII 0x1e),
with the exception of a short whitelist of PATH, MANPATH,
CDPATH. Likewise, exported variables are also exported
delimited by rs, with the exception of the above whitelist.

Fixes #1374, also see #1656
---
 env.cpp         | 28 ++++++++++++----------------
 tests/test3.in  |  6 ++++--
 tests/test3.out |  1 +
 3 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/env.cpp b/env.cpp
index 5fcaecde5..fc10e820a 100644
--- a/env.cpp
+++ b/env.cpp
@@ -415,17 +415,10 @@ wcstring env_get_pwd_slash(void)
     return pwd;
 }
 
-// Some variables should not be arrays. This used to be handled by a startup script, but we'd like to get down to 0 forks for startup, so handle it here.
-static bool variable_can_be_array(const wcstring &key)
+/* Here is the whitelist of variables that we colon-delimit, both incoming from the environment and outgoing back to it. This is deliberately very short - we don't want to add language-specific values like CLASSPATH. */
+static bool variable_is_colon_delimited_array(const wcstring &str)
 {
-    if (key == L"DISPLAY")
-    {
-        return false;
-    }
-    else
-    {
-        return true;
-    }
+    return contains(str, L"PATH", L"MANPATH", L"CDPATH");
 }
 
 void env_init(const struct config_paths_t *paths /* or NULL */)
@@ -487,7 +480,7 @@ void env_init(const struct config_paths_t *paths /* or NULL */)
             wcstring key = key_and_val.substr(0, eql);
             if (is_read_only(key) || is_electric(key)) continue;
             wcstring val = key_and_val.substr(eql + 1);
-            if (variable_can_be_array(key))
+            if (variable_is_colon_delimited_array(key))
             {
                 std::replace(val.begin(), val.end(), L':', ARRAY_SEP);
             }
@@ -1293,19 +1286,22 @@ static void get_exported(const env_node_t *n, std::map<wcstring, wcstring> &h)
     }
 }
 
+/* Given a map from key to value, add values to out of the form key=value */
 static void export_func(const std::map<wcstring, wcstring> &envs, std::vector<std::string> &out)
 {
+    out.reserve(out.size() + envs.size());
     std::map<wcstring, wcstring>::const_iterator iter;
     for (iter = envs.begin(); iter != envs.end(); ++iter)
     {
-        const std::string ks = wcs2string(iter->first);
+        const wcstring &key = iter->first;
+        const std::string &ks = wcs2string(key);
         std::string vs = wcs2string(iter->second);
 
-        for (size_t i=0; i < vs.size(); i++)
+        /* Arrays in the value are ASCII record separator (0x1e) delimited. But some variables should have colons. Add those. */
+        if (variable_is_colon_delimited_array(key))
         {
-            char &vc = vs.at(i);
-            if (vc == ARRAY_SEP)
-                vc = ':';
+            /* Replace ARRAY_SEP with colon */
+            std::replace(vs.begin(), vs.end(), (char)ARRAY_SEP, ':');
         }
 
         /* Put a string on the vector */
diff --git a/tests/test3.in b/tests/test3.in
index 4dec9a838..b162939d5 100644
--- a/tests/test3.in
+++ b/tests/test3.in
@@ -248,7 +248,9 @@ env SHLVL="  3" ../fish -c 'echo SHLVL: $SHLVL'
 env DISPLAY="localhost:0.0" ../fish -c 'echo Elements in DISPLAY: (count $DISPLAY)'
 # We can't use PATH for this because the global configuration will modify PATH
 # based on /etc/paths and /etc/paths.d.
-# At the moment, most variables split on :. So we can use an arbitrary variable for this.
-env FOO="one:two:three:four" ../fish -c 'echo Elements in FOO: (count $FOO)'
+# Exported arrays should use record separator, with a few exceptions. So we can use an arbitrary variable for this.
+env FOO=one\x1etwo\x1ethree\x1efour ../fish -c 'echo Elements in FOO: (count $FOO)'
+# some must use colon separators!
+set -lx MANPATH man1 man2 man3 ; env | grep MANPATH
 
 true
diff --git a/tests/test3.out b/tests/test3.out
index 5d3a48b23..d7dd2c434 100644
--- a/tests/test3.out
+++ b/tests/test3.out
@@ -30,3 +30,4 @@ SHLVL: 4
 SHLVL: 4
 Elements in DISPLAY: 1
 Elements in FOO: 4
+MANPATH=man1:man2:man3