From b8d40c7ae8e515d3b74ae30348f1d702f61bb057 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Tue, 13 Sep 2022 08:29:12 +0100 Subject: [PATCH 01/46] Checklist updates for pre-release setup. I don't know why I never bothered to write it down before, but it's a good idea to let a pre-release build actually *happen* between turning them on and updating the website to claim they exist. Also, for the first time ever, I've just sent out an announcement email for the 0.78 *pre-releases*, soliciting testing in advance of the actual release. So, add that to the checklist as well. (cherry picked from commit 1a3655013d5438f80e3a0d8de1b9f31f39134ec6) --- CHECKLST.txt | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHECKLST.txt b/CHECKLST.txt index 3d618cbd..a67395de 100644 --- a/CHECKLST.txt +++ b/CHECKLST.txt @@ -19,11 +19,17 @@ pre-releases on the website: - Edit ~/adm/puttysnap.sh on the master machine to enable pre-release builds, by changing the 'if false' to 'if true'. + - Wait for a nightly build to run, so that the first pre-release + snapshot actually exists. + - Put the website into pre-release mode, by defining prerel_version() in components/Base.mc to return the upcoming version number. Also add a news announcement in components/news. (Previous naming convention has been to name it in the form 'X.YZ-pre.mi'.) + - Optionally: write an announcement email for the availability of + pre-releases, and send it out to . + Things to do during the branch-stabilisation period: - Go through the source (including the documentation), and the From 7398dfa3fccf9781d81d4cbc990baa7602b50398 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Tue, 13 Sep 2022 15:48:11 +0100 Subject: [PATCH 02/46] Remove a pointless allocation. The char buffer 'blob' allocated in read_blob was never actually used for anything, so there was no need to allocate it - and also no need for the assert() just before it, which was added in commit 63a58759b5c0c11 to be extra safe against integer overflow when computing the buffer size. I feel a bit silly for having added that assertion in the first place rather than removing the allocation it was protecting! It was unnecessary even then, and I completely failed to notice :-) (cherry picked from commit c780cffb7a6f1c972f5629e6e4976c5616ca0df3) --- sshpubk.c | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/sshpubk.c b/sshpubk.c index 0648b4c4..cc44a9b4 100644 --- a/sshpubk.c +++ b/sshpubk.c @@ -516,24 +516,18 @@ static char *read_body(BinarySource *src) static bool read_blob(BinarySource *src, int nlines, BinarySink *bs) { - unsigned char *blob; char *line; int linelen; int i, j, k; /* We expect at most 64 base64 characters, ie 48 real bytes, per line. */ - assert(nlines < MAX_KEY_BLOB_LINES); - blob = snewn(48 * nlines, unsigned char); for (i = 0; i < nlines; i++) { line = read_body(src); - if (!line) { - sfree(blob); + if (!line) return false; - } linelen = strlen(line); if (linelen % 4 != 0 || linelen > 64) { - sfree(blob); sfree(line); return false; } @@ -542,14 +536,12 @@ static bool read_blob(BinarySource *src, int nlines, BinarySink *bs) k = base64_decode_atom(line + j, decoded); if (!k) { sfree(line); - sfree(blob); return false; } put_data(bs, decoded, k); } sfree(line); } - sfree(blob); return true; } From 5a50288718443632719605e83e04949cd86863e7 Mon Sep 17 00:00:00 2001 From: Jacob Nevins Date: Mon, 24 Oct 2022 12:45:17 +0100 Subject: [PATCH 03/46] authplugin-example.py: Mention documentation. (Just in case anyone's entry point is this example, and concludes they have to reverse-engineer the protocol from the script.) (cherry picked from commit 538c8fd29ce89fcb69dd7af6ec372667a1b47775) --- contrib/authplugin-example.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/contrib/authplugin-example.py b/contrib/authplugin-example.py index 395bd2c8..1fc27257 100755 --- a/contrib/authplugin-example.py +++ b/contrib/authplugin-example.py @@ -3,6 +3,8 @@ # This is a demonstration example of how to write a # keyboard-interactive authentication helper plugin using PuTTY's # protocol for involving it in SSH connection setup. +# The protocol, and the purpose of an authentication plugin, is +# fully documented in an appendix to the PuTTY manual. import io import os From fe11c1e4980d94e2448adfbebec1bf65dab56b9d Mon Sep 17 00:00:00 2001 From: Jacob Nevins Date: Mon, 24 Oct 2022 12:45:37 +0100 Subject: [PATCH 04/46] authplugin-example.py: Flush stderr. Python 3's stderr was fully-buffered when non-interactive, unlike Python 2 and more or less everything else, until 3.9 in 2020(!): https://bugs.python.org/issue13601 (It would be less faff to sys.stderr.reconfigure(line_buffering=True) at the start, but that was only added in 3.7, whereas the 'flush' argument to print() dates back to 3.3, so I chose that to minimise the risk of version dependencies getting in the way of using this as a working example.) (cherry picked from commit 329a4cdd7942c441403374c19b0de287a67d21b7) --- contrib/authplugin-example.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/contrib/authplugin-example.py b/contrib/authplugin-example.py index 1fc27257..5932ad1a 100755 --- a/contrib/authplugin-example.py +++ b/contrib/authplugin-example.py @@ -136,7 +136,8 @@ def protocol(): hostname = rd_string_utf8(msg) port = rd_uint32(msg) username = rd_string_utf8(msg) - print(f"Got hostname {hostname!r}, port {port!r}", file=sys.stderr) + print(f"Got hostname {hostname!r}, port {port!r}", file=sys.stderr, + flush=True) # Decide which protocol version we're speaking. version = min(their_version, our_max_version) @@ -281,7 +282,7 @@ def protocol(): # Demonstration write to stderr, to prove that it shows up in PuTTY's # Event Log. -print("Hello from test plugin's stderr", file=sys.stderr) +print("Hello from test plugin's stderr", file=sys.stderr, flush=True) try: protocol() From 32e5215129aa85101f6712c7cfb8552ff725b099 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Sat, 29 Oct 2022 10:52:29 +0100 Subject: [PATCH 05/46] Some checklist updates for the Windows Store. I didn't actually get these things wrong during the submission of 0.78, but I did notice that I'd forgotten to write them down. (cherry picked from commit d39bcaedbaf6a44d17aa9db81291c06bf1932333) --- CHECKLST.txt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHECKLST.txt b/CHECKLST.txt index a67395de..98a3f61c 100644 --- a/CHECKLST.txt +++ b/CHECKLST.txt @@ -247,9 +247,14 @@ locally, this is the procedure for putting it up on the web. automatically create a new submission * provide a new set of installer URLs, then click "save all" which actually uploads them + + be careful to use URLs without "latest" in the pathname! + Just copying from the links on the download page is wrong. + Change "latest" to the version number, and test-download + via those URLs to check you didn't make a typo. * change the "what's new in this release" text in the store listing * upload revised screenshots, if necessary + * update the URL in the "Applicable license terms" box + press Publish to submit that to the actual upload process - Announce the release! From 01b4fb746c83b3917e09197cea26a4ae6e4a50a4 Mon Sep 17 00:00:00 2001 From: Jacob Nevins Date: Sat, 29 Oct 2022 15:30:26 +0100 Subject: [PATCH 06/46] Docs: add a 'pdf' cmake target. We still don't build or ship a PDF PuTTY manual by default, but we may as well conveniently expose Halibut's ability to do so. (I don't guarantee the resulting PDF is particularly pretty -- some of our overlong code lines do go off the right margin currently.) (cherry picked from commit f9a8213d956fe4276a60a691f5ce5e224021834c) --- doc/CMakeLists.txt | 7 +++++++ doc/blurb.but | 1 + 2 files changed, 8 insertions(+) diff --git a/doc/CMakeLists.txt b/doc/CMakeLists.txt index 79b1ba1f..293cd6f1 100644 --- a/doc/CMakeLists.txt +++ b/doc/CMakeLists.txt @@ -109,6 +109,13 @@ if(HALIBUT AND PERL_EXECUTABLE) WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} DEPENDS ${manual_sources} ${manual_dependencies}) list(APPEND doc_outputs puttydoc.txt) + + # PDF. (We don't ship this; so it's only built on explicit request.) + add_custom_command(OUTPUT putty.pdf + COMMAND ${HALIBUT} --pdf ${manual_sources} + WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} + DEPENDS ${manual_sources} ${manual_dependencies}) + add_custom_target(pdf DEPENDS putty.pdf) endif() macro(register_manpage title section) diff --git a/doc/blurb.but b/doc/blurb.but index c68a6262..84105d0f 100644 --- a/doc/blurb.but +++ b/doc/blurb.but @@ -23,6 +23,7 @@ page.

} \cfg{winhelp-filename}{putty.hlp} \cfg{info-filename}{putty.info} \cfg{chm-filename}{putty.chm} +\cfg{pdf-filename}{putty.pdf} PuTTY is a free (MIT-licensed) Windows Telnet and SSH client. This manual documents PuTTY, and its companion utilities PSCP, PSFTP, From 5678b4c7cf07da12694d69113017c033fde079a4 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Wed, 19 Apr 2023 14:19:22 +0100 Subject: [PATCH 07/46] Windows: fix terminal hang with resize forbidden. A server attempt to resize the window (for instance via DECCOLM) when "When window is resized" was set to "Forbid resizing completely" would cause all terminal output to be suspended, due to the resize attempt never being acknowledged. (There are other code paths like this, which I've fixed for completeness, but I don't think they have any effect: the terminal filters out resize attempts to the current size before this point, and even if a server can get such a request through the SUPDUP protocol, the test for that is wrong and will never fire -- this needs fixing separately.) (cherry picked from commit ebceb8bc9494af6d93ace9f6ea84bb3d09adb8f3) --- windows/window.c | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/windows/window.c b/windows/window.c index 70548a5d..937a5f8b 100644 --- a/windows/window.c +++ b/windows/window.c @@ -1675,20 +1675,27 @@ static void wintw_request_resize(TermWin *tw, int w, int h) { const struct BackendVtable *vt; int width, height; + int resize_action = conf_get_int(conf, CONF_resize_action); + bool deny_resize = false; - /* If the window is maximized suppress resizing attempts */ - if (IsZoomed(wgs.term_hwnd)) { - if (conf_get_int(conf, CONF_resize_action) == RESIZE_TERM) { - term_resize_request_completed(term); - return; - } + /* Suppress server-originated resizing attempts if local resizing + * is disabled entirely, or if it's supposed to change + * rows/columns but the window is maximised. */ + if (resize_action == RESIZE_DISABLED + || (resize_action == RESIZE_TERM && IsZoomed(wgs.term_hwnd))) { + deny_resize = true; } - if (conf_get_int(conf, CONF_resize_action) == RESIZE_DISABLED) return; vt = backend_vt_from_proto(be_default_protocol); if (vt && vt->flags & BACKEND_RESIZE_FORBIDDEN) + deny_resize = true; + if (h == term->rows && w == term->cols) deny_resize = true; + + /* We still need to acknowledge a suppressed resize attempt. */ + if (deny_resize) { + term_resize_request_completed(term); return; - if (h == term->rows && w == term->cols) return; + } /* Sanity checks ... */ { @@ -1709,8 +1716,7 @@ static void wintw_request_resize(TermWin *tw, int w, int h) } } - if (conf_get_int(conf, CONF_resize_action) != RESIZE_FONT && - !IsZoomed(wgs.term_hwnd)) { + if (resize_action != RESIZE_FONT && !IsZoomed(wgs.term_hwnd)) { width = extra_width + font_width * w; height = extra_height + font_height * h; From 4d888d1ff45d92f7abe29d3943816fe6670f59a3 Mon Sep 17 00:00:00 2001 From: Jacob Nevins Date: Wed, 2 Nov 2022 23:58:23 +0000 Subject: [PATCH 08/46] Docs: fix typo in SUPDUP section. (cherry picked from commit f78a1a944ff20f39dc157c5dd0c5d609943e5a28) --- doc/config.but | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/config.but b/doc/config.but index f258a356..9770697d 100644 --- a/doc/config.but +++ b/doc/config.but @@ -4071,7 +4071,7 @@ at the bottom of the screen, until a space is typed. \S{supdup-scroll} \q{Terminal scrolling} -This controls whether the terminal will perform scrolling then the +This controls whether the terminal will perform scrolling when the cursor goes below the last line, or if the cursor will return to the first line. From b760a2a040f7fcb043d6a8d21ed25d0dd9379818 Mon Sep 17 00:00:00 2001 From: Jacob Nevins Date: Sat, 5 Nov 2022 23:55:13 +0000 Subject: [PATCH 09/46] Use correct date in cert check error. When a host certificate was used outside its valid date range, we were displaying the current time where we meant to show the relevant bound of the validity range. (cherry picked from commit 68db3d195d00e3f3904a7e004a4efd00fd303efb) --- crypto/openssh-certs.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/crypto/openssh-certs.c b/crypto/openssh-certs.c index cf0c2af3..4cd984e8 100644 --- a/crypto/openssh-certs.c +++ b/crypto/openssh-certs.c @@ -1033,12 +1033,14 @@ static bool opensshcert_check_cert( */ if (time < ck->valid_after) { put_fmt(error, "Certificate is not valid until "); - opensshcert_time_to_iso8601(BinarySink_UPCAST(error), time); + opensshcert_time_to_iso8601(BinarySink_UPCAST(error), + ck->valid_after); goto out; } if (time >= ck->valid_before) { put_fmt(error, "Certificate expired at "); - opensshcert_time_to_iso8601(BinarySink_UPCAST(error), time); + opensshcert_time_to_iso8601(BinarySink_UPCAST(error), + ck->valid_before); goto out; } From 4eb089f601388365f00f078b1fa90dd265f707a5 Mon Sep 17 00:00:00 2001 From: Jacob Nevins Date: Sun, 6 Nov 2022 01:56:20 +0000 Subject: [PATCH 10/46] Tweak another certified-host-key-prompt. Like 5f3b743eb0, specifically reassure the user that taking the add-to-cache action will not cause the CA that signed the key to be trusted in any wider context, in the case where there was no previous certified key cached. (I don't know why I missed this out before.) (cherry picked from commit 9209c7ea38bf6bf72f9fbbafce6acbcf92ca73ae) --- ssh/common.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ssh/common.c b/ssh/common.c index a1b4d77d..02bf7322 100644 --- a/ssh/common.c +++ b/ssh/common.c @@ -1023,6 +1023,12 @@ SeatPromptResult verify_ssh_host_key( text, SDT_PARA, "If you trust this host, %s to add the key to " "%s's cache and carry on connecting.", pds->hk_accept_action, appname); + if (key && ssh_key_alg(key)->is_certificate) { + seat_dialog_text_append( + text, SDT_PARA, "(Storing this certified key in the cache " + "will NOT cause its certification authority to be trusted " + "for any other key or host.)"); + } seat_dialog_text_append( text, SDT_PARA, "If you want to carry on connecting just once, " "without adding the key to the cache, %s.", From 0112167f98aea2d627c982e5fc9bdcb69faf7bf0 Mon Sep 17 00:00:00 2001 From: Ben Jackson Date: Fri, 20 Dec 2019 13:56:58 +0000 Subject: [PATCH 11/46] Support xterm any-event mouse tracking From https://invisible-island.net/xterm/ctlseqs/ctlseqs.html#h3-Any-event-tracking: Any-event mode is the same as button-event mode, except that all motion events are reported, even if no mouse button is down. It is enabled by specifying 1003 to DECSET. Normally the front ends only report mouse events when buttons are pressed, so we introduce a MA_MOVE event with MBT_NOTHING set to indicate such a mouse movement. (cherry picked from commit 3cfbd3df0f0e07282f3a45ccb58fa1df1ce08606) --- putty.h | 2 +- terminal/terminal.c | 34 +++++++++++++++++++++++++++++++++- terminal/terminal.h | 2 ++ unix/window.c | 10 +++++++--- windows/window.c | 5 +++++ 5 files changed, 48 insertions(+), 5 deletions(-) diff --git a/putty.h b/putty.h index 5c3adfe9..0ee3a7b6 100644 --- a/putty.h +++ b/putty.h @@ -367,7 +367,7 @@ typedef enum { } Mouse_Button; typedef enum { - MA_NOTHING, MA_CLICK, MA_2CLK, MA_3CLK, MA_DRAG, MA_RELEASE + MA_NOTHING, MA_CLICK, MA_2CLK, MA_3CLK, MA_DRAG, MA_RELEASE, MA_MOVE } Mouse_Action; /* Keyboard modifiers -- keys the user is actually holding down */ diff --git a/terminal/terminal.c b/terminal/terminal.c index 37fa3513..0eee7318 100644 --- a/terminal/terminal.c +++ b/terminal/terminal.c @@ -1374,6 +1374,8 @@ static void power_on(Terminal *term, bool clear) term->xterm_mouse = 0; term->xterm_extended_mouse = false; term->urxvt_extended_mouse = false; + term->raw_mouse_reported_x = 0; + term->raw_mouse_reported_y = 0; win_set_raw_mouse_mode(term->win, false); term->win_pointer_shape_pending = true; term->win_pointer_shape_raw = false; @@ -3068,6 +3070,10 @@ static void toggle_mode(Terminal *term, int mode, int query, bool state) term->xterm_mouse = state ? 2 : 0; term_update_raw_mouse_mode(term); break; + case 1003: /* xterm mouse any-event tracking */ + term->xterm_mouse = state ? 3 : 0; + term_update_raw_mouse_mode(term); + break; case 1006: /* xterm extended mouse */ term->xterm_extended_mouse = state; break; @@ -7055,6 +7061,13 @@ void term_mouse(Terminal *term, Mouse_Button braw, Mouse_Button bcooked, !(term->mouse_override && shift)); int default_seltype; + // Don't do anything if mouse movement events weren't requested; + // Note: return early to avoid doing all of this code on every mouse move + // event only to throw it away. + if (a == MA_MOVE && (!raw_mouse || term->xterm_mouse < 3)) { + return; + } + if (y < 0) { y = 0; if (a == MA_DRAG && !raw_mouse) @@ -7141,6 +7154,11 @@ void term_mouse(Terminal *term, Mouse_Button braw, Mouse_Button bcooked, encstate = 0x41; wheel = true; break; + case MBT_NOTHING: + assert( a == MA_MOVE ); + encstate = 0x03; // release; no buttons pressed + wheel = false; + break; default: return; } @@ -7155,7 +7173,21 @@ void term_mouse(Terminal *term, Mouse_Button braw, Mouse_Button bcooked, case MA_DRAG: if (term->xterm_mouse == 1) return; - encstate += 0x20; + encstate += 0x20; // motion indicator + break; + case MA_MOVE: // mouse move without buttons + assert( braw == MBT_NOTHING && bcooked == MBT_NOTHING ); + if (term->xterm_mouse < 3) + return; + + if (selpoint.x == term->raw_mouse_reported_x && + selpoint.y == term->raw_mouse_reported_y) + return; + + term->raw_mouse_reported_x = x; + term->raw_mouse_reported_y = y; + + encstate += 0x20; // motion indicator break; case MA_RELEASE: /* If multiple extensions are enabled, the xterm 1006 is used, so it's okay to check for only that */ diff --git a/terminal/terminal.h b/terminal/terminal.h index 3f918b22..87441855 100644 --- a/terminal/terminal.h +++ b/terminal/terminal.h @@ -155,6 +155,8 @@ struct terminal_tag { bool xterm_extended_mouse; bool urxvt_extended_mouse; int mouse_is_down; /* used while tracking mouse buttons */ + int raw_mouse_reported_x; + int raw_mouse_reported_y; bool bracketed_paste, bracketed_paste_active; diff --git a/unix/window.c b/unix/window.c index 18cce475..33f4ec98 100644 --- a/unix/window.c +++ b/unix/window.c @@ -495,6 +495,8 @@ static Mouse_Button translate_button(Mouse_Button button) return MBT_PASTE; if (button == MBT_RIGHT) return MBT_EXTEND; + if (button == MBT_NOTHING) + return MBT_NOTHING; return 0; /* shouldn't happen */ } @@ -2298,7 +2300,9 @@ gint motion_event(GtkWidget *widget, GdkEventMotion *event, gpointer data) { GtkFrontend *inst = (GtkFrontend *)data; bool shift, ctrl, alt; - int x, y, button; + Mouse_Action action = MA_DRAG; + Mouse_Button button = MBT_NOTHING; + int x, y; /* Remember the timestamp. */ inst->input_event_time = event->time; @@ -2318,12 +2322,12 @@ gint motion_event(GtkWidget *widget, GdkEventMotion *event, gpointer data) else if (event->state & GDK_BUTTON3_MASK) button = MBT_RIGHT; else - return false; /* don't even know what button! */ + action = MA_MOVE; x = (event->x - inst->window_border) / inst->font_width; y = (event->y - inst->window_border) / inst->font_height; - term_mouse(inst->term, button, translate_button(button), MA_DRAG, + term_mouse(inst->term, button, translate_button(button), action, x, y, shift, ctrl, alt); return true; diff --git a/windows/window.c b/windows/window.c index 937a5f8b..63cc58ed 100644 --- a/windows/window.c +++ b/windows/window.c @@ -2754,6 +2754,11 @@ static LRESULT CALLBACK WndProc(HWND hwnd, UINT message, TO_CHR_X(X_POS(lParam)), TO_CHR_Y(Y_POS(lParam)), wParam & MK_SHIFT, wParam & MK_CONTROL, is_alt_pressed()); + } else { + term_mouse(term, MBT_NOTHING, MBT_NOTHING, MA_MOVE, + TO_CHR_X(X_POS(lParam)), + TO_CHR_Y(Y_POS(lParam)), false, + false, false); } return 0; } From 65639fbbe72266c326e53d0b07ac3291a5c27b3e Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Mon, 14 Nov 2022 22:21:49 +0000 Subject: [PATCH 12/46] Fix duplicate call to term_resize_request_completed(). A KDE user observed that if you 'dock' a GTK PuTTY window to the side of the screen (by dragging it to the RH edge, causing it to half-maximise over the right-hand half of the display, similarly to Windows), and then send a terminal resize sequence, then PuTTY fails the assertion in term_resize_request_completed() which expects that an unacknowledged resize request was currently in flight. When drawing_area_setup() calls term_resize_request_completed() in response to the inst->term_resize_notification_required flag, it resets the inst->win_resize_pending flag, but doesn't reset inst->term_resize_notification_required. As a result, the _next_ call to drawing_area_setup will find that flag still set, and make a duplicate call to term_resize_request_completed, after the terminal no longer believes it's waiting for a response to a resize request. And in this 'docked to the right-hand side of the display' state, KDE apparently triggers two calls to drawing_area_setup() in quick succession, making this bug manifest. I could fix this by clearing inst->term_resize_notification_required. But inspecting all the other call sites, it seems clear to me that my original intention was for inst->term_resize_notification_required to be a flag that's only meaningful if inst->win_resize_pending is set. So I think a better fix is to conditionalise the check in drawing_area_setup so that we don't even check inst->term_resize_notification_required if !inst->win_resize_pending. (cherry picked from commit fec6719a2b7ac48372d8d44098a3b2938948fd2e) --- unix/window.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/unix/window.c b/unix/window.c index 33f4ec98..dbb0ec69 100644 --- a/unix/window.c +++ b/unix/window.c @@ -781,10 +781,11 @@ static void drawing_area_setup(GtkFrontend *inst, int width, int height) inst->drawing_area_setup_called = true; if (inst->term) term_size(inst->term, h, w, conf_get_int(inst->conf, CONF_savelines)); - if (inst->term_resize_notification_required) - term_resize_request_completed(inst->term); - if (inst->win_resize_pending) + if (inst->win_resize_pending) { + if (inst->term_resize_notification_required) + term_resize_request_completed(inst->term); inst->win_resize_pending = false; + } if (!inst->drawing_area_setup_needed) return; From cfda9585ac79f27f177d7f2da0f9da73b6cb445b Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Sun, 20 Nov 2022 10:55:33 +0000 Subject: [PATCH 13/46] Build option to disable scrollback compression. This was requested by a downstream of the code, who wanted to change the time/space tradeoff in the terminal. I currently have no plans to change this setting for upstream PuTTY, although there is a cmake option for it just to make testing it easy. To avoid sprinkling ifdefs over the whole terminal code, the strategy is to keep the separate type 'compressed_scrollback_line', and turn it into a typedef for a 'termline *'. So compressline() becomes almost trivial, and decompressline() even more so. Memory management is the fiddly part. To make this work sensibly on both sides, I've broken up each of compressline() and decompressline() into two versions, one of which takes ownership of (and logically speaking frees) its input, and the other doesn't. So at call sites where a function was followed by a free, it's now calling the 'and_free' version of the function, and where the input object was reused afterwards, it's calling the 'no_free' version. This means that in different branches of the #if, I can make one function call the other or vice versa, and no call site is stuck with having to do things in a more roundabout way than necessary. The freeing of the _return_ value from decompressline() is handled for us, because termlines already have a 'temporary' flag which is set when they're returned from the decompressor, and anyone receiving a termline from lineptr() calls unlineptr() when they're finished with it, which will _conditionally_ free it, depending on that 'temporary' flag. So in the new mode, 'temporary' is never set at all, and all those unlineptr() calls do nothing. However, we also still need to free compressed lines properly when they're actually being thrown away (scrolled off the top of the scrollback, or cleaned up in term_free), and for that, I've made a new special-purpose free_compressed_line() function. (cherry picked from commit 5f2eff2fea10642a445c90d377d816fa5cace3d4) --- cmake/cmake.h.in | 1 + cmake/setup.cmake | 9 +++++ terminal/terminal.c | 97 ++++++++++++++++++++++++++++++++++++++------- 3 files changed, 93 insertions(+), 14 deletions(-) diff --git a/cmake/cmake.h.in b/cmake/cmake.h.in index 5ad32515..3882148b 100644 --- a/cmake/cmake.h.in +++ b/cmake/cmake.h.in @@ -1,6 +1,7 @@ #cmakedefine NO_IPV6 #cmakedefine NO_GSSAPI #cmakedefine STATIC_GSSAPI +#cmakedefine NO_SCROLLBACK_COMPRESSION #cmakedefine NO_MULTIMON diff --git a/cmake/setup.cmake b/cmake/setup.cmake index 7a650771..d81d5a5c 100644 --- a/cmake/setup.cmake +++ b/cmake/setup.cmake @@ -15,6 +15,12 @@ set(PUTTY_FUZZING OFF CACHE BOOL "Build PuTTY binaries suitable for fuzzing, NOT FOR REAL USE") set(PUTTY_COVERAGE OFF CACHE BOOL "Build PuTTY binaries suitable for code coverage analysis") +set(PUTTY_COMPRESS_SCROLLBACK ON + # This is always on in production versions of PuTTY, but downstreams + # of the code have been known to find it a better tradeoff to + # disable it. So there's a #ifdef in terminal.c, and a cmake option + # to enable that ifdef just in case it needs testing or debugging. + CACHE BOOL "Store terminal scrollback in compressed form") set(STRICT OFF CACHE BOOL "Enable extra compiler warnings and make them errors") @@ -108,6 +114,9 @@ endif() if(PUTTY_FUZZING) add_compile_definitions(FUZZING) endif() +if(NOT PUTTY_COMPRESS_SCROLLBACK) + set(NO_SCROLLBACK_COMPRESSION ON) +endif() if(PUTTY_COVERAGE) set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fprofile-arcs -ftest-coverage -g ") endif() diff --git a/terminal/terminal.c b/terminal/terminal.c index 0eee7318..e4063b66 100644 --- a/terminal/terminal.c +++ b/terminal/terminal.c @@ -398,6 +398,8 @@ static void move_termchar(termline *line, termchar *dest, termchar *src) #endif } +#ifndef NO_SCROLLBACK_COMPRESSION + /* * Compress and decompress a termline into an RLE-based format for * storing in scrollback. (Since scrollback almost never needs to @@ -688,11 +690,12 @@ static void makeliteral_cc(strbuf *b, termchar *c, unsigned long *state) typedef struct compressed_scrollback_line { size_t len; + /* compressed data follows after this */ } compressed_scrollback_line; -static termline *decompressline(compressed_scrollback_line *line); +static termline *decompressline_no_free(compressed_scrollback_line *line); -static compressed_scrollback_line *compressline(termline *ldata) +static compressed_scrollback_line *compressline_no_free(termline *ldata) { strbuf *b = strbuf_new(); @@ -768,7 +771,7 @@ static compressed_scrollback_line *compressline(termline *ldata) printf("\n"); #endif - dcl = decompressline(line); + dcl = decompressline_no_free(line); assert(ldata->cols == dcl->cols); assert(ldata->lattr == dcl->lattr); for (i = 0; i < ldata->cols; i++) @@ -788,6 +791,13 @@ static compressed_scrollback_line *compressline(termline *ldata) return line; } +static compressed_scrollback_line *compressline_and_free(termline *ldata) +{ + compressed_scrollback_line *cline = compressline_no_free(ldata); + freetermline(ldata); + return cline; +} + static void readrle(BinarySource *bs, termline *ldata, void (*readliteral)(BinarySource *bs, termchar *c, termline *ldata, unsigned long *state)) @@ -921,7 +931,7 @@ static void readliteral_cc(BinarySource *bs, termchar *c, termline *ldata, } } -static termline *decompressline(compressed_scrollback_line *line) +static termline *decompressline_no_free(compressed_scrollback_line *line) { int ncols, byte, shift; BinarySource bs[1]; @@ -988,6 +998,66 @@ static termline *decompressline(compressed_scrollback_line *line) return ldata; } +static inline void free_compressed_line(compressed_scrollback_line *cline) +{ + sfree(cline); +} + +static termline *decompressline_and_free(compressed_scrollback_line *cline) +{ + termline *ldata = decompressline_no_free(cline); + free_compressed_line(cline); + return ldata; +} + +#else /* NO_SCROLLBACK_COMPRESSION */ + +static termline *duptermline(termline *oldline) +{ + termline *newline = snew(termline); + *newline = *oldline; /* copy the POD structure fields */ + newline->chars = snewn(newline->size, termchar); + for (int j = 0; j < newline->size; j++) + newline->chars[j] = oldline->chars[j]; + return newline; +} + +typedef termline compressed_scrollback_line; + +static inline compressed_scrollback_line *compressline_and_free( + termline *ldata) +{ + return ldata; +} + +static inline compressed_scrollback_line *compressline_no_free(termline *ldata) +{ + return duptermline(ldata); +} + +static inline termline *decompressline_no_free( + compressed_scrollback_line *line) +{ + /* This will return a line without the 'temporary' flag, which + * means that unlineptr() is already set up to avoid freeing it */ + return line; +} + +static inline termline *decompressline_and_free( + compressed_scrollback_line *line) +{ + /* Same as decompressline_no_free, because the caller will free + * our returned termline, and that does all the freeing necessary */ + return line; +} + +static inline void free_compressed_line(compressed_scrollback_line *line) +{ + freetermline(line); +} + +#endif /* NO_SCROLLBACK_COMPRESSION */ + /* * Resize a line to make it `cols' columns wide. */ @@ -1134,7 +1204,7 @@ static termline *lineptr(Terminal *term, int y, int lineno, int screen) compressed_scrollback_line *cline = index234(whichtree, treeindex); if (!cline) null_line_error(term, y, lineno, whichtree, treeindex, "cline"); - line = decompressline(cline); + line = decompressline_no_free(cline); } else { line = index234(whichtree, treeindex); } @@ -2066,12 +2136,13 @@ Terminal *term_init(Conf *myconf, struct unicode_data *ucsdata, TermWin *win) void term_free(Terminal *term) { + compressed_scrollback_line *cline; termline *line; struct beeptime *beep; int i; - while ((line = delpos234(term->scrollback, 0)) != NULL) - sfree(line); /* compressed data, not a termline */ + while ((cline = delpos234(term->scrollback, 0)) != NULL) + free_compressed_line(cline); freetree234(term->scrollback); while ((line = delpos234(term->screen, 0)) != NULL) freetermline(line); @@ -2195,8 +2266,7 @@ void term_size(Terminal *term, int newrows, int newcols, int newsavelines) /* Insert a line from the scrollback at the top of the screen. */ assert(sblen >= term->tempsblines); cline = delpos234(term->scrollback, --sblen); - line = decompressline(cline); - sfree(cline); + line = decompressline_and_free(cline); line->temporary = false; /* reconstituted line is now real */ term->tempsblines -= 1; addpos234(term->screen, line, 0); @@ -2220,8 +2290,7 @@ void term_size(Terminal *term, int newrows, int newcols, int newsavelines) } else { /* push top row to scrollback */ line = delpos234(term->screen, 0); - addpos234(term->scrollback, compressline(line), sblen++); - freetermline(line); + addpos234(term->scrollback, compressline_and_free(line), sblen++); term->tempsblines += 1; term->curs.y -= 1; term->savecurs.y -= 1; @@ -2597,15 +2666,15 @@ static void scroll(Terminal *term, int topline, int botline, * the scrollback is full. */ if (sblen == term->savelines) { - unsigned char *cline; + compressed_scrollback_line *cline; sblen--; cline = delpos234(term->scrollback, 0); - sfree(cline); + free_compressed_line(cline); } else term->tempsblines += 1; - addpos234(term->scrollback, compressline(line), sblen); + addpos234(term->scrollback, compressline_no_free(line), sblen); /* now `line' itself can be reused as the bottom line */ From 1526b56332ce2ce5844f29730255a1069fbacab3 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Wed, 19 Apr 2023 14:20:39 +0100 Subject: [PATCH 14/46] Support horizontal scroll events in mouse tracking. Horizontal scroll events aren't generated by the traditional mouse wheel, but they can be generated by trackpad gestures, though this isn't always configured on. The cross-platform and Windows parts of this patch is due to Christopher Plewright; I added the GTK support. (cherry picked from commit 819efc3c2150ed70c9e5f3a0abad777043e159d0) --- putty.h | 3 ++- terminal/terminal.c | 8 ++++++++ unix/window.c | 50 ++++++++++++++++++++++++++++++++++++--------- windows/window.c | 14 ++++++++----- 4 files changed, 59 insertions(+), 16 deletions(-) diff --git a/putty.h b/putty.h index 0ee3a7b6..e413552c 100644 --- a/putty.h +++ b/putty.h @@ -363,7 +363,8 @@ typedef enum { MBT_NOTHING, MBT_LEFT, MBT_MIDDLE, MBT_RIGHT, /* `raw' button designations */ MBT_SELECT, MBT_EXTEND, MBT_PASTE, /* `cooked' button designations */ - MBT_WHEEL_UP, MBT_WHEEL_DOWN /* mouse wheel */ + MBT_WHEEL_UP, MBT_WHEEL_DOWN, /* vertical mouse wheel */ + MBT_WHEEL_LEFT, MBT_WHEEL_RIGHT /* horizontal mouse wheel */ } Mouse_Button; typedef enum { diff --git a/terminal/terminal.c b/terminal/terminal.c index e4063b66..34a8b2f2 100644 --- a/terminal/terminal.c +++ b/terminal/terminal.c @@ -7223,6 +7223,14 @@ void term_mouse(Terminal *term, Mouse_Button braw, Mouse_Button bcooked, encstate = 0x41; wheel = true; break; + case MBT_WHEEL_LEFT: + encstate = 0x42; + wheel = true; + break; + case MBT_WHEEL_RIGHT: + encstate = 0x43; + wheel = true; + break; case MBT_NOTHING: assert( a == MA_MOVE ); encstate = 0x03; // release; no buttons pressed diff --git a/unix/window.c b/unix/window.c index dbb0ec69..934311f2 100644 --- a/unix/window.c +++ b/unix/window.c @@ -169,7 +169,7 @@ struct GtkFrontend { guint32 input_event_time; /* Timestamp of the most recent input event. */ GtkWidget *dialogs[DIALOG_SLOT_LIMIT]; #if GTK_CHECK_VERSION(3,4,0) - gdouble cumulative_scroll; + gdouble cumulative_hscroll, cumulative_vscroll; #endif /* Cached things out of conf that we refer to a lot */ int bold_style; @@ -2111,8 +2111,8 @@ void input_method_commit_event(GtkIMContext *imc, gchar *str, gpointer data) #define SCROLL_INCREMENT_LINES 5 #if GTK_CHECK_VERSION(3,4,0) -gboolean scroll_internal(GtkFrontend *inst, gdouble delta, guint state, - gdouble ex, gdouble ey) +gboolean scroll_internal(GtkFrontend *inst, gdouble xdelta, gdouble ydelta, + guint state, gdouble ex, gdouble ey) { int x, y; bool shift, ctrl, alt, raw_mouse_mode; @@ -2130,22 +2130,22 @@ gboolean scroll_internal(GtkFrontend *inst, gdouble delta, guint state, !(shift && conf_get_bool(inst->conf, CONF_mouse_override))); - inst->cumulative_scroll += delta * SCROLL_INCREMENT_LINES; + inst->cumulative_vscroll += ydelta * SCROLL_INCREMENT_LINES; if (!raw_mouse_mode) { - int scroll_lines = (int)inst->cumulative_scroll; /* rounds toward 0 */ + int scroll_lines = (int)inst->cumulative_vscroll; /* rounds toward 0 */ if (scroll_lines) { term_scroll(inst->term, 0, scroll_lines); - inst->cumulative_scroll -= scroll_lines; + inst->cumulative_vscroll -= scroll_lines; } return true; } else { - int scroll_events = (int)(inst->cumulative_scroll / + int scroll_events = (int)(inst->cumulative_vscroll / SCROLL_INCREMENT_LINES); if (scroll_events) { int button; - inst->cumulative_scroll -= scroll_events * SCROLL_INCREMENT_LINES; + inst->cumulative_vscroll -= scroll_events * SCROLL_INCREMENT_LINES; if (scroll_events > 0) { button = MBT_WHEEL_DOWN; @@ -2159,6 +2159,35 @@ gboolean scroll_internal(GtkFrontend *inst, gdouble delta, guint state, MA_CLICK, x, y, shift, ctrl, alt); } } + + /* + * Now do the same for horizontal scrolling. But because we + * _only_ use that for passing through to mouse reporting, we + * don't even collect the scroll deltas while not in + * raw_mouse_mode. (Otherwise there would likely be a huge + * unexpected lurch when raw_mouse_mode was enabled!) + */ + inst->cumulative_hscroll += xdelta * SCROLL_INCREMENT_LINES; + scroll_events = (int)(inst->cumulative_hscroll / + SCROLL_INCREMENT_LINES); + if (scroll_events) { + int button; + + inst->cumulative_hscroll -= scroll_events * SCROLL_INCREMENT_LINES; + + if (scroll_events > 0) { + button = MBT_WHEEL_RIGHT; + } else { + button = MBT_WHEEL_LEFT; + scroll_events = -scroll_events; + } + + while (scroll_events-- > 0) { + term_mouse(inst->term, button, translate_button(button), + MA_CLICK, x, y, shift, ctrl, alt); + } + } + return true; } } @@ -2260,7 +2289,7 @@ gboolean scroll_event(GtkWidget *widget, GdkEventScroll *event, gpointer data) #if GTK_CHECK_VERSION(3,4,0) gdouble dx, dy; if (gdk_event_get_scroll_deltas((GdkEvent *)event, &dx, &dy)) { - return scroll_internal(inst, dy, event->state, event->x, event->y); + return scroll_internal(inst, dx, dy, event->state, event->x, event->y); } else if (!gdk_event_get_scroll_direction((GdkEvent *)event, &dir)) { return false; } @@ -5276,7 +5305,8 @@ void new_session_window(Conf *conf, const char *geometry_string) inst->wintitle = inst->icontitle = NULL; inst->drawtype = DRAWTYPE_DEFAULT; #if GTK_CHECK_VERSION(3,4,0) - inst->cumulative_scroll = 0.0; + inst->cumulative_vscroll = 0.0; + inst->cumulative_hscroll = 0.0; #endif inst->drawing_area_setup_needed = true; diff --git a/windows/window.c b/windows/window.c index 63cc58ed..e59bebce 100644 --- a/windows/window.c +++ b/windows/window.c @@ -74,6 +74,9 @@ #ifndef WM_MOUSEWHEEL #define WM_MOUSEWHEEL 0x020A /* not defined in earlier SDKs */ #endif +#ifndef WM_MOUSEHWHEEL +#define WM_MOUSEHWHEEL 0x020E /* not defined in earlier SDKs */ +#endif #ifndef WHEEL_DELTA #define WHEEL_DELTA 120 #endif @@ -3403,10 +3406,11 @@ static LRESULT CALLBACK WndProc(HWND hwnd, UINT message, process_clipdata((HGLOBAL)lParam, wParam); return 0; default: - if (message == wm_mousewheel || message == WM_MOUSEWHEEL) { + if (message == wm_mousewheel || message == WM_MOUSEWHEEL + || message == WM_MOUSEHWHEEL) { bool shift_pressed = false, control_pressed = false; - if (message == WM_MOUSEWHEEL) { + if (message == WM_MOUSEWHEEL || message == WM_MOUSEHWHEEL) { wheel_accumulator += (short)HIWORD(wParam); shift_pressed=LOWORD(wParam) & MK_SHIFT; control_pressed=LOWORD(wParam) & MK_CONTROL; @@ -3425,10 +3429,10 @@ static LRESULT CALLBACK WndProc(HWND hwnd, UINT message, /* reduce amount for next time */ if (wheel_accumulator > 0) { - b = MBT_WHEEL_UP; + b = message == WM_MOUSEHWHEEL ? MBT_WHEEL_RIGHT : MBT_WHEEL_UP; wheel_accumulator -= WHEEL_DELTA; } else if (wheel_accumulator < 0) { - b = MBT_WHEEL_DOWN; + b = message == WM_MOUSEHWHEEL ? MBT_WHEEL_LEFT : MBT_WHEEL_DOWN; wheel_accumulator += WHEEL_DELTA; } else break; @@ -3448,7 +3452,7 @@ static LRESULT CALLBACK WndProc(HWND hwnd, UINT message, TO_CHR_Y(p.y), shift_pressed, control_pressed, is_alt_pressed()); } /* else: not sure when this can fail */ - } else { + } else if (message != WM_MOUSEHWHEEL) { /* trigger a scroll */ term_scroll(term, 0, b == MBT_WHEEL_UP ? From 2698c2944f2016dedbaba767e993c417c481f0b1 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Sun, 27 Nov 2022 11:14:23 +0000 Subject: [PATCH 15/46] Stop selectable GTK message boxes clobbering PRIMARY. I noticed today that when GTK PuTTY puts up a message box such as a host key dialog, which calls our create_message_box function with selectable=true (so that the host key fingerprint can be conveniently copy-pasted), a side effect is to take the X11 PRIMARY selection away from whoever previously had it, even though the message box isn't actually selecting anything right now. I don't fully understand what's going on, but it apparently has something to do with 'select on focus' behaviour, in which tabbing into a selectable text control automatically selects its entire contents. That makes sense for edit boxes, but not really for this kind of thing. Unfortunately, GTK apparently has no per-widget configuration to turn that off. (The closest I found is not even per _application_: it lives in GtkSettings, whose documentation says that it's general across all GTK apps run by a user!) So instead I work around it by moving the gtk_label_set_selectable call to after the focus of the new window has already been sorted out. Ugly, but it seems to work. (cherry picked from commit c14f0e02cce022c7bee77935238a4b34f3c3a261) --- unix/dialog.c | 48 ++++++++++++++++++++++++++++-------------------- 1 file changed, 28 insertions(+), 20 deletions(-) diff --git a/unix/dialog.c b/unix/dialog.c index 5846466a..5663ca92 100644 --- a/unix/dialog.c +++ b/unix/dialog.c @@ -3448,26 +3448,6 @@ static GtkWidget *create_message_box_general( dp->retval = 0; dp->window = window; - if (selectable) { -#if GTK_CHECK_VERSION(2,0,0) - struct uctrl *uc = dlg_find_byctrl(dp, textctrl); - gtk_label_set_selectable(GTK_LABEL(uc->text), true); - - /* - * GTK selectable labels have a habit of selecting their - * entire contents when they gain focus. It's ugly to have - * text in a message box start up all selected, so we suppress - * this by manually selecting none of it - but we must do this - * when the widget _already has_ focus, otherwise our work - * will be undone when it gains it shortly. - */ - gtk_widget_grab_focus(uc->text); - gtk_label_select_region(GTK_LABEL(uc->text), 0, 0); -#else - (void)textctrl; /* placate warning */ -#endif - } - if (parentwin) { set_transient_window_pos(parentwin, window); gtk_window_set_transient_for(GTK_WINDOW(window), @@ -3478,6 +3458,34 @@ static GtkWidget *create_message_box_general( gtk_widget_show(window); gtk_window_set_focus(GTK_WINDOW(window), NULL); +#if GTK_CHECK_VERSION(2,0,0) + if (selectable) { + /* + * GTK selectable labels have a habit of selecting their + * entire contents when they gain focus. As far as I can see, + * an individual GtkLabel has no way to turn this off - source + * diving suggests that the only configurable option for it is + * "gtk-label-select-on-focus" in the cross-application + * GtkSettings, and there's no per-label or even + * per-application override. + * + * It's ugly to have text in a message box start up all + * selected, and also it interferes with any PRIMARY selection + * you might already have had. So for this purpose we'd prefer + * that the text doesn't _start off_ selected, but it should + * be selectable later. + * + * So we make the label selectable _now_, after the widget is + * shown and the focus has already gone wherever it's going. + */ + struct uctrl *uc = dlg_find_byctrl(dp, textctrl); + gtk_label_select_region(GTK_LABEL(uc->text), 0, 0); + gtk_label_set_selectable(GTK_LABEL(uc->text), true); + } +#else + (void)textctrl; /* placate warning */ +#endif + #if !GTK_CHECK_VERSION(2,0,0) dp->currtreeitem = NULL; dp->treeitems = NULL; From dcac0f1d43bf3d69cb07ebb1ca9dcc51f0c63742 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Sun, 4 Dec 2022 11:50:08 +0000 Subject: [PATCH 16/46] GTK: fix crash changing font size when terminal maximised. When I maximised a terminal window today and then used Ctrl-< to reduce its font size (expecting that the window size would stay the same but more characters would be squeezed in), pterm failed the assertion in term_request_resize_completed() that checks term->win_resize_pending == WIN_RESIZE_AWAIT_REPLY. This happened because in this situation request_resize_internal() was called from within window.c rather than from within the terminal code itself. So the terminal didn't know a resize is pending at all, and was surprised to be told that one had finished. request_resize_internal() already has a flag parameter to tell it whether a given resize came from the terminal or not. On the main code path, that flag is used to decide whether to notify the terminal. But on the early exit path when the window is maximised, we weren't checking the flag. An easy fix. (cherry picked from commit 95b926865a065e02b294b40ab899c8e921d87227) --- unix/window.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/unix/window.c b/unix/window.c index 934311f2..1ca52833 100644 --- a/unix/window.c +++ b/unix/window.c @@ -2602,7 +2602,8 @@ static void request_resize_internal(GtkFrontend *inst, bool from_terminal, #endif 0)) { queue_toplevel_callback(gtkwin_deny_term_resize, inst); - term_resize_request_completed(inst->term); + if (from_terminal) + term_resize_request_completed(inst->term); return; } } From 1405659dee92bf4b498ba63c557211ce64a3f3cd Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Wed, 21 Dec 2022 15:05:04 +0000 Subject: [PATCH 17/46] ldisc: fix unwanted double-action of ^U. In ldisc's line editing mode, pressing ^U is supposed to erase the current unsent line rather than inserting a literal ^U into the buffer. In fact, when using a non-Telnet backend, it erases the line *and* inserts ^U into the buffer! This happens because it shares a case handler with three other disruptive control characters (^C, ^\, ^Z), which all also clear the line-editing buffer before doing their various actions. But in non-Telnet mode, their actions become literal insertion of themselves, so the combined effect is to erase the line and them self-insert. I'm not 100% convinced that was what I actually meant to do with those characters. But it _certainly_ wasn't what I meant to do with ^U, so that one at least I should fix right now! (cherry picked from commit 5ade8c00479520031b6a507e4db088fca7d82893) --- ldisc.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ldisc.c b/ldisc.c index caff52d0..27f41a6f 100644 --- a/ldisc.c +++ b/ldisc.c @@ -442,6 +442,8 @@ void ldisc_send(Ldisc *ldisc, const void *vbuf, int len, bool interactive) bsb(ldisc, plen(ldisc, ldisc->buf[ldisc->buflen - 1])); ldisc->buflen--; } + if (c == CTRL('U')) + break; /* ^U *just* erases a line */ ldisc_to_backend_special(ldisc, SS_EL, 0); /* * We don't send IP, SUSP or ABORT if the user has From 07911c87352b185a92057629a9e87a89d04bbf7a Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Wed, 21 Dec 2022 15:13:46 +0000 Subject: [PATCH 18/46] Prevent sending double-EOF in raw backend. You can't call sk_write_eof() twice on the same socket, because the second one will fail an assertion. Instead, you're supposed to know you've already sent EOF, and not try to send it again. The call to sk_write_eof() in raw_special (triggered by pressing ^D in GUI PuTTY, for example) sets the flag raw->sent_socket_eof in an attempt to prevent this. But it doesn't _check_ that flag, so a second press of ^D can reach that assertion failure. (cherry picked from commit 9f2e1e6e03043f551b1a449991ea45d954204808) --- otherbackends/raw.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/otherbackends/raw.c b/otherbackends/raw.c index c9a32e05..c95db970 100644 --- a/otherbackends/raw.c +++ b/otherbackends/raw.c @@ -285,7 +285,8 @@ static void raw_special(Backend *be, SessionSpecialCode code, int arg) { Raw *raw = container_of(be, Raw, backend); if (code == SS_EOF && raw->s) { - sk_write_eof(raw->s); + if (!raw->sent_socket_eof) + sk_write_eof(raw->s); raw->sent_socket_eof= true; raw_check_close(raw); } From 2687d9ec36ada217eeaf31c24efd057106e15db3 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Fri, 30 Dec 2022 11:09:31 +0000 Subject: [PATCH 19/46] Fix typo in 'plink -share' documentation. (cherry picked from commit 752f5028f07da188788d0d208bb0cc3e39ddc076) --- doc/plink.but | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/plink.but b/doc/plink.but index 39b14f2b..e73830eb 100644 --- a/doc/plink.but +++ b/doc/plink.but @@ -254,7 +254,7 @@ line. \S2{plink-option-share} \I{-share-plink}\c{-share}: Test and try to share an existing connection. -This option tris to detect if an existing connection can be shared +This option tries to detect if an existing connection can be shared (See \k{config-ssh-sharing} for more information about SSH connection sharing.) and reuses that connection. From 1aa97cfb89c7a785fa3431fe7863e44ce09f2a82 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Fri, 30 Dec 2022 20:08:46 +0000 Subject: [PATCH 20/46] Another minor docs typo. (cherry picked from commit 37f67bc9378cec81746c21c1317fe025e59a04c3) --- doc/pubkey.but | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/pubkey.but b/doc/pubkey.but index 5ac59390..cdd33301 100644 --- a/doc/pubkey.but +++ b/doc/pubkey.but @@ -367,7 +367,7 @@ will need to tell PuTTY to use for authentication (see \k{pageant-mainwin-addkey}). (You can optionally change some details of the PPK format for your saved -key files; see \k{puttygen-save-params}. But The defaults should be +key files; see \k{puttygen-save-params}. But the defaults should be fine for most purposes.) \S{puttygen-savepub} Saving your public key to a disk file From 3c13ea609240a77ce0a13b4c9c726f5a6f297975 Mon Sep 17 00:00:00 2001 From: Jacob Nevins Date: Sat, 7 Jan 2023 14:02:14 +0000 Subject: [PATCH 21/46] FAQ about private key configuration control move. This is genuinely a FAQ -- we've been asked about it 3-4 times now. (cherry picked from commit 943e54db4aff58b7cc904d3abb0e69281814ba5a) --- doc/faq.but | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/doc/faq.but b/doc/faq.but index 9a8deea2..b46c127c 100644 --- a/doc/faq.but +++ b/doc/faq.but @@ -1084,6 +1084,24 @@ sending of the \cw{IUTF8} mode: on the SSH / TTY panel, select \cw{IUTF8} on the list, select \q{Nothing}, and press \q{Set}. (It's not possible to disable sending this mode in 0.68.) +\S{faq-privkey-control-moved}{Question} Since 0.78, I can't find where +to configure my private key. + +In PuTTY 0.78, the \q{\ii{Private key} file for authentication} control, +where you specify a \c{.\i{PPK}} file for SSH public key authentication, +moved to a new \q{Credentials} panel in the configuration dialog. You can +find this by opening the \q{SSH} category in the tree view on the left, +then opening the \q{Auth} subcategory under that, then clicking on +\q{Credentials}. On this page you'll find the \q{Browse...} button you +need to select a \c{.PPK} file for authentication, as described in +\k{config-ssh-privkey}. + +(This control had previously been on the \q{Auth} panel since public +key authentication was first released in 2002, so many online how-to +guides still describe it there. The configuration controls were +reorganised to make room for features added in 0.78, such as OpenSSH +certificates.) + \H{faq-secure} Security questions \S{faq-publicpc}{Question} Is it safe for me to download PuTTY and From ca331eeed78b39c4c245a48c8c0f28fcd4aa67d5 Mon Sep 17 00:00:00 2001 From: Jacob Nevins Date: Sat, 7 Jan 2023 14:03:12 +0000 Subject: [PATCH 22/46] It's a new year. (cherry picked from commit 89014315ed8e872c4f43b8dbe16a549db7b26694) --- LICENCE | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/LICENCE b/LICENCE index 57ea292b..c4210950 100644 --- a/LICENCE +++ b/LICENCE @@ -1,4 +1,4 @@ -PuTTY is copyright 1997-2022 Simon Tatham. +PuTTY is copyright 1997-2023 Simon Tatham. Portions copyright Robert de Bath, Joris van Rantwijk, Delian Delchev, Andreas Schultz, Jeroen Massar, Wez Furlong, Nicolas Barry, From ae3c419ec1b8d8068baf07030599f08a23ba0561 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Wed, 18 Jan 2023 18:01:37 +0000 Subject: [PATCH 23/46] Fix build failure on systems without fstatat. cmake's configure-time #defines (at least the way I use them) are defined to 0 or 1, rather than sometimes not defined at all, so you have to test them with plain #if rather than #ifdef or #if defined. I _thought_ I'd caught all of those in previous fixes, but apparently there were a couple still lurking. Oops. (cherry picked from commit e289265e3712a20fa03e6da2ee14a0d441056c6c) --- unix/sftpserver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unix/sftpserver.c b/unix/sftpserver.c index 453a3ee0..fe3c2501 100644 --- a/unix/sftpserver.c +++ b/unix/sftpserver.c @@ -602,7 +602,7 @@ static void uss_readdir(SftpServer *srv, SftpReplyBuilder *reply, char *longnamebuf = NULL; struct fxp_attrs attrs = no_attrs; -#if defined HAVE_FSTATAT && defined HAVE_DIRFD +#if HAVE_FSTATAT && HAVE_DIRFD struct stat st; if (!fstatat(dirfd(udh->dp), de->d_name, &st, AT_SYMLINK_NOFOLLOW)) { char perms[11], *uidbuf = NULL, *gidbuf = NULL; From 5e250745ead921d5dc1aa8a4999e23ea9bd1fb24 Mon Sep 17 00:00:00 2001 From: Jacob Nevins Date: Sun, 22 Jan 2023 10:13:37 +0000 Subject: [PATCH 24/46] 'private key' -> 'SSH private key' in new FAQ. (cherry picked from commit 343f64c2ca7a7a76321111185c7593a0cf619d55) --- doc/faq.but | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/faq.but b/doc/faq.but index b46c127c..61d2a5a2 100644 --- a/doc/faq.but +++ b/doc/faq.but @@ -1085,7 +1085,7 @@ sending of the \cw{IUTF8} mode: on the SSH / TTY panel, select not possible to disable sending this mode in 0.68.) \S{faq-privkey-control-moved}{Question} Since 0.78, I can't find where -to configure my private key. +to configure my SSH private key. In PuTTY 0.78, the \q{\ii{Private key} file for authentication} control, where you specify a \c{.\i{PPK}} file for SSH public key authentication, From 5cac358f7f1cce00b344ea2716a4e3af37eca0a1 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Sat, 4 Feb 2023 15:06:21 +0000 Subject: [PATCH 25/46] Reinstate putty.chm in Windows binary zipfiles. A user just reported that it hasn't been there since 0.76. This turns out to be because I put the wrong pathname on the 'zip' commands in Buildscr (miscounted the number of ../ segments). I would have noticed immediately, if Info-Zip had failed with an error when it found I'd given it a nonexistent filename to add to the zip file. But in fact it just prints a warning and proceeds to add all the other files I specified. It looks as if it will only return a nonzero exit status if _all_ the filenames you specified were nonexistent. Therefore, I've rewritten the zip-creation commands so that they run zip once per file. That way if any file is unreadable we _will_ get a build error. (Also, while I'm here, I took the opportunity to get rid of that ugly ls|grep.) (cherry picked from commit 9d308b39da715f77c9c07ea28eeb3016d1be12b1) --- Buildscr | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/Buildscr b/Buildscr index 02de6432..a7cb49df 100644 --- a/Buildscr +++ b/Buildscr @@ -270,12 +270,16 @@ in putty/windows do mkdir deliver in putty/windows do for subdir in build32 abuild32 build64 abuild64 buildold; do mkdir deliver/$$subdir; done in putty/windows do while read x; do mv $$x deliver/$$x; mv $$x.map deliver/$$x.map; done < to-sign.txt -in putty/windows/deliver/buildold do zip -k -j putty.zip `ls *.exe | grep -vxE '^(puttytel|pterm).exe'` ../../../docbuild/putty.chm -in putty/windows/deliver/build32 do zip -k -j putty.zip `ls *.exe | grep -vxE '^(puttytel|pterm).exe'` ../../../docbuild/putty.chm -in putty/windows/deliver/build64 do zip -k -j putty.zip `ls *.exe | grep -vxE '^(puttytel|pterm).exe'` ../../../docbuild/putty.chm -in putty/windows/deliver/abuild32 do zip -k -j putty.zip `ls *.exe | grep -vxE '^(puttytel|pterm).exe'` ../../../docbuild/putty.chm -in putty/windows/deliver/abuild64 do zip -k -j putty.zip `ls *.exe | grep -vxE '^(puttytel|pterm).exe'` ../../../docbuild/putty.chm -in docbuild/html do zip puttydoc.zip *.html +# Make putty.zip in each Windows directory. We add the files one by +# one, because 'zip' exits with a success status if it managed to add +# _at least one_ of the input files, even if another didn't exist. So +# doing them one at a time gets us proper error reporting. +in putty/windows/deliver/buildold do for file in *.exe ../../../../docbuild/putty.chm; do case "$$file" in puttytel.exe | pterm.exe) ;; *) zip -k -j putty.zip "$$file";; esac; done +in putty/windows/deliver/build32 do for file in *.exe ../../../../docbuild/putty.chm; do case "$$file" in puttytel.exe | pterm.exe) ;; *) zip -k -j putty.zip "$$file";; esac; done +in putty/windows/deliver/build64 do for file in *.exe ../../../../docbuild/putty.chm; do case "$$file" in puttytel.exe | pterm.exe) ;; *) zip -k -j putty.zip "$$file";; esac; done +in putty/windows/deliver/abuild32 do for file in *.exe ../../../../docbuild/putty.chm; do case "$$file" in puttytel.exe | pterm.exe) ;; *) zip -k -j putty.zip "$$file";; esac; done +in putty/windows/deliver/abuild64 do for file in *.exe ../../../../docbuild/putty.chm; do case "$$file" in puttytel.exe | pterm.exe) ;; *) zip -k -j putty.zip "$$file";; esac; done +in docbuild/html do for file in *.html; do case "$$file" in puttytel.exe | pterm.exe) ;; *) zip puttydoc.zip "$$file";; esac; done # Deliver the actual PuTTY release directory into a subdir `putty'. deliver putty/windows/deliver/buildold/*.exe putty/w32old/$@ From 407bf88a959cce39bc284ce77f71d017c2c7c9e1 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Tue, 28 Feb 2023 18:58:14 +0000 Subject: [PATCH 26/46] Document our long-standing workarounds policy. For years I've been following the principle that before I'll add auto-detection of an SSH server bug, I want the server maintainer to have fixed the bug, so that the list of affected version numbers triggering the workaround is complete, and to provide an incentive for implementations to gradually converge on rightness. *Finally*, I've got round to documenting that policy in public, for the Feedback page! (cherry picked from commit b5645f79ddadeecfe15c4db24b64a0afc48fca4a) --- doc/config.but | 3 +++ doc/feedback.but | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/doc/config.but b/doc/config.but index 9770697d..56eac575 100644 --- a/doc/config.but +++ b/doc/config.but @@ -3521,6 +3521,9 @@ not available for bugs that \e{cannot} be detected from the server version, e.g. because they must be acted on before the server version is known.) +(The PuTTY project has a defined policy about when we're prepared to +add auto-detection for a bug workaround. See \k{feedback-workarounds}.) + \S{config-ssh-bug-ignore2} \q{Chokes on SSH-2 \i{ignore message}s} An ignore message (SSH_MSG_IGNORE) is a message in the SSH protocol diff --git a/doc/feedback.but b/doc/feedback.but index b7aa9c87..d4f58c09 100644 --- a/doc/feedback.but +++ b/doc/feedback.but @@ -297,6 +297,54 @@ high-quality software to the users comes first.) way to get a feature implemented quickly, if it's a big one that we don't have time to do ourselves. +\H{feedback-workarounds} Workarounds for SSH server bugs + +It's normal for SSH implementations to automatically enable +workarounds for each other's bugs, using the software version strings +that are exchanged at the start of the connection. Typically an SSH +client will have a list of server version strings that it believes to +have particular bugs, and auto-enable the appropriate set of +workarounds when it sees one of those strings. (And servers will have +a similar list of workarounds for \e{client} software they believe to +be buggy.) + +If you've found a bug in an SSH server, and you'd like us to add an +auto-detected workaround for it, our policy is that \s{the server +implementor should fix it first}. + +If the server implementor has fixed it in the latest version, and can +give us a complete description of the version strings that go with the +bug, then we're happy to use those version strings as a trigger to +automatically enable our workaround (assuming one is possible). We +\e{won't} accept requests to auto-enable workarounds for an open-ended +set of version strings, such as \q{any version of FooServer, including +future ones not yet released}. + +The aim of this policy is to encourage implementors to gradually +converge on the actual standardised SSH protocol. If we enable people +to continue violating the spec, by installing open-ended workarounds +in PuTTY for bugs they're never going to fix, then we're contributing +to an ecosystem in which everyone carries on having bugs and everyone +else carries on having to work around them. + +An exception: if an SSH server is no longer maintained \e{at all} +(e.g. the company that produced it has gone out of business), and +every version of it that was ever released has a bug, then that's one +situation in which we may be prepared to add a workaround rule that +matches all versions of that software. (The aim is to stop +implementors from continuing to release software with the bug \dash +and if they're not releasing it \e{at all} any more, then that's +already done!) + +We do recognise that sometimes it will be difficult to get the server +maintainer to fix a bug, or even to answer support requests at all. Or +it might take them a very long time to get round to doing anything +about it. We're not completely unwilling to compromise: we're prepared +to add \e{manually enabled} workarounds to PuTTY even for bugs that an +implementation hasn't fixed yet. We just won't \e{automatically} +enable the workaround unless the server maintainer has also done their +part. + \H{feedback-support} \ii{Support requests} If you're trying to make PuTTY do something for you and it isn't From 05276bda5cbc7fbacb71cf3b32997a9a62efbfd7 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Wed, 19 Apr 2023 14:20:58 +0100 Subject: [PATCH 27/46] Make backspace take account of LATTR_WRAPPED2. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Suppose an application tries to print a double-width character starting in the rightmost column of the screen, so that we apply our emergency fix of wrapping to the next line immediately and printing the character in the first two columns. Suppose they then backspace twice, taking the cursor to the RHS and then the LHS of that character. What should happen if they backspace a third time? Our previous behaviour was to completely ignore the unusual situation, and do the same thing we'd do in any other backspace from column 0: anti-wrap the cursor to the last column of the previous line, leaving it in the empty character cell that was skipped when the DW char couldn't be printed in it. But I think this isn't the best response, because it breaks the invariant that printing N columns' worth of graphic characters and then backspacing N times should leave the cursor on the first of those characters. If I print "a가" (for example) and then backspace three times, I want the cursor on the a, _even_ if weird line wrapping behaviour happened somewhere in that sequence. (Rationale: this helps naïve terminal applications which don't even know what the terminal width is, and aren't tracking their absolute x position. In particular, the simplistic line-based input systems that appear in OS kernels and our own lineedit.c will want to emit a fixed number of backspace-space-backspace sequences to delete characters previously entered on to the line by the user. They still need to check the wcwidth of the characters they're emitting, so that they can BSB twice for a DW character or 0 times for a combining one, but it would be *hugely* more awkward for them to ask the terminal where the cursor is so that they can take account of difficult line wraps!) We already have the ability to _recognise_ this situation: on a line that was wrapped in this unusual way, we set the LATTR_WRAPPED2 line attribute flag, to prevent the empty rightmost column from injecting an unwanted space into copy-pastes from the terminal. Now we also use the same flag to cause the backspace control character to do something interesting. This was the fix that inspired me to start writing test_terminal, because I knew it was touching a delicate area. However, in the course of writing this fix and its tests, I encountered two (!) further bugs, which I'll fix in followup commits! (cherry picked from commit 9ba742ad9f44ffddf053b77064e4b2694b2a1a37) --- terminal/terminal.c | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/terminal/terminal.c b/terminal/terminal.c index 34a8b2f2..f1e8c7c0 100644 --- a/terminal/terminal.c +++ b/terminal/terminal.c @@ -3965,14 +3965,35 @@ static void term_out(Terminal *term, bool called_from_term_data) break; } case '\b': /* BS: Back space */ - if (term->curs.x == 0 && (term->curs.y == 0 || !term->wrap)) - /* do nothing */ ; - else if (term->curs.x == 0 && term->curs.y > 0) + if (term->curs.x == 0 && (term->curs.y == 0 || !term->wrap)) { + /* do nothing */ + } else if (term->curs.x == 0 && term->curs.y > 0) { term->curs.x = term->cols - 1, term->curs.y--; - else if (term->wrapnext) + + /* + * If the line we've just wrapped back on to had the + * LATTR_WRAPPED2 flag set, it means that the line wrapped + * because a double-width character was printed with the + * cursor in the rightmost column, and the best handling + * available was to leave that column empty and move the + * whole character to the next line. In that situation, + * backspacing needs to put the cursor on the previous + * _logical_ character, i.e. skip the empty space left by + * the wrapping. This arranges that if an application + * unaware of the terminal width or cursor position prints + * a number of printing characters and then tries to return + * to a particular one of them by emitting the right number + * of backspaces, it's still the right number even if a + * line break appeared in a maximally awkward position. + */ + termline *ldata = scrlineptr(term->curs.y); + if (term->curs.x > 0 && (ldata->lattr & LATTR_WRAPPED2)) + term->curs.x--; + } else if (term->wrapnext) { term->wrapnext = false; - else + } else { term->curs.x--; + } seen_disp_event(term); break; case '\016': /* LS1: Locking-shift one */ From 8b8b774fc02a67bd9278f4780d9c613ca5eca012 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Wed, 19 Apr 2023 14:21:09 +0100 Subject: [PATCH 28/46] Fix behaviour of backspace in a 1-column terminal. This is the first bug found as a direct result of writing that terminal test program - I added some tests for things I expected to work already, and some of them didn't, proving immediately that it was a good idea! If the terminal is one column wide, and you've printed a character (hence, set the wrapnext flag), what should backspace do? Surely it should behave like any other backspace with wrapnext set, i.e. clear the wrapnext flag, returning the cursor's _logical_ position to the location of the most recently printed character. But in fact it was anti-wrapping to the previous line, because I'd got the cases in the wrong order in the if-else chain that forms the backspace handler. So the handler for 'we're in column 0, wrapping time' was coming before 'wrapnext is set, just clear it'. Now wrapnext is checked _first_, before checking anything at all. Any time we can just clear that, we should. (cherry picked from commit 069f7c8b21df80a80445ffc986f1e243c72bc1cb) --- terminal/terminal.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/terminal/terminal.c b/terminal/terminal.c index f1e8c7c0..6a407be3 100644 --- a/terminal/terminal.c +++ b/terminal/terminal.c @@ -3965,7 +3965,10 @@ static void term_out(Terminal *term, bool called_from_term_data) break; } case '\b': /* BS: Back space */ - if (term->curs.x == 0 && (term->curs.y == 0 || !term->wrap)) { + if (term->wrapnext) { + term->wrapnext = false; + } else if (term->curs.x == 0 && + (term->curs.y == 0 || !term->wrap)) { /* do nothing */ } else if (term->curs.x == 0 && term->curs.y > 0) { term->curs.x = term->cols - 1, term->curs.y--; @@ -3989,8 +3992,6 @@ static void term_out(Terminal *term, bool called_from_term_data) termline *ldata = scrlineptr(term->curs.y); if (term->curs.x > 0 && (ldata->lattr & LATTR_WRAPPED2)) term->curs.x--; - } else if (term->wrapnext) { - term->wrapnext = false; } else { term->curs.x--; } From 15081bb0ad3ddde9c98c75b9e0675eff0cfe75aa Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Wed, 19 Apr 2023 14:21:19 +0100 Subject: [PATCH 29/46] Fix printing double-width char in rightmost column without wrap. Another bug turned up by writing tests. The code that spots that the character won't fit, and wraps it to the next line setting LATTR_WRAPPED2, was not checking that wrap mode was _enabled_ before doing that. So if you printed a DW character in the rightmost column while the terminal was in non-auto-wrap mode, you'd get an unwanted wrap. Other terminals disagree on what to do here. xterm leaves the cursor in the same place and doesn't print any character at all. gnome-terminal, on the other hand, backspaces by a character so that it _can_ print the requested DW character, in the rightmost _two_ columns. I think I don't much like either of those, so instead I'm using the same fallback we use for displaying a DW character when the whole terminal is only one column wide: if there is physically no room to print the requested character, turn it into U+FFFD REPLACEMENT CHARACTER. (cherry picked from commit ed5bf9b3b86a45b36468a6f29d535d2f74436387) --- terminal/terminal.c | 45 +++++++++++++++++++++++++++------------------ 1 file changed, 27 insertions(+), 18 deletions(-) diff --git a/terminal/terminal.c b/terminal/terminal.c index 6a407be3..c5af4dc6 100644 --- a/terminal/terminal.c +++ b/terminal/terminal.c @@ -3331,34 +3331,43 @@ static void term_display_graphic_char(Terminal *term, unsigned long c) linecols -= TRUST_SIGIL_WIDTH; /* - * Preliminary check: if the terminal is only one character cell - * wide, then we cannot display any double-width character at all. - * Substitute single-width REPLACEMENT CHARACTER instead. + * Before we switch on the character width, do a preliminary check for + * cases where we might have no room at all to display a double-width + * character. Our fallback is to substitute REPLACEMENT CHARACTER, + * which is single-width, and it's easiest to do that _before_ having + * to 'goto' from one switch case to another. */ - if (width == 2 && linecols < 2) { - width = 1; - c = 0xFFFD; + if (width == 2 && term->curs.x >= linecols-1) { + /* + * If we're in wrapping mode and the terminal is at least 2 cells + * wide, it's OK, we have a fallback. But otherwise, substitute. + */ + if (linecols < 2 || !term->wrap) { + width = 1; + c = 0xFFFD; + } } switch (width) { case 2: /* - * If we're about to display a double-width character starting - * in the rightmost column, then we do something special - * instead. We must print a space in the last column of the - * screen, then wrap; and we also set LATTR_WRAPPED2 which - * instructs subsequent cut-and-pasting not only to splice - * this line to the one after it, but to ignore the space in - * the last character position as well. (Because what was - * actually output to the terminal was presumably just a - * sequence of CJK characters, and we don't want a space to be - * pasted in the middle of those just because they had the - * misfortune to start in the wrong parity column. xterm - * concurs.) + * If we're about to display a double-width character starting in + * the rightmost column (and we're in wrapping mode - the other + * case was disposed of above), then we do something special + * instead. We must print a space in the last column of the screen, + * then wrap; and we also set LATTR_WRAPPED2 which instructs + * subsequent cut-and-pasting not only to splice this line to the + * one after it, but to ignore the space in the last character + * position as well. (Because what was actually output to the + * terminal was presumably just a sequence of CJK characters, and + * we don't want a space to be pasted in the middle of those just + * because they had the misfortune to start in the wrong parity + * column. xterm concurs.) */ check_boundary(term, term->curs.x, term->curs.y); check_boundary(term, term->curs.x+2, term->curs.y); if (term->curs.x >= linecols-1) { + assert(term->wrap); /* we handled the non-wrapping case above */ copy_termchar(cline, term->curs.x, &term->erase_char); cline->lattr |= LATTR_WRAPPED | LATTR_WRAPPED2; From bece41ddb0098f30d038cb4bcf6befe9c514c776 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Wed, 19 Apr 2023 14:21:32 +0100 Subject: [PATCH 30/46] Add some missing casts in ctype functions. I thought I'd found all of these before, but perhaps a few managed to slip in since I last looked. The character argument to the functions must have the value of an unsigned char or EOF; passing an ordinary char (unless you know char is unsigned on every platform the code will ever go near) risks mistaking '\xFF' for EOF, and causing outright undefined behaviour on byte values in the range 80-FE. Never do it. (cherry picked from commit a76109c586944d9ace03ca6a54b7f8173f6c0deb) --- ssh/sesschan.c | 3 ++- utils/validate_manual_hostkey.c | 2 +- windows/pageant.c | 2 +- windows/unicode.c | 10 ++++++---- windows/utils/split_into_argv.c | 6 +++--- windows/window.c | 2 +- 6 files changed, 14 insertions(+), 11 deletions(-) diff --git a/ssh/sesschan.c b/ssh/sesschan.c index 9776eaf2..cb5f0af1 100644 --- a/ssh/sesschan.c +++ b/ssh/sesschan.c @@ -425,7 +425,8 @@ bool sesschan_enable_x11_forwarding( const unsigned char *hex = authdata_hex.ptr; char hexbuf[3]; - if (!isxdigit(hex[i]) || !isxdigit(hex[i+1])) { + if (!isxdigit((unsigned char)hex[i]) || + !isxdigit((unsigned char)hex[i+1])) { strbuf_free(authdata_bin); return false; /* not hex */ } diff --git a/utils/validate_manual_hostkey.c b/utils/validate_manual_hostkey.c index 7c5d1b88..dd7c1aee 100644 --- a/utils/validate_manual_hostkey.c +++ b/utils/validate_manual_hostkey.c @@ -63,7 +63,7 @@ bool validate_manual_hostkey(char *key) if (r[3*i+2] != ':') goto not_fingerprint; /* sorry */ for (i = 0; i < 16*3 - 1; i++) - key[i] = tolower(r[i]); + key[i] = tolower((unsigned char)r[i]); key[16*3 - 1] = '\0'; return true; } diff --git a/windows/pageant.c b/windows/pageant.c index d1368903..c2380954 100644 --- a/windows/pageant.c +++ b/windows/pageant.c @@ -1835,7 +1835,7 @@ int WINAPI WinMain(HINSTANCE inst, HINSTANCE prev, LPSTR cmdline, int show) args = strchr(command, ' '); if (args) { *args++ = 0; - while(*args && isspace(*args)) args++; + while (*args && isspace((unsigned char)*args)) args++; } spawn_cmd(command, args, show); } diff --git a/windows/unicode.c b/windows/unicode.c index b3f6d802..807783b2 100644 --- a/windows/unicode.c +++ b/windows/unicode.c @@ -1071,9 +1071,11 @@ static int check_compose_internal(int first, int second, int recurse) if (recurse == 0) { nc = check_compose_internal(second, first, 1); if (nc == -1) - nc = check_compose_internal(toupper(first), toupper(second), 1); + nc = check_compose_internal(toupper((unsigned char)first), + toupper((unsigned char)second), 1); if (nc == -1) - nc = check_compose_internal(toupper(second), toupper(first), 1); + nc = check_compose_internal(toupper((unsigned char)second), + toupper((unsigned char)first), 1); } return nc; } @@ -1097,9 +1099,9 @@ int decode_codepage(const char *cp_name) s = cp_name; d = cpi->name; for (;;) { - while (*s && !isalnum(*s) && *s != ':') + while (*s && !isalnum((unsigned char)*s) && *s != ':') s++; - while (*d && !isalnum(*d) && *d != ':') + while (*d && !isalnum((unsigned char)*d) && *d != ':') d++; if (*s == 0) { codepage = cpi->codepage; diff --git a/windows/utils/split_into_argv.c b/windows/utils/split_into_argv.c index c42c7a0b..fe6bdbf7 100644 --- a/windows/utils/split_into_argv.c +++ b/windows/utils/split_into_argv.c @@ -173,7 +173,7 @@ void split_into_argv(char *cmdline, int *argc, char ***argv, * First deal with the simplest of all special cases: if there * aren't any arguments, return 0,NULL,NULL. */ - while (*cmdline && isspace(*cmdline)) cmdline++; + while (*cmdline && isspace((unsigned char)*cmdline)) cmdline++; if (!*cmdline) { if (argc) *argc = 0; if (argv) *argv = NULL; @@ -195,7 +195,7 @@ void split_into_argv(char *cmdline, int *argc, char ***argv, bool quote; /* Skip whitespace searching for start of argument. */ - while (*p && isspace(*p)) p++; + while (*p && isspace((unsigned char)*p)) p++; if (!*p) break; /* We have an argument; start it. */ @@ -206,7 +206,7 @@ void split_into_argv(char *cmdline, int *argc, char ***argv, /* Copy data into the argument until it's finished. */ while (*p) { - if (!quote && isspace(*p)) + if (!quote && isspace((unsigned char)*p)) break; /* argument is finished */ if (*p == '"' || *p == '\\') { diff --git a/windows/window.c b/windows/window.c index e59bebce..50ed35ac 100644 --- a/windows/window.c +++ b/windows/window.c @@ -912,7 +912,7 @@ char *handle_restrict_acl_cmdline_prefix(char *p) * pointer past the prefix. Returns the updated pointer (whether * it moved or not). */ - while (*p && isspace(*p)) + while (*p && isspace((unsigned char)*p)) p++; if (*p == '&' && p[1] == 'R' && (!p[2] || p[2] == '@' || p[2] == '&')) { From bdf7f73d3db1bff40607266f98e8f10a26f00a52 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Wed, 19 Apr 2023 14:21:51 +0100 Subject: [PATCH 31/46] split_into_argv: stop using isspace(). I checked exhaustively today and found that the only characters (even in Unicode) that Windows's default argv splitter will recognise as word separators are the space and tab characters. So I think it's a mistake to use functions to identify word separators; we should use that fixed character pair, and then we know we're getting the right ones only. (cherry picked from commit 9adfa797677ba5cc5a5c9db45e593a9e4ab293aa) --- windows/utils/split_into_argv.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/windows/utils/split_into_argv.c b/windows/utils/split_into_argv.c index fe6bdbf7..e65ae0a2 100644 --- a/windows/utils/split_into_argv.c +++ b/windows/utils/split_into_argv.c @@ -161,6 +161,11 @@ #define MOD3 0 #endif +static inline bool is_word_sep(char c) +{ + return c == ' ' || c == '\t'; +} + void split_into_argv(char *cmdline, int *argc, char ***argv, char ***argstart) { @@ -173,7 +178,7 @@ void split_into_argv(char *cmdline, int *argc, char ***argv, * First deal with the simplest of all special cases: if there * aren't any arguments, return 0,NULL,NULL. */ - while (*cmdline && isspace((unsigned char)*cmdline)) cmdline++; + while (*cmdline && is_word_sep(*cmdline)) cmdline++; if (!*cmdline) { if (argc) *argc = 0; if (argv) *argv = NULL; @@ -195,7 +200,7 @@ void split_into_argv(char *cmdline, int *argc, char ***argv, bool quote; /* Skip whitespace searching for start of argument. */ - while (*p && isspace((unsigned char)*p)) p++; + while (*p && is_word_sep(*p)) p++; if (!*p) break; /* We have an argument; start it. */ @@ -206,7 +211,7 @@ void split_into_argv(char *cmdline, int *argc, char ***argv, /* Copy data into the argument until it's finished. */ while (*p) { - if (!quote && isspace((unsigned char)*p)) + if (!quote && is_word_sep(*p)) break; /* argument is finished */ if (*p == '"' || *p == '\\') { From 4d92ca80de4262f3f857cd04041bb07c071a4e97 Mon Sep 17 00:00:00 2001 From: Jacob Nevins Date: Fri, 17 Mar 2023 16:48:19 +0000 Subject: [PATCH 32/46] Windows installer: restore InstallScope setting. This reverts commit 0615767224a5a5234b1916d45eca29cec1fe4a59 ("Windows installer: remove explicit InstallScope setting"), albeit with different comments. The original change worked around a Windows security vulnerability (CVE-2023-21800), but also resulted in a rather broken installer. (cherry picked from commit cedeb75d59ce0c4b5b18270dbada6976ba07d7b1) --- windows/installer.wxs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/windows/installer.wxs b/windows/installer.wxs index 65a7f379..25e8008a 100644 --- a/windows/installer.wxs +++ b/windows/installer.wxs @@ -121,6 +121,16 @@ Language="1033" Codepage="1252" Version="$(var.Winver)">