Large chunks of the GTK setup code had a 2-space indent for some
reason, in place of the usual 4-space in this code base. I've been
meaning to sort it out for ages, because it makes it hard to have a
single set of editor settings suitable for the whole code base.
The polynomial Stein's algorithm in that code was adapted from the
binary Stein in mpint.c. One of the comments which originally said
'dividing by 2' should have been updated to say 'dividing by x' in the
polynomial case, and didn't.
If you had a key stored encrypted in Pageant, and you launched two
PuTTY sessions both trying to generate a signature with the key, and
then didn't type the passphrase into the async passphrase prompt until
after both sessions had made requests, then only one of the requests
would be replied to once you decrypted the key. The other would sit
there waiting for a response that Pageant never got round to sending.
This would also happen in the case where you launched two PuTTY
sessions each trying to use a _different_ encrypted key in Pageant, in
which case Pageant should have presented the next GUI passphrase box
after the first one was dismissed. That didn't happen either.
This was the result of two separate bugs in the code. Firstly, when
signop_coroutine() noticed that a GUI request was already in progress,
it would crReturn without making any arrangement to be called back.
Now there's a queue of 'requests blocked on waiting for some GUI
prompt to finish'.
Secondly, if multiple requests were blocked on the same key, the code
that went over the linked list of them had a bug in which it would
unblock the _first_ list item in every iteration, instead of
unblocking all the items once each.
ctrl_radiobuttons has a variadic argument list that it expects to be
terminated by a null pointer where a const char * should be. So the
terminating NULL at each call site ought to be cast to const char *,
for the usual reason (NULL might expand literally to something not the
same size as a pointer and get mis-marshalled in the variadic argument
list). It wasn't being.
Fixed in the same way as dupcat: I've turned ctrl_radiobuttons into a
macro wrapper on the underlying function, which adds the NULL
automatically with its correct cast. This also saves verbiage at every
call site, because now I can leave out all the NULLs there.
Instead of maintaining a single sparse table mapping Unicode to the
currently selected code page, we now maintain a collection of such
tables mapping Unicode to any code page we've so far found a need to
work with, and we add code pages to that list as necessary, and never
throw them away (since there are a limited number of them).
This means that the wc_to_mb family of functions are effectively
stateless: they no longer depend on a 'struct unicode_data'
corresponding to the current terminal settings. So I've removed that
parameter from all of them.
This fills in the missing piece of yesterday's commit a216d86106:
now wc_to_mb too should be able to handle internally-implemented
character sets, by hastily making their reverse mapping table if it
doesn't already have it.
(That was only a _latent_ bug, because the only use of wc_to_mb in the
cross-platform or Windows code _did_ want to convert to the currently
selected code page, so the old strategy worked in that case. But there
was no protection against an unworkable use of it being added later.)
A user points out that the new charset-aware window title setting
doesn't work if the configured character set is one of the entries in
cp_list[] based on a hard-coded Unicode translation table, such as the
ISO 8859 family.
That's because the Windows mb_to_wc() function assumes that the code
page it's given will always be OK to pass to the Windows API function
MultiByteToWideChar, forgetting that for those internally implemented
single-byte character sets are not.
This commit adds a manual implementation of SBCS -> Unicode based on
those tables, which restores the ability to set a window title
specified in ISO 8859.
However, it's not a full fix to windows/unicode.c in general, because
wc_to_mb has a similar blind spot: it's only prepared to convert
Unicode to an internally implemented SBCS if that SBCS happens to be
the one currently set in ucsdata->line_codepage, because that's when
we've already prepared the reverse lookup table. Probably we ought to
sort that out, and arrange that it can make the reverse lookup table
if suddenly called on to do a different conversion. But that needs
more refactoring, so I haven't done it in this commit.
This is the first release for which I've had to submit a revised Store
entry, and now I've worked out how to do it, I should write it down
for next time.
In the echo "\\versionid foo" statement, the double \ turns into a
single \ during dash's expansion phase, and the remaining '\v' turns
into a vertical tab when dash's 'echo' builtin processes it. We need
twice as many \ to generate a literal \ in the actual output.
I'd forgotten to initialise it at all, which meant it was set to zero
by the initial memset of the whole BidiContext on creation. But in our
enumeration of bidi character types, zero corresponds to L (the most
common left-to-right alphabetic character class), and as a value for
paragraphOverride, that is not neutral.
As a result, a command such as this (assuming UTF-8)
echo -e '\xD7\x90\xD7\x91'
would produce Hebrew aleph and beth in the correct display order
(aleph on the right), but aligned to the left margin of the terminal
instead of the right margin, because the overall direction of the line
was taken to be forcibly overridden to "left-to-right" instead of
being inferred dynamically from the line contents.
do_bidi() is a tiny wrapper on the inner function that does all the
real work. And the inner function has been subjected to the whole
Unicode 14 bidi conformance test. So naturally, the "trivial" but
untested function just outside it is where the embarrassing bug was.
Fixes a cosmetic issue where the new ConPTY error added in 4ae8b742ab
had an ugly "Unable to open connection to".
(Arguably this ought to test a backend property rather than
cmdline_tooltype.)
I thought of a couple more things it's worth double-checking at
release time, and also, moved the 'merge to main' instructions from
the RC build step to the final release phase, because that way they
don't have to be pointlessly redone if commits have appeared on main
in between.
This makes pterm.exe support the same (very small) subset of the
standard option collection that Unix pterm does. Namely, -load (which
won't do anything useful with a hostname to connect to, but is still
useful if you have a saved session containing configuration like
colours or default size or what have you), and also -sessionlog.
To make this work, I've had to move the 'tooltype' definition out of
window.c into {putty,pterm}.c, so that it can be defined differently
in the two.
They were accepted by Unix pterm, by mistake, and then totally ignored
because nothing in pterm ever makes a network connection, so nothing
cares whether you configured it to use IPv4 or IPv6.
Turns out that standard C 'size_t' and the Win32 API's 'SIZE_T' aren't
the same integer type in all cases! They have the same _size_, but in
32-bit, one of them is a typedef for 'unsigned int' and the other for
'unsigned long', leading to annoying pointer-type mismatch warnings.
This matches the way it's done in the GTK backend: the only thing that
happens inside seat_notify_remote_exit is that we schedule a toplevel
callback, and all the real work happens later on when that callback is
called.
The particular situation where this makes a difference is if you
perform a user abort during proxy authentication (e.g. hit ^D at a
proxy password prompt), so that the main SSH backend gets
PLUGCLOSE_USER_ABORT and calls ssh_user_close. That doesn't
immediately close the socket; it just sets ssh->pending_close,
schedules a run of ssh_bpp_output_raw_data_callback, and returns.
So if seat_notify_remote_exit calls back _synchronously_ to
ssh_return_exitcode, it will see that the socket is still open and
return -1. But if it schedules a callback and waits, then the callback
to ssh_bpp_output_raw_data_callback will happen first, which will
close the socket, and then the exit processing will get the right
answer.
So the user-visible effect is that if you ^D a proxy auth prompt, GTK
PuTTY will close the whole window (assuming you didn't set close-on-
exit to 'never'), whereas Windows PuTTY will leave the window open,
and not even know that the connection is now shut (in that it'll still
ask 'Are you sure you want to close this session?' if you try to close
it by hand).
With this fix, Windows PuTTY behaves the same as GTK in this respect.
cmake doesn't have convincing facilities for doing this in its install
step, so the new advice is to do it manually (we've provided no
equivalent to the autotools --enable-setuid or --enable-setgid options,
nor UTMP_USER/GROUP).
Without this, the build of e.g. psusan would fail on systems without
that header (such as Termux on Android).
This is similar to how things were pre-cmake, but not identical. We used
to treat lack of updwtmpx() as a reason to OMIT_UTMP (as of f0dfa73982),
but usage of that function got conditionalised in c19e7215dd, so I
haven't restored that exclusion.
When the user provides a password on the PuTTY command line, via -pw
or -pwfile, the flag 'tried_once' inside cmdline_get_passwd_input() is
intended to arrange that we only try sending that password once, and
after we've sent it, we don't try again.
But this plays badly with the 'Restart Session' operation. If the
connection is lost and then restarted at user request, we _do_ want to
send that password again!
So this commit moves that static variable out into a small state
structure held by the client of cmdline_get_passwd_input. Each client
can decide how to manage that state itself.
Clients that support 'Restart Session' - i.e. just GUI PuTTY itself -
will initialise the state at the same time as instantiating the
backend, so that every time the session is restarted, we return
to (correctly) believing that we _haven't_ yet tried the password
provided on the command line.
But clients that don't support 'Restart Session' - i.e. Plink and file
transfer tools - can do the same thing that cmdline.c was doing
before: just keep the state in a static variable.
This also means that the GUI login tools will now retain the
command-line password in memory, whereas previously they'd have wiped
it out once it was used. But the other tools will still wipe and free
the password, because I've also added a 'bool restartable' flag to
cmdline_get_passwd_input to let it know when it _is_ allowed to do
that.
In the GUI tools, I don't see any way to get round that, because if
the session is restarted you _have_ to still have the password to use
again. (And you can't infer that that will never happen from the
CONF_close_on_exit setting, because that too could be changed in
mid-session.) On the other hand, I think it's not all that worrying,
because the use of either -pw or -pwfile means that a persistent copy
of your password is *already* stored somewhere, so another one isn't
too big a stretch.
(Due to the change of -pw policy in 0.77, the effect of this bug was
that an attempt to reconnect in a session set up this way would lead
to "Configured password was not accepted". In 0.76, the failure mode
was different: PuTTY would interactively prompt for the password,
having wiped it out of memory after it was used the first time round.)
When talking to a web proxy which requires a password, our HTTP proxy
code sends an initial attempt to connect without authentication,
receives the 407 status indicating that authentication was required
(and which kind), and then closes and reopens the connection (if given
"Connection: close"). Then, on the next attempt, we try again with
authentication, and expect the first thing in the input bufchain to be
the HTTP status response to the revised request.
But what happened about the error document that followed those HTTP
headers? It - or at least some of it - would already have been in the
input bufchain.
With an HTTP/1.1 proxy, we already read it and discarded it (either
via a Content-length header or chunked transfer encoding), before we
set the 'reconnect' flag. So, assuming the proxy HTTP server is
behaving sensibly, our input bufchain ought to be empty at the point
when we start the fresh connection.
But if the proxy only speaks HTTP/1.0 (which does still happen -
'tinyproxy' is a still-current example), then we didn't get a
Content-length header either, so we didn't run any of the code that
skips over the error document. (And HTTP/1.0 implicitly has
"Connection: close" semantics, which is why that doesn't matter.) As a
result, some of it would still be in the input bufchain, and never got
cleared out, and we'd try to parse _that_ as if it was the HTTP
response from the second network connection.
The simple solution is that when we close and reopen the proxy network
connection, we also clear the input bufchain, so that the fresh
connection starts from a clean slate.
Commit 5390aef3fc broke it, because GTK1 has neither
gtk_label_set_selectable nor gtk_widget_set_can_focus. Happily, those
are both more or less optional (only a minor UI awkwardness arises
from not having them), so I'll just condition them out.
If the user holds down Alt-> so that the key repeats, then a second
call to change_font_size can occur while the window resize from the
previous one has yet to complete. This leads to the new pixel size of
the window from resize #1 being interpreted in the light of the font
size from reesize #2, so that the two get out of step and the
_character_ size of the terminal changes as a result.
The simplest fix is to disallow starting a second font-size-based
window resize while the first is still in flight - which, now that the
'win_resize_pending' flag lives in window.c and not terminal.c, is
easy to achieve.
In the changes around commit 420fe75552, I made the terminal
suspend output processing while it waited for a term_size() callback
in response to a resize request. Because on X11 there are unusual
circumstances in which you never receive that callback, I also added a
last-ditch 5-second timeout, so that eventually we'll resume terminal
output processing regardless.
But the timeout lives in terminal.c, in the cross-platform code. This
is pointless on Windows (where resize processing is synchronous, so we
always finish it before the timer code next gets called anyway), but I
decided it was easier to keep the whole mechanism in terminal.c in the
absence of a good reason not to.
Now I've found that reason. We _also_ generate window resizes locally
to the GTK front end, in response to the key combinations that change
the font size, and _those_ still have an asynchrony problem.
So, to begin with, I'm refactoring the request_resize system so that
now there's an explicit callback from the frontend to the terminal to
say 'Your resize request has now been processed, whether or not you've
received a term_size() call'. On Windows, this simplifies matters
greatly because we always know exactly when to call that, and don't
have to keep a 'have we called term_size() already?' flag. On GTK, the
timing complexity previously in terminal.c has moved into window.c.
No functional change (I hope). The payoff will be in the next commit.
This is a partial cherry-pick of commit de66b0313a from main,
which allows all the forms of OSC sequence termination to apply in the
preliminary states as well as OSC_STRING.
The reporting user only mentioned the case of OSC 112 BEL, and not the
various forms of ST. So the former is actually known to be occurring
in the wild, and is also the least complicated part of the full patch
on main. Therefore I think this part is worthwhile and reasonably safe
to cherry-pick to 0.77 just before a release, whereas I'd be
uncomfortable making the rest of the changes at this late stage.
A user reports that the xterm OSC 112 sequence (reset cursor colour)
is sometimes sent as simply OSC 112 BEL, rather than OSC 112 ; BEL.
When xterm parses this, the BEL still acts as an OSC terminator, even
though it appears before the separating semicolon that shifts into the
'absorb the notional command string' state.
PuTTY doesn't support that sequence at all. But currently, the way it
doesn't support it is by treating the BEL completely normally, so that
you get an annoying beep when a client application sends that
abbreviated sequence.
Now we recognise all the OSC terminator sequences even in the OSC
setup termstates, as well as the final OSC_STRING state. That goes
equally for BEL, ST in the form of ESC \, ST in the form of
single-byte 0x9C, and ST in the UTF-8 encoding.
