correct escape_string corner case

Fixes #3892

src/builtin_string.cpp

@@ -137,7 +137,7 @@ static int string_escape(parser_t &parser, io_streams_t &streams, int argc, wcha
     wcstring storage;
     const wchar_t *arg;
     while ((arg = string_get_arg(&i, argv, &storage, streams)) != 0) {
-        streams.out.append(escape(arg, flags));
+        streams.out.append(escape_string(arg, flags));
         streams.out.append(L'\n');
         nesc++;
     }

src/common.cpp

@@ -852,7 +852,8 @@ static void escape_string_internal(const wchar_t *orig_in, size_t in_len, wcstri
                 case L'\\':
                 case L'\'': {
                     need_escape = need_complex_escape = 1;
-                    if (escape_all) out += L'\\';
+                    //WTF if (escape_all) out += L'\\';
+                    out += L'\\';
                     out += *in;
                     break;
                 }
@@ -937,7 +938,7 @@ static void escape_string_internal(const wchar_t *orig_in, size_t in_len, wcstri
     }
 }
 
-wcstring escape(const wchar_t *in, escape_flags_t flags) {
+wcstring escape_string(const wchar_t *in, escape_flags_t flags) {
     wcstring result;
     escape_string_internal(in, wcslen(in), &result, flags);
     return result;

src/common.h

@@ -94,7 +94,7 @@ enum {
 };
 typedef unsigned int unescape_flags_t;
 
-// Flags for the escape() and escape_string() functions.
+// Flags for the escape_string() and escape_string() functions.
 enum {
     /// Escape all characters, including magic characters like the semicolon.
     ESCAPE_ALL = 1 << 0,
@@ -715,7 +715,7 @@ ssize_t read_loop(int fd, void *buff, size_t count);
 /// \param in The string to be escaped
 /// \param flags Flags to control the escaping
 /// \return The escaped string
-wcstring escape(const wchar_t *in, escape_flags_t flags);
+wcstring escape_string(const wchar_t *in, escape_flags_t flags);
 wcstring escape_string(const wcstring &in, escape_flags_t flags);
 
 /// Expand backslashed escapes and substitute them with their unescaped counterparts. Also

src/expand.cpp

@@ -620,7 +620,7 @@ static bool expand_pid(const wcstring &instr_with_sep, expand_flags_t flags,
     if (prev_count == out->size() && !(flags & EXPAND_FOR_COMPLETIONS)) {
         // We failed to find anything.
         append_syntax_error(errors, 1, FAILED_EXPANSION_PROCESS_ERR_MSG,
-                            escape(in + 1, ESCAPE_NO_QUOTED).c_str());
+                            escape_string(in + 1, ESCAPE_NO_QUOTED).c_str());
         return false;
     }
 

src/fish_tests.cpp

@@ -296,6 +296,7 @@ static void test_escape_crazy() {
     wcstring random_string;
     wcstring escaped_string;
     wcstring unescaped_string;
+    bool unescaped_success;
     for (size_t i = 0; i < ESCAPE_TEST_COUNT; i++) {
         random_string.clear();
         while (rand() % ESCAPE_TEST_LENGTH) {
@@ -303,8 +304,7 @@ static void test_escape_crazy() {
         }
 
         escaped_string = escape_string(random_string, ESCAPE_ALL);
-        bool unescaped_success =
+        unescaped_success = unescape_string(escaped_string, &unescaped_string, UNESCAPE_DEFAULT);
-            unescape_string(escaped_string, &unescaped_string, UNESCAPE_DEFAULT);
 
         if (!unescaped_success) {
             err(L"Failed to unescape string <%ls>", escaped_string.c_str());
@@ -313,6 +313,18 @@ static void test_escape_crazy() {
                 random_string.c_str(), unescaped_string.c_str());
         }
     }
+
+    // Verify that not using `ESCAPE_ALL` also escapes backslashes so we don't regress on issue
+    // #3892.
+    random_string = L"line 1\\n\nline 2";
+    escaped_string = escape_string(random_string, ESCAPE_NO_QUOTED);
+    unescaped_success = unescape_string(escaped_string, &unescaped_string, UNESCAPE_DEFAULT);
+    if (!unescaped_success) {
+        err(L"Failed to unescape string <%ls>", escaped_string.c_str());
+    } else if (unescaped_string != random_string) {
+        err(L"Escaped and then unescaped string '%ls', but got back a different string '%ls'",
+            random_string.c_str(), unescaped_string.c_str());
+    }
 }
 
 static void test_format(void) {

src/input.cpp

@@ -248,8 +248,9 @@ void input_mapping_add(const wchar_t *sequence, const wchar_t *const *commands,
     CHECK(mode, );
     CHECK(sets_mode, );
 
-    // debug( 0, L"Add mapping from %ls to %ls in mode %ls", escape(sequence, ESCAPE_ALL).c_str(),
+    // debug( 0, L"Add mapping from %ls to %ls in mode %ls", escape_string(sequence,
-    // escape(command, ESCAPE_ALL).c_str(), mode);
+    // ESCAPE_ALL).c_str(),
+    // escape_string(command, ESCAPE_ALL).c_str(), mode);
 
     // Remove existing mappings with this sequence.
     const wcstring_list_t commands_vector(commands, commands + commands_len);
@@ -408,7 +409,7 @@ static bool input_mapping_is_match(const input_mapping_t &m) {
     wchar_t c = 0;
     int j;
 
-    debug(2, L"trying to match mapping %ls", escape(m.seq.c_str(), ESCAPE_ALL).c_str());
+    debug(2, L"trying to match mapping %ls", escape_string(m.seq.c_str(), ESCAPE_ALL).c_str());
     const wchar_t *str = m.seq.c_str();
     for (j = 0; str[j] != L'\0'; j++) {
         bool timed = (j > 0 && iswcntrl(str[0]));
@@ -420,7 +421,8 @@ static bool input_mapping_is_match(const input_mapping_t &m) {
     }
 
     if (str[j] == L'\0') {
-        // debug(0, L"matched mapping %ls (%ls)\n", escape(m.seq.c_str(), ESCAPE_ALL).c_str(),
+        // debug(0, L"matched mapping %ls (%ls)\n", escape_string(m.seq.c_str(),
+        // ESCAPE_ALL).c_str(),
         // m.command.c_str());
         // We matched the entire sequence.
         return true;
@@ -446,7 +448,8 @@ static void input_mapping_execute_matching_or_generic(bool allow_commands) {
     for (size_t i = 0; i < mapping_list.size(); i++) {
         const input_mapping_t &m = mapping_list.at(i);
 
-        // debug(0, L"trying mapping (%ls,%ls,%ls)\n", escape(m.seq.c_str(), ESCAPE_ALL).c_str(),
+        // debug(0, L"trying mapping (%ls,%ls,%ls)\n", escape_string(m.seq.c_str(),
+        // ESCAPE_ALL).c_str(),
         //           m.mode.c_str(), m.sets_mode.c_str());
 
         if (m.mode != bind_mode) {

src/reader.cpp

@@ -1027,8 +1027,8 @@ wcstring completion_apply_to_command_line(const wcstring &val_str, complete_flag
         if (do_escape) {
             // Respect COMPLETE_DONT_ESCAPE_TILDES.
             bool no_tilde = static_cast<bool>(flags & COMPLETE_DONT_ESCAPE_TILDES);
-            wcstring escaped =
+            wcstring escaped = escape_string(
-                escape(val, ESCAPE_ALL | ESCAPE_NO_QUOTED | (no_tilde ? ESCAPE_NO_TILDE : 0));
+                val, ESCAPE_ALL | ESCAPE_NO_QUOTED | (no_tilde ? ESCAPE_NO_TILDE : 0));
             sb.append(escaped);
             move_cursor = escaped.size();
         } else {