From 669eafb55f1a6eff3b72bec73470bb31d4497cb3 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sat, 24 Mar 2018 23:32:55 -0700 Subject: [PATCH] Stop exporting empty variables as ENV_NULL Localize the encoding of empty variables as ENV_NULL into the universal variables component, and ensure they are not exported as ENV_NULL. Fixes #4846 --- src/common.cpp | 27 ++++++++++++++++++ src/common.h | 6 ++++ src/env.cpp | 53 ++++-------------------------------- src/env.h | 9 ------ src/env_universal_common.cpp | 19 +++++++++++-- tests/set.err | 3 ++ tests/set.in | 6 ++++ tests/set.out | 6 ++++ 8 files changed, 71 insertions(+), 58 deletions(-) diff --git a/src/common.cpp b/src/common.cpp index fbee108ff..f20f5206f 100644 --- a/src/common.cpp +++ b/src/common.cpp @@ -1791,6 +1791,33 @@ bool contains(const wcstring_list_t &list, const wcstring &str) { return std::find(list.begin(), list.end(), str) != list.end(); } +wcstring_list_t split_string(const wcstring &val, wchar_t sep) { + wcstring_list_t out; + size_t pos = 0, end = val.size(); + while (pos <= end) { + size_t next_pos = val.find(sep, pos); + if (next_pos == wcstring::npos) { + next_pos = end; + } + out.emplace_back(val, pos, next_pos - pos); + pos = next_pos + 1; // skip the separator, or skip past the end + } + return out; +} + +wcstring join_strings(const wcstring_list_t &vals, wchar_t sep) { + wcstring result; + bool first = true; + for (const wcstring &s : vals) { + if (!first) { + result.push_back(sep); + } + result.append(s); + first = false; + } + return result; +} + int create_directory(const wcstring &d) { bool ok = false; struct stat buf; diff --git a/src/common.h b/src/common.h index 9b84f7625..765412ca5 100644 --- a/src/common.h +++ b/src/common.h @@ -326,6 +326,12 @@ bool string_suffixes_string(const wchar_t *proposed_suffix, const wcstring &valu bool string_prefixes_string_case_insensitive(const wcstring &proposed_prefix, const wcstring &value); +/// Split a string by a separator character. +wcstring_list_t split_string(const wcstring &val, wchar_t sep); + +/// Join a list of strings by a separator character. +wcstring join_strings(const wcstring_list_t &vals, wchar_t sep); + enum fuzzy_match_type_t { // We match the string exactly: FOOBAR matches FOOBAR. fuzzy_match_exact = 0, diff --git a/src/env.cpp b/src/env.cpp index 568d392af..a6189def6 100644 --- a/src/env.cpp +++ b/src/env.cpp @@ -109,30 +109,6 @@ static const wcstring_list_t curses_variables({L"TERM", L"TERMINFO", L"TERMINFO_ static void init_locale(); static void init_curses(); -/// This is used to convert a serialized env_var_t back into a list. It is used when reading legacy -/// (fish 2.x) encoded vars stored in the universal variable file and the environment. -static wcstring_list_t tokenize_variable_array(const wcstring &val) { - // Zero element arrays are externally encoded as this placeholder string. - if (val == ENV_NULL) return {}; - - wcstring_list_t out; - size_t pos = 0, end = val.size(); - while (pos <= end) { - size_t next_pos = val.find(ARRAY_SEP, pos); - if (next_pos == wcstring::npos) { - next_pos = end; - } - out.emplace_back(val, pos, next_pos - pos); - pos = next_pos + 1; // skip the separator, or skip past the end - } - return out; -} - -/// This is used to convert a serialized env_var_t back into a list. -wcstring_list_t decode_serialized(const wcstring &s) { - return tokenize_variable_array(s); -} - // Struct representing one level in the function variable stack. // Only our variable stack should create and destroy these class env_node_t { @@ -916,14 +892,8 @@ void env_init(const struct config_paths_t *paths /* or NULL */) { key.assign(key_and_val, 0, eql); if (is_read_only(key) || is_electric(key)) continue; val.assign(key_and_val, eql + 1, wcstring::npos); - if (variable_is_colon_delimited_var(key)) { - std::replace(val.begin(), val.end(), L':', ARRAY_SEP); - wcstring_list_t values = decode_serialized(val); - env_set(key, ENV_EXPORT | ENV_GLOBAL, values); - } else { - wcstring_list_t values = decode_serialized(val); - env_set(key, ENV_EXPORT | ENV_GLOBAL, values); - } + wchar_t sep = variable_is_colon_delimited_var(key) ? L':' : ARRAY_SEP; + env_set(key, ENV_EXPORT | ENV_GLOBAL, split_string(val, sep)); } } @@ -1322,16 +1292,8 @@ const wcstring_list_t &env_var_t::as_list() const { return vals; } /// Return a string representation of the var. At the present time this uses the legacy 2.x /// encoding. wcstring env_var_t::as_string() const { - if (this->vals.empty()) return wcstring(ENV_NULL); - wchar_t sep = (flags & flag_colon_delimit) ? L':' : ARRAY_SEP; - auto it = this->vals.cbegin(); - wcstring result(*it); - while (++it != vals.end()) { - result.push_back(sep); - result.append(*it); - } - return result; + return join_strings(vals, sep); } void env_var_t::to_list(wcstring_list_t &out) const { @@ -1539,16 +1501,13 @@ static void export_func(const var_table_t &envs, std::vector &out) // Replace ARRAY_SEP with colon. std::replace(vs.begin(), vs.end(), (char)ARRAY_SEP, ':'); } - - // Put a string on the vector. - out.push_back(std::string()); - std::string &str = out.back(); + // Create and append a string of the form ks=vs + std::string str; str.reserve(ks.size() + 1 + vs.size()); - - // Append our environment variable data to it. str.append(ks); str.append("="); str.append(vs); + out.push_back(std::move(str)); } } diff --git a/src/env.h b/src/env.h index 543b4dd11..7ebafb51a 100644 --- a/src/env.h +++ b/src/env.h @@ -20,12 +20,6 @@ extern bool curses_initialized; /// that seems logical. #define ARRAY_SEP (wchar_t)0x1e -/// String containing the character for separating two array elements. -#define ARRAY_SEP_STR L"\x1e" - -/// Value denoting a null string. -#define ENV_NULL L"\x1d" - // Flags that may be passed as the 'mode' in env_set / env_get. enum { /// Default mode. Used with `env_get()` to indicate the caller doesn't care what scope the var @@ -122,9 +116,6 @@ class env_var_t { bool operator!=(const env_var_t &var) const { return vals != var.vals; } }; -/// This is used to convert a serialized env_var_t back into a list. -wcstring_list_t decode_serialized(const wcstring &s); - /// Gets the variable with the specified name, or none() if it does not exist. maybe_t env_get(const wcstring &key, env_mode_flags_t mode = ENV_DEFAULT); diff --git a/src/env_universal_common.cpp b/src/env_universal_common.cpp index 758c16725..643078f3e 100644 --- a/src/env_universal_common.cpp +++ b/src/env_universal_common.cpp @@ -255,6 +255,21 @@ static bool append_file_entry(fish_message_type_t type, const wcstring &key_in, return success; } +/// Encoding of a null string. +static const wchar_t *ENV_NULL = L"\x1d"; + +/// Decode a serialized universal variable value into a list. +static wcstring_list_t decode_serialized(const wcstring &val) { + if (val == ENV_NULL) return {}; + return split_string(val, ARRAY_SEP); +} + +/// Decode a a list into a serialized universal variable value. +static wcstring encode_serialized(const wcstring_list_t &vals) { + if (vals.empty()) return ENV_NULL; + return join_strings(vals, ARRAY_SEP); +} + env_universal_t::env_universal_t(wcstring path) : explicit_vars_path(std::move(path)), tried_renaming(false), last_read_file(kInvalidFileID) {} @@ -448,8 +463,8 @@ bool env_universal_t::write_to_fd(int fd, const wcstring &path) { // variable; soldier on. const wcstring &key = iter->first; const env_var_t &var = iter->second; - append_file_entry(var.exports() ? SET_EXPORT : SET, key, var.as_string(), &contents, - &storage); + append_file_entry(var.exports() ? SET_EXPORT : SET, key, encode_serialized(var.as_list()), + &contents, &storage); // Go to next. ++iter; diff --git a/tests/set.err b/tests/set.err index 9f4b92c4d..196a395a2 100644 --- a/tests/set.err +++ b/tests/set.err @@ -17,3 +17,6 @@ $argle bargle: invalid var name #################### # Setting local scope when no local scope of the var uses the closest scope + +#################### +# Exporting works diff --git a/tests/set.in b/tests/set.in index 8893210d3..0fdcbe2eb 100644 --- a/tests/set.in +++ b/tests/set.in @@ -54,3 +54,9 @@ begin set -l -a var6 mno set --show var6 end + +logmsg Exporting works +set -x TESTVAR0 +set -x TESTVAR1 a +set -x TESTVAR2 a b +env | grep TESTVAR | cat -v diff --git a/tests/set.out b/tests/set.out index 29e8bf82c..3d4535bab 100644 --- a/tests/set.out +++ b/tests/set.out @@ -100,3 +100,9 @@ $var6[1]: length=3 value=|ghi| $var6[2]: length=3 value=|jkl| $var6: not set in universal scope + +#################### +# Exporting works +TESTVAR0= +TESTVAR1=a +TESTVAR2=a^^b