From 2d0b2e97d00c5ca08a6cbd4a7d724b41e1222514 Mon Sep 17 00:00:00 2001 From: Jacob Nevins Date: Mon, 6 Mar 2017 10:36:26 +0000 Subject: [PATCH] Restore ability to not send SSH terminal modes. 2ce0b680c inadvertently removed this ability in trying to ensure that everyone got the new IUTF8 mode by default; you could remove a mode from the list in the UI, but this would just revert PuTTY to its default. The UI and storage have been revamped; the storage format now explicitly says when a mode is not to be sent, and the configuration UI always shows all modes known to PuTTY; if a mode is not to be sent it now shows up as "(don't send)" in the list. Old saved settings are migrated so as to preserve previous removals of longstanding modes, while automatically adding IUTF8. (In passing, this removes a bug where pressing the 'Remove' button of the previous UI would populate the value edit box with garbage.) --- config.c | 133 +++++++++++++++++++++---------------------------- doc/config.but | 42 ++++++++-------- settings.c | 49 +++++++++++++++++- ssh.c | 21 ++++---- 4 files changed, 137 insertions(+), 108 deletions(-) diff --git a/config.c b/config.c index 59d01e74..220c1aa8 100644 --- a/config.c +++ b/config.c @@ -946,8 +946,7 @@ static void colour_handler(union control *ctrl, void *dlg, } struct ttymodes_data { - union control *modelist, *valradio, *valbox; - union control *addbutton, *rembutton, *listbox; + union control *valradio, *valbox, *setbutton, *listbox; }; static void ttymodes_handler(union control *ctrl, void *dlg, @@ -966,69 +965,67 @@ static void ttymodes_handler(union control *ctrl, void *dlg, val != NULL; val = conf_get_str_strs(conf, CONF_ttymodes, key, &key)) { char *disp = dupprintf("%s\t%s", key, - (val[0] == 'A') ? "(auto)" : val+1); + (val[0] == 'A') ? "(auto)" : + ((val[0] == 'N') ? "(don't send)" + : val+1)); dlg_listbox_add(ctrl, dlg, disp); sfree(disp); } dlg_update_done(ctrl, dlg); - } else if (ctrl == td->modelist) { - int i; - dlg_update_start(ctrl, dlg); - dlg_listbox_clear(ctrl, dlg); - for (i = 0; ttymodes[i]; i++) - dlg_listbox_add(ctrl, dlg, ttymodes[i]); - dlg_listbox_select(ctrl, dlg, 0); /* *shrug* */ - dlg_update_done(ctrl, dlg); } else if (ctrl == td->valradio) { dlg_radiobutton_set(ctrl, dlg, 0); } + } else if (event == EVENT_SELCHANGE) { + if (ctrl == td->listbox) { + int ind = dlg_listbox_index(td->listbox, dlg); + char *val; + if (ind < 0) { + return; /* no item selected */ + } + val = conf_get_str_str(conf, CONF_ttymodes, + conf_get_str_nthstrkey(conf, CONF_ttymodes, + ind)); + assert(val != NULL); + /* Do this first to defuse side-effects on radio buttons: */ + dlg_editbox_set(td->valbox, dlg, val+1); + dlg_radiobutton_set(td->valradio, dlg, + val[0] == 'A' ? 0 : (val[0] == 'N' ? 1 : 2)); + } + } else if (event == EVENT_VALCHANGE) { + if (ctrl == td->valbox) { + /* If they're editing the text box, we assume they want its + * value to be used. */ + dlg_radiobutton_set(td->valradio, dlg, 2); + } } else if (event == EVENT_ACTION) { - if (ctrl == td->addbutton) { - int ind = dlg_listbox_index(td->modelist, dlg); + if (ctrl == td->setbutton) { + int ind = dlg_listbox_index(td->listbox, dlg); + const char *key; + char *str, *val; + char type; + + { + const char *types = "ANV"; + int button = dlg_radiobutton_get(td->valradio, dlg); + assert(button >= 0 && button < lenof(types)); + type = types[button]; + } + + /* Construct new entry */ if (ind >= 0) { - char type = dlg_radiobutton_get(td->valradio, dlg) ? 'V' : 'A'; - const char *key; - char *str, *val; - /* Construct new entry */ - key = ttymodes[ind]; - str = dlg_editbox_get(td->valbox, dlg); + key = conf_get_str_nthstrkey(conf, CONF_ttymodes, ind); + str = (type == 'V' ? dlg_editbox_get(td->valbox, dlg) + : dupstr("")); val = dupprintf("%c%s", type, str); sfree(str); conf_set_str_str(conf, CONF_ttymodes, key, val); sfree(val); dlg_refresh(td->listbox, dlg); - } else + dlg_listbox_select(td->listbox, dlg, ind); + } else { + /* Not a multisel listbox, so this means nothing selected */ dlg_beep(dlg); - } else if (ctrl == td->rembutton) { - int i = 0; - char *key, *val; - int multisel = dlg_listbox_index(td->listbox, dlg) < 0; - for (val = conf_get_str_strs(conf, CONF_ttymodes, NULL, &key); - val != NULL; - val = conf_get_str_strs(conf, CONF_ttymodes, key, &key)) { - if (dlg_listbox_issel(td->listbox, dlg, i)) { - if (!multisel) { - /* Populate controls with entry we're about to - * delete, for ease of editing. - * (If multiple entries were selected, don't - * touch the controls.) */ - int ind = 0; - val++; - while (ttymodes[ind]) { - if (!strcmp(ttymodes[ind], key)) - break; - ind++; - } - dlg_listbox_select(td->modelist, dlg, ind); - dlg_radiobutton_set(td->valradio, dlg, - (*val == 'V')); - dlg_editbox_set(td->valbox, dlg, val+1); - } - conf_del_str_str(conf, CONF_ttymodes, key); - } - i++; } - dlg_refresh(td->listbox, dlg); } } } @@ -2491,54 +2488,40 @@ void setup_config_box(struct controlbox *b, int midsession, "Terminal modes"); td = (struct ttymodes_data *) ctrl_alloc(b, sizeof(struct ttymodes_data)); - ctrl_columns(s, 2, 75, 25); c = ctrl_text(s, "Terminal modes to send:", HELPCTX(ssh_ttymodes)); - c->generic.column = 0; - td->rembutton = ctrl_pushbutton(s, "Remove", 'r', - HELPCTX(ssh_ttymodes), - ttymodes_handler, P(td)); - td->rembutton->generic.column = 1; - td->rembutton->generic.tabdelay = 1; - ctrl_columns(s, 1, 100); td->listbox = ctrl_listbox(s, NULL, NO_SHORTCUT, HELPCTX(ssh_ttymodes), ttymodes_handler, P(td)); - td->listbox->listbox.multisel = 1; - td->listbox->listbox.height = 4; + td->listbox->listbox.height = 8; td->listbox->listbox.ncols = 2; td->listbox->listbox.percentages = snewn(2, int); td->listbox->listbox.percentages[0] = 40; td->listbox->listbox.percentages[1] = 60; - ctrl_tabdelay(s, td->rembutton); ctrl_columns(s, 2, 75, 25); - td->modelist = ctrl_droplist(s, "Mode:", 'm', 67, - HELPCTX(ssh_ttymodes), - ttymodes_handler, P(td)); - td->modelist->generic.column = 0; - td->addbutton = ctrl_pushbutton(s, "Add", 'd', + c = ctrl_text(s, "For selected mode, send:", HELPCTX(ssh_ttymodes)); + c->generic.column = 0; + td->setbutton = ctrl_pushbutton(s, "Set", 's', HELPCTX(ssh_ttymodes), ttymodes_handler, P(td)); - td->addbutton->generic.column = 1; - td->addbutton->generic.tabdelay = 1; + td->setbutton->generic.column = 1; + td->setbutton->generic.tabdelay = 1; ctrl_columns(s, 1, 100); /* column break */ /* Bit of a hack to get the value radio buttons and * edit-box on the same row. */ - ctrl_columns(s, 3, 25, 50, 25); - c = ctrl_text(s, "Value:", HELPCTX(ssh_ttymodes)); - c->generic.column = 0; - td->valradio = ctrl_radiobuttons(s, NULL, NO_SHORTCUT, 2, + ctrl_columns(s, 2, 75, 25); + td->valradio = ctrl_radiobuttons(s, NULL, NO_SHORTCUT, 3, HELPCTX(ssh_ttymodes), ttymodes_handler, P(td), "Auto", NO_SHORTCUT, P(NULL), + "Nothing", NO_SHORTCUT, P(NULL), "This:", NO_SHORTCUT, P(NULL), NULL); - td->valradio->generic.column = 1; + td->valradio->generic.column = 0; td->valbox = ctrl_editbox(s, NULL, NO_SHORTCUT, 100, HELPCTX(ssh_ttymodes), ttymodes_handler, P(td), P(NULL)); - td->valbox->generic.column = 2; - ctrl_tabdelay(s, td->addbutton); - + td->valbox->generic.column = 1; + ctrl_tabdelay(s, td->setbutton); } if (!midsession) { diff --git a/doc/config.but b/doc/config.but index 04c9cd70..eb96fc17 100644 --- a/doc/config.but +++ b/doc/config.but @@ -2934,24 +2934,17 @@ the remote pseudo-terminal. These usually control the server's expectation of the local terminal's behaviour. If your server does not have sensible defaults for these modes, you -may find that changing them here helps. If you don't understand any of -this, it's safe to leave these settings alone. +may find that changing them here helps, although the server is at +liberty to ignore your changes. If you don't understand any of this, +it's safe to leave these settings alone. (None of these settings will have any effect if no pseudo-terminal is requested or allocated.) -You can add or modify a mode by selecting it from the drop-down list, -choosing whether it's set automatically or to a specific value with -the radio buttons and edit box, and hitting \q{Add}. A mode (or -several) can be removed from the list by selecting them and hitting -\q{Remove}. The effect of the mode list is as follows: - -\b If a mode is not on the list, it will not be specified to the -server under any circumstances. - -\b If a mode is on the list: - -\lcont{ +You can change what happens for a particular mode by selecting it in +the list, choosing one of the options and specifying the exact value +if necessary, and hitting \q{Set}. The effect of the options is as +follows: \b If the \q{Auto} option is selected, the PuTTY tools will decide whether to specify that mode to the server, and if so, will send @@ -2966,12 +2959,13 @@ modes from the local terminal, if any. } +\b If \q{Nothing} is selected, no value for the mode will not be +specified to the server under any circumstances. + \b If a value is specified, it will be sent to the server under all circumstances. The precise syntax of the value box depends on the mode. -} - By default, all of the available modes are listed as \q{Auto}, which should do the right thing in most circumstances. @@ -3008,18 +3002,22 @@ character or turn it off entirely. \b Boolean modes such as \cw{ECHO} and \cw{ICANON} can be specified in PuTTY in a variety of ways, such as \cw{true}/\cw{false}, -\cw{yes}/\cw{no}, and \cw{0}/\cw{1}. +\cw{yes}/\cw{no}, and \cw{0}/\cw{1}. (Explicitly specifying a value of +\cw{no} is different from not sending the mode at all.) \b The boolean mode \I{IUTF8 terminal mode}\cw{IUTF8} signals to the server whether the terminal character set is \i{UTF-8} or not. -If this is set incorrectly, actions like backspace may behave -incorrectly in some circumstances. However, setting this is not usually +If this is set incorrectly, keys like backspace may do the wrong thing +in some circumstances. However, setting this is not usually sufficient to cause servers to expect the terminal to be in UTF-8 mode; POSIX servers will generally require the locale to be set (by some server-dependent means), although many default to UTF-8. Also, -\#{circa 2016} many servers (particularly older servers) do not honour -this mode sent over SSH. When set to \q{Auto}, this follows the -local configured character set (see \k{config-charset}). +since this mode was added to the SSH protocol much later than the +others, \#{circa 2016} many servers (particularly older servers) do +not honour this mode sent over SSH; indeed, a few poorly-written +servers object to its mere presence, so you may find you need to set +it to not be sent at all. When set to \q{Auto}, this follows the local +configured character set (see \k{config-charset}). \b Terminal speeds are configured elsewhere; see \k{config-termspeed}. diff --git a/settings.c b/settings.c index 72a7c4ff..f810d3f9 100644 --- a/settings.c +++ b/settings.c @@ -46,6 +46,9 @@ static const struct keyvalwhere hknames[] = { * This is currently precisely the same as the set in ssh.c, but could * in principle differ if other backends started to support tty modes * (e.g., the pty backend). + * The set of modes in in this array is currently significant for + * settings migration from old versions; if they change, review the + * gppmap() invocation for "TerminalModes". */ const char *const ttymodes[] = { "INTR", "QUIT", "ERASE", "KILL", "EOF", @@ -748,7 +751,51 @@ void load_open_settings(void *sesskey, Conf *conf) gppi(sesskey, "TCPKeepalives", 0, conf, CONF_tcp_keepalives); gpps(sesskey, "TerminalType", "xterm", conf, CONF_termtype); gpps(sesskey, "TerminalSpeed", "38400,38400", conf, CONF_termspeed); - if (!gppmap(sesskey, "TerminalModes", conf, CONF_ttymodes)) { + if (gppmap(sesskey, "TerminalModes", conf, CONF_ttymodes)) { + /* + * Backwards compatibility with old saved settings. + * + * From the invention of this setting through 0.67, the set of + * terminal modes was fixed, and absence of a mode from this + * setting meant the user had explicitly removed it from the + * UI and we shouldn't send it. + * + * In 0.68, the IUTF8 mode was added, and in handling old + * settings we inadvertently removed the ability to not send + * a mode. Any mode not mentioned was treated as if it was + * set to 'auto' (A). + * + * After 0.68, we added explicit notation to the setting format + * when the user removes a known terminal mode from the list. + * + * So: if any of the modes from the original set is missing, we + * assume this was an intentional removal by the user and add + * an explicit removal ('N'); but if IUTF8 (or any other mode + * added after 0.67) is missing, we assume that its absence is + * due to the setting being old rather than intentional, and + * add it with its default setting. + * + * (This does mean that if a 0.68 user explicitly removed IUTF8, + * we add it back; but removing IUTF8 had no effect in 0.68, so + * we're preserving behaviour, which is the best we can do.) + */ + for (i = 0; ttymodes[i]; i++) { + if (!conf_get_str_str_opt(conf, CONF_ttymodes, ttymodes[i])) { + /* Mode not mentioned in setting. */ + const char *def; + if (!strcmp(ttymodes[i], "IUTF8")) { + /* Any new modes we add in future should be treated + * this way too. */ + def = "A"; /* same as new-setting default below */ + } else { + /* One of the original modes. Absence is probably + * deliberate. */ + def = "N"; /* don't send */ + } + conf_set_str_str(conf, CONF_ttymodes, ttymodes[i], def); + } + } + } else { /* This hardcodes a big set of defaults in any new saved * sessions. Let's hope we don't change our mind. */ for (i = 0; ttymodes[i]; i++) diff --git a/ssh.c b/ssh.c index 4de741c7..693f52d8 100644 --- a/ssh.c +++ b/ssh.c @@ -1036,20 +1036,20 @@ static void parse_ttymodes(Ssh ssh, int i; const struct ssh_ttymode *mode; char *val; - char default_val[2]; - - strcpy(default_val, "A"); for (i = 0; i < lenof(ssh_ttymodes); i++) { mode = ssh_ttymodes + i; - val = conf_get_str_str_opt(ssh->conf, CONF_ttymodes, mode->mode); - if (!val) - val = default_val; + /* Every mode known to the current version of the code should be + * mentioned; this was ensured when settings were loaded. */ + val = conf_get_str_str(ssh->conf, CONF_ttymodes, mode->mode); /* - * val[0] is either 'V', indicating that an explicit value - * follows it, or 'A' indicating that we should pass the - * value through from the local environment via get_ttymode. + * val[0] can be + * - 'V', indicating that an explicit value follows it; + * - 'A', indicating that we should pass the value through from + * the local environment via get_ttymode; or + * - 'N', indicating that we should explicitly not send this + * mode. */ if (val[0] == 'A') { val = get_ttymode(ssh->frontend, mode->mode); @@ -1057,8 +1057,9 @@ static void parse_ttymodes(Ssh ssh, do_mode(data, mode, val); sfree(val); } - } else + } else if (val[0] == 'V') { do_mode(data, mode, val + 1); /* skip the 'V' */ + } /* else 'N', or something from the future we don't understand */ } }