From 02699d1acc4890b74c1098fa8bebdc2126c788d4 Mon Sep 17 00:00:00 2001
From: Fabian Homborg <FHomborg@gmail.com>
Date: Sun, 14 Mar 2021 20:03:56 +0100
Subject: [PATCH] Reject empty variable names

This allowed `set "" foo`, which is bogus and results in an unusable variable.
---
 src/builtin_set.cpp   |  5 +++--
 src/common.cpp        |  3 ++-
 tests/checks/set.fish | 20 +++++++++++++++++++-
 3 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/src/builtin_set.cpp b/src/builtin_set.cpp
index 7c76dafe6..5f64349b7 100644
--- a/src/builtin_set.cpp
+++ b/src/builtin_set.cpp
@@ -634,8 +634,9 @@ static int builtin_set_show(const wchar_t *cmd, const set_cmd_opts_t &opts, int
             wchar_t *arg = argv[i];
 
             if (!valid_var_name(arg)) {
-                streams.err.append_format(_(L"$%ls: invalid var name\n"), arg);
-                continue;
+                streams.err.append_format(BUILTIN_ERR_VARNAME, cmd, arg);
+                builtin_print_error_trailer(parser, streams.err, cmd);
+                return STATUS_INVALID_ARGS;
             }
 
             if (std::wcschr(arg, L'[')) {
diff --git a/src/common.cpp b/src/common.cpp
index a1b3125cf..e58d1f67d 100644
--- a/src/common.cpp
+++ b/src/common.cpp
@@ -1881,10 +1881,11 @@ bool valid_var_name_char(wchar_t chr) { return fish_iswalnum(chr) || chr == L'_'
 /// Test if the given string is a valid variable name.
 bool valid_var_name(const wcstring &str) {
     // Note do not use c_str(), we want to fail on embedded nul bytes.
-    return std::all_of(str.begin(), str.end(), valid_var_name_char);
+    return !str.empty() && std::all_of(str.begin(), str.end(), valid_var_name_char);
 }
 
 bool valid_var_name(const wchar_t *str) {
+    if (str[0] == L'\0') return false;
     for (size_t i = 0; str[i] != L'\0'; i++) {
         if (!valid_var_name_char(str[i])) return false;
     }
diff --git a/tests/checks/set.fish b/tests/checks/set.fish
index e586aff53..2e05e629c 100644
--- a/tests/checks/set.fish
+++ b/tests/checks/set.fish
@@ -504,7 +504,11 @@ sh -c "EDITOR='vim -g' $FISH -c "'\'set -S EDITOR\'' | string match -r -e 'globa
 
 # Verify behavior of `set --show` given an invalid var name
 set --show 'argle bargle'
-#CHECKERR: $argle bargle: invalid var name
+#CHECKERR: set: Variable name 'argle bargle' is not valid. See `help identifiers`.
+#CHECKERR: {{.*}}set.fish (line {{\d+}}):
+#CHECKERR: set --show 'argle bargle'
+#CHECKERR: ^
+#CHECKERR: (Type 'help set' for related documentation)
 
 # Verify behavior of `set --show`
 set semiempty ''
@@ -690,3 +694,17 @@ echo $status
 #CHECK: 255
 
 true
+
+set "" foo
+#CHECKERR: set: Variable name '' is not valid. See `help identifiers`.
+#CHECKERR: {{.*}}set.fish (line {{\d+}}): 
+#CHECKERR: set "" foo
+#CHECKERR: ^
+#CHECKERR: (Type 'help set' for related documentation)
+
+set --show ""
+#CHECKERR: set: Variable name '' is not valid. See `help identifiers`.
+#CHECKERR: {{.*}}set.fish (line {{\d+}}): 
+#CHECKERR: set --show ""
+#CHECKERR: ^
+#CHECKERR: (Type 'help set' for related documentation)