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

@@ -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 */

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

tests/test3.out

@@ -30,3 +30,4 @@ SHLVL: 4
 SHLVL: 4
 Elements in DISPLAY: 1
 Elements in FOO: 4
+MANPATH=man1:man2:man3