Allow binding nul (zero byte)

This sequence can be generatd by control-spacebar. Allow it to be bound
properly.

To do this we must be sure that we never round-trip the key sequence
through a C string.

src/builtin_bind.cpp

@@ -150,7 +150,7 @@ void builtin_bind_t::function_names(io_streams_t &streams) {
 }
 
 /// Wraps input_terminfo_get_sequence(), appending the correct error messages as needed.
-bool builtin_bind_t::get_terminfo_sequence(const wchar_t *seq, wcstring *out_seq,
+bool builtin_bind_t::get_terminfo_sequence(const wcstring &seq, wcstring *out_seq,
                                            io_streams_t &streams) {
     if (input_terminfo_get_sequence(seq, out_seq)) {
         return true;
@@ -173,13 +173,13 @@ bool builtin_bind_t::get_terminfo_sequence(const wchar_t *seq, wcstring *out_seq
 }
 
 /// Add specified key binding.
-bool builtin_bind_t::add(const wchar_t *seq, const wchar_t *const *cmds, size_t cmds_len,
+bool builtin_bind_t::add(const wcstring &seq, const wchar_t *const *cmds, size_t cmds_len,
                          const wchar_t *mode, const wchar_t *sets_mode, bool terminfo, bool user,
                          io_streams_t &streams) {
     if (terminfo) {
         wcstring seq2;
         if (get_terminfo_sequence(seq, &seq2, streams)) {
-            input_mappings_->add(seq2.c_str(), cmds, cmds_len, mode, sets_mode, user);
+            input_mappings_->add(seq2, cmds, cmds_len, mode, sets_mode, user);
         } else {
             return true;
         }

src/builtin_bind.h

@@ -26,11 +26,11 @@ class builtin_bind_t {
     void list(const wchar_t *bind_mode, bool user, io_streams_t &streams);
     void key_names(bool all, io_streams_t &streams);
     void function_names(io_streams_t &streams);
-    bool add(const wchar_t *seq, const wchar_t *const *cmds, size_t cmds_len, const wchar_t *mode,
+    bool add(const wcstring &seq, const wchar_t *const *cmds, size_t cmds_len, const wchar_t *mode,
              const wchar_t *sets_mode, bool terminfo, bool user, io_streams_t &streams);
     bool erase(wchar_t **seq, bool all, const wchar_t *mode, bool use_terminfo, bool user,
                io_streams_t &streams);
-    bool get_terminfo_sequence(const wchar_t *seq, wcstring *out_seq, io_streams_t &streams);
+    bool get_terminfo_sequence(const wcstring &seq, wcstring *out_seq, io_streams_t &streams);
     bool insert(int optind, int argc, wchar_t **argv, io_streams_t &streams);
     void list_modes(io_streams_t &streams);
     bool list_one(const wcstring &seq, const wcstring &bind_mode, bool user, io_streams_t &streams);

src/fish_key_reader.cpp

@@ -67,29 +67,30 @@ static bool should_exit(wchar_t wc) {
 }
 
 /// Return the name if the recent sequence of characters matches a known terminfo sequence.
-static char *sequence_name(wchar_t wc) {
-    unsigned char c = wc < 0x80 ? wc : 0;
-    static char recent_chars[8] = {0};
-
-    recent_chars[0] = recent_chars[1];
-    recent_chars[1] = recent_chars[2];
-    recent_chars[2] = recent_chars[3];
-    recent_chars[3] = recent_chars[4];
-    recent_chars[4] = recent_chars[5];
-    recent_chars[5] = recent_chars[6];
-    recent_chars[6] = recent_chars[7];
-    recent_chars[7] = c;
-
-    for (int idx = 7; idx >= 0; idx--) {
-        wcstring out_name;
-        wcstring seq = str2wcstring(recent_chars + idx, 8 - idx);
-        bool found = input_terminfo_get_name(seq, &out_name);
-        if (found) {
-            return strdup(wcs2string(out_name).c_str());
-        }
+static maybe_t<wcstring> sequence_name(wchar_t wc) {
+    static std::string recent_chars;
+    if (wc >= 0x80) {
+        // Terminfo sequences are always ASCII.
+        recent_chars.clear();
+        return none();
     }
 
-    return NULL;
+    unsigned char c = wc;
+    recent_chars.push_back(c);
+    while (recent_chars.size() > 8) {
+        recent_chars.erase(recent_chars.begin());
+    }
+
+    // Check all nonempty substrings extending to the end.
+    for (size_t i = 0; i < recent_chars.size(); i++) {
+        wcstring out_name;
+        wcstring seq = str2wcstring(recent_chars.substr(i));
+        if (input_terminfo_get_name(seq, &out_name)) {
+            return out_name;
+        }
+    }
+    return none();
+    ;
 }
 
 /// Return true if the character must be escaped when used in the sequence of chars to be bound in
@@ -174,10 +175,8 @@ static void output_info_about_char(wchar_t wc) {
 }
 
 static bool output_matching_key_name(wchar_t wc) {
-    char *name = sequence_name(wc);
-    if (name) {
-        std::fwprintf(stdout, L"bind -k %s 'do something'\n", name);
-        free(name);
+    if (maybe_t<wcstring> name = sequence_name(wc)) {
+        std::fwprintf(stdout, L"bind -k %ls 'do something'\n", name->c_str());
         return true;
     }
     return false;
@@ -223,7 +222,12 @@ static void process_input(bool continuous_mode) {
 
         wchar_t wc = evt.get_char();
         prev_tstamp = output_elapsed_time(prev_tstamp, first_char_seen);
-        add_char_to_bind_command(wc, bind_chars);
+        // Hack for #3189. Do not suggest \c@ as the binding for nul, because a string containing
+        // nul cannot be passed to builtin_bind since it uses C strings. We'll output the name of
+        // this key (nul) elsewhere.
+        if (wc) {
+            add_char_to_bind_command(wc, bind_chars);
+        }
         output_info_about_char(wc);
         if (output_matching_key_name(wc)) {
             output_bind_command(bind_chars);

src/input.cpp

@@ -34,6 +34,9 @@
 
 #define MAX_INPUT_FUNCTION_ARGS 20
 
+/// A name for our own key mapping for nul.
+static const wchar_t *k_nul_mapping_name = L"nul";
+
 /// Struct representing a keybinding. Returned by input_get_mappings.
 struct input_mapping_t {
     /// Character sequence which generates this event.
@@ -59,8 +62,17 @@ struct input_mapping_t {
 
 /// A struct representing the mapping from a terminfo key name to a terminfo character sequence.
 struct terminfo_mapping_t {
-    const wchar_t *name;  // name of key
-    const char *seq;      // character sequence generated on keypress
+    // name of key
+    const wchar_t *name;
+
+    // character sequence generated on keypress, or none if there was no mapping.
+    maybe_t<std::string> seq;
+
+    terminfo_mapping_t(const wchar_t *name, const char *s) : name(name) {
+        if (s) seq.emplace(s);
+    }
+
+    terminfo_mapping_t(const wchar_t *name, std::string s) : name(name), seq(std::move(s)) {}
 };
 
 static constexpr size_t input_function_count = R_END_INPUT_FUNCTIONS;
@@ -159,9 +171,6 @@ acquired_lock<input_mapping_set_t> input_mappings() {
 /// Terminfo map list.
 static latch_t<std::vector<terminfo_mapping_t>> s_terminfo_mappings;
 
-#define TERMINFO_ADD(key) \
-    { (L## #key) + 4, key }
-
 /// \return the input terminfo.
 static std::vector<terminfo_mapping_t> create_input_terminfo();
 
@@ -211,10 +220,10 @@ static void input_mapping_insert_sorted(mapping_list_t &ml, input_mapping_t new_
 }
 
 /// Adds an input mapping.
-void input_mapping_set_t::add(const wchar_t *sequence, const wchar_t *const *commands,
+void input_mapping_set_t::add(wcstring sequence, const wchar_t *const *commands,
                               size_t commands_len, const wchar_t *mode, const wchar_t *sets_mode,
                               bool user) {
-    assert(sequence && commands && mode && sets_mode && "Null parameter");
+    assert(commands && mode && sets_mode && "Null parameter");
 
     // Clear cached mappings.
     all_mappings_cache_.reset();
@@ -233,13 +242,14 @@ void input_mapping_set_t::add(const wchar_t *sequence, const wchar_t *const *com
     }
 
     // Add a new mapping, using the next order.
-    const input_mapping_t new_mapping = input_mapping_t(sequence, commands_vector, mode, sets_mode);
+    input_mapping_t new_mapping =
+        input_mapping_t(std::move(sequence), commands_vector, mode, sets_mode);
     input_mapping_insert_sorted(ml, std::move(new_mapping));
 }
 
-void input_mapping_set_t::add(const wchar_t *sequence, const wchar_t *command, const wchar_t *mode,
+void input_mapping_set_t::add(wcstring sequence, const wchar_t *command, const wchar_t *mode,
                               const wchar_t *sets_mode, bool user) {
-    input_mapping_set_t::add(sequence, &command, 1, mode, sets_mode, user);
+    input_mapping_set_t::add(std::move(sequence), &command, 1, mode, sets_mode, user);
 }
 
 /// Handle interruptions to key reading by reaping finished jobs and propagating the interrupt to
@@ -587,6 +597,10 @@ std::shared_ptr<const mapping_list_t> input_mapping_set_t::all_mappings() {
 static std::vector<terminfo_mapping_t> create_input_terminfo() {
     assert(curses_initialized);
     if (!cur_term) return {};  // setupterm() failed so we can't referency any key definitions
+
+#define TERMINFO_ADD(key) \
+    { (L## #key) + 4, key }
+
     return {
         TERMINFO_ADD(key_a1), TERMINFO_ADD(key_a3), TERMINFO_ADD(key_b2),
             TERMINFO_ADD(key_backspace), TERMINFO_ADD(key_beg), TERMINFO_ADD(key_btab),
@@ -670,22 +684,26 @@ static std::vector<terminfo_mapping_t> create_input_terminfo() {
             TERMINFO_ADD(key_sr), TERMINFO_ADD(key_sredo), TERMINFO_ADD(key_sreplace),
             TERMINFO_ADD(key_sright), TERMINFO_ADD(key_srsume), TERMINFO_ADD(key_ssave),
             TERMINFO_ADD(key_ssuspend), TERMINFO_ADD(key_stab), TERMINFO_ADD(key_sundo),
-            TERMINFO_ADD(key_suspend), TERMINFO_ADD(key_undo), TERMINFO_ADD(key_up)
+            TERMINFO_ADD(key_suspend), TERMINFO_ADD(key_undo), TERMINFO_ADD(key_up),
+
+            // We introduce our own name for the string containing only the nul character - see
+            // #3189. This can typically be generated via control-space.
+            terminfo_mapping_t(k_nul_mapping_name, std::string{'\0'})
     };
+#undef TERMINFO_ADD
 }
 
-bool input_terminfo_get_sequence(const wchar_t *name, wcstring *out_seq) {
+bool input_terminfo_get_sequence(const wcstring &name, wcstring *out_seq) {
     ASSERT_IS_MAIN_THREAD();
     assert(s_input_initialized);
-    assert(name && "null name");
     for (const terminfo_mapping_t &m : *s_terminfo_mappings) {
-        if (!std::wcscmp(name, m.name)) {
+        if (name == m.name) {
             // Found the mapping.
             if (!m.seq) {
                 errno = EILSEQ;
                 return false;
             } else {
-                *out_seq = str2wcstring(m.seq);
+                *out_seq = str2wcstring(*m.seq);
                 return true;
             }
         }
@@ -698,12 +716,7 @@ bool input_terminfo_get_name(const wcstring &seq, wcstring *out_name) {
     assert(s_input_initialized);
 
     for (const terminfo_mapping_t &m : *s_terminfo_mappings) {
-        if (!m.seq) {
-            continue;
-        }
-
-        const wcstring map_buf = format_string(L"%s", m.seq);
-        if (map_buf == seq) {
+        if (m.seq && seq == str2wcstring(*m.seq)) {
             out_name->assign(m.name);
             return true;
         }

src/input.h

@@ -109,11 +109,10 @@ class input_mapping_set_t {
     ///
     /// \param sequence the sequence to bind
     /// \param command an input function that will be run whenever the key sequence occurs
-    void add(const wchar_t *sequence, const wchar_t *command,
-             const wchar_t *mode = DEFAULT_BIND_MODE, const wchar_t *new_mode = DEFAULT_BIND_MODE,
-             bool user = true);
+    void add(wcstring sequence, const wchar_t *command, const wchar_t *mode = DEFAULT_BIND_MODE,
+             const wchar_t *new_mode = DEFAULT_BIND_MODE, bool user = true);
 
-    void add(const wchar_t *sequence, const wchar_t *const *commands, size_t commands_len,
+    void add(wcstring sequence, const wchar_t *const *commands, size_t commands_len,
              const wchar_t *mode = DEFAULT_BIND_MODE, const wchar_t *new_mode = DEFAULT_BIND_MODE,
              bool user = true);
 
@@ -128,7 +127,7 @@ acquired_lock<input_mapping_set_t> input_mappings();
 ///
 /// If no terminfo variable of the specified name could be found, return false and set errno to
 /// ENOENT. If the terminfo variable does not have a value, return false and set errno to EILSEQ.
-bool input_terminfo_get_sequence(const wchar_t *name, wcstring *out_seq);
+bool input_terminfo_get_sequence(const wcstring &name, wcstring *out_seq);
 
 /// Return the name of the terminfo variable with the specified sequence, in out_name. Returns true
 /// if found, false if not found.

tests/bind.expect

@@ -288,3 +288,14 @@ expect_prompt -re {git@} {
 } unmatched {
     puts stderr "ctrl-w does not stop at @"
 }
+
+# Ensure that nul can be bound properly (#3189).
+send "bind -k nul 'echo nul seen'\r"
+expect_prompt
+send -null 3
+send "\r"
+expect_prompt -re {nul seen\r\nnul seen\r\nnul seen} {
+    puts "nul seen"
+} unmatched {
+    puts stderr "nul not seen"
+}

tests/bind.expect.out

@@ -22,3 +22,4 @@ ctrl-v seen
 ctrl-o seen
 ctrl-w stops at :
 ctrl-w stops at @
+nul seen

tests/fkr.expect

@@ -31,7 +31,7 @@ expect -ex "char: \\u1234\r\nbind \\e\\u1234 'do something'\r\n" {
 # Is a NULL char echoed correctly?
 sleep 0.020
 send -null
-expect -ex "char: \\c@\r\nbind \\c@ 'do something'\r\n" {
+expect -ex "char: \\c@\r\nbind -k nul 'do something'\r\n" {
     puts "\\c@ handled"
 } unmatched {
     puts stderr "\\c@ not handled"