A couple of users report that my recent reworking of the Windows
top-level message loop has led to messages occasionally being lost,
and MsgWaitForMultipleObjects blocking when it ought to have been
called with a zero timeout. I haven't been able to reproduce this
myself, but according to one reporter, PeekMessage(PM_NOREMOVE) is
effective at checking for a non-empty message queue in a way that
GetQueueStatus is not. Switch to using that instead. Thanks to Eric
Flumerfelt for debugging and testing help.
[originally from svn r10057]
Automake now insists that we run AM_PROG_AR if we're going to build a
library, and AM_PROG_CC_C_O if we're going to build anything with
extra compile options. Those extra macros seem harmless in previous
versions of automake.
[originally from svn r10053]
Jochen Erwied points out that once you've used PeekMessage to remove
_one_ message from the message queue, MsgWaitForMultipleObjects will
consider the whole queue to have been 'read', or at least looked at
and deemed uninteresting, and so it will block until a further message
comes in. Hence, my change in r10040 which stops us from looping on
PeekMessage until the queue is empty has the effect of causing the
rest of the message queue not to be acted on until a new message comes
in to unblock it. Fix by checking if the queue is nonempty in advance
of calling MsgWaitForMultipleObjects, and if so, giving it a zero
timeout just as we do if there's a pending toplevel callback.
[originally from svn r10052]
[r10040 == 5c4ce2fadf]
No current PuTTY utility was calling random_ref more than once per run
(ssh.c and the two main PuTTYgen programs call it once each), but if
one ever does (or if derived code does), it will want the reference
count to actually work sensibly.
[originally from svn r10049]
Martin Prikryl helpfully points out that when I revamped the socket
error mechanism using toplevel callbacks, I also accidentally passed
the error code to the wrong function. Use winsock_error_string instead.
[originally from svn r10048]
Unix GUI programs should not say 'Fatal Error' in the message box
title, and Plink should not destroy its logging context as a side
effect of printing a non-fatal error. Both appear to have been due to
inattentive cut and paste from the pre-existing fatal error functions.
[originally from svn r10044]
I temporarily applied it as a means of testing the revised event loops
in r10040, and accidentally folded it into my final commit instead of
backing it out. Ahem.
[originally from svn r10042]
[r10040 == 5c4ce2fadf]
In r10020 I carefully reimplemented using timing.c and callback.c the
same policy for large pastes that the previous code appeared to be
implementing ad-hoc, which included a 450ms delay between sending
successive lines of pasted text if no visible acknowledgment of the
just-sent line (in the form of a \n or \r) came back from the
application.
However, it turns out that that *wasn't* what the old code was doing.
It *would* have done that, but for the bug that it never actually set
the 'last_paste' variable, and never has done since it was first
introduced way back in r516! So the policy I thought had been in force
forever has in fact only been in force since I unwittingly fixed that
bug in r10020 - and it turns out to be a bad idea, breaking pastes
into vi in particular.
So I've removed the timed paste code completely, on the basis that
it's never actually worked and nobody seems to have been unhappy about
that. Now we still break large pastes into separate lines and send
them in successive top-level callbacks, and the user can still press a
key to interrupt a paste if they manage to catch it still going on,
but there's no attempted *delay* any more.
(It's possible that what I *really* ought to be doing is calling
back->sendbuffer() to see whether the backend is consuming the data
pasted so far, and if not, deferring the rest of the paste until the
send buffer becomes smaller. Then we could have pasting be delayed by
back-pressure from the recipient, and still manually interruptible
during that delay, but not have it delayed by anything else. But what
we have here should at least manage to be equivalent to the *actual*
rather than the intended old policy.)
[originally from svn r10041]
[r516 == 0d5d39064a]
[r10020 == 7be9af74ec]
This change attempts to reinstate as a universal property something
which was sporadically true of the ad-hockery that came before
toplevel callbacks: that if there's a _very long_ queue of things to
be done through the callback mechanism, the doing of them will be
interleaved with re-checks of other event sources, which might (e.g.)
cause a flag to be set which makes the next callback decide not to do
anything after all.
[originally from svn r10040]
Anthony Ho reports that this can occur naturally in some situation
involving Windows 8 + IE 11 and dynamic port forwarding: apparently we
get through the SOCKS negotiation, send our CHANNEL_OPEN, and then
*immediately* suffer a local WSAECONNABORTED error before the server
has sent back its OPEN_CONFIRMATION or OPEN_FAILURE. In this situation
ssh2_channel_check_close was failing to notice that the channel didn't
yet have a valid server id, and sending out a CHANNEL_CLOSE anyway
containing 32 bits of uninitialised nonsense.
We now handle this by turning our half-open CHAN_SOCKDATA_DORMANT into
a half-open CHAN_ZOMBIE, which means in turn that our handler
functions for OPEN_CONFIRMATION and OPEN_FAILURE have to recognise and
handle that case, the former by immediately initiating channel closure
once we _do_ have the channel's server id to do it with.
[originally from svn r10039]
It looks as if it's never worked at all: it had a spurious second
printf, it completely forgot to allow for the uint32 type code that
SSH2_MSG_CHANNEL_DATA doesn't have, it accessed the channel state's
sequence number fields in a way that made no sense and didn't match
the rest of the program, *and* it misinvoked the file opening API. I
must have never had an occasion to test it.
[originally from svn r10037]
Previously it would throw a bunch of Perl undefined-variable-usage
warnings; now it cleanly detects the problem, dumps as much of the
message as it still reasonably can, and doesn't update any channel
states.
[originally from svn r10036]
CHAN_AGENT channels need c->u.a.message to be either NULL or valid
dynamically allocated memory, because it'll be freed by
ssh_channel_destroy. This bug triggers if an agent forwarding channel
is opened and closed without having sent any queries.
[originally from svn r10032]
During the Conf revamp, I changed the internal representation of
dynamic forwardings so that they were stored as the conceptually
sensible L12345=D rather than the old D12345, and added compensation
code to translate to the latter form for backwards-compatible data
storage and for OpenSSH-harmonised GUI display. Unfortunately I forgot
that keys in the forwarding data can also prefix the L/R with a
character indicating IPv4/IPv6, and my translations didn't take
account of that possibility. Fix them.
[originally from svn r10031]
We now only present the full set of host key algorithms we can handle
in the first key exchange. In subsequent rekeys, we present only the
host key algorithm that we agreed on the previous time, and then we
verify the host key by simply enforcing that it's exactly the same as
the one we saw at first and disconnecting rudely if it isn't.
[originally from svn r10027]
It was one of those things that went in ages ago on Windows and never
got replicated in the Unix front end. And it needn't be: ldisc.c is a
perfect place to put it, since it knows which of the data it's sending
is based on a keystroke and which is automatically generated, and it
also has access to the terminal context. So now a keypress can
interrupt a runaway paste on all platforms.
[originally from svn r10025]
Again, I've removed the special-purpose ad-hockery from the assorted
front end message loops that dealt with deferred handling of socket
errors, and instead uxnet.c and winnet.c arrange that for themselves
by calling the new general top-level callback mechanism.
[originally from svn r10023]
Instead of having a special GTK idle function for dealing with session
closing, I now use the new top-level callback mechanism which is
slightly simpler for calling a one-off function.
Also in this commit, I've arranged for connection_fatal to queue a
call to the same session close function after displaying the message
box, with the effect that now all the same processing takes place no
matter whether the session closes cleanly or uncleanly - e.g. the SSH
specials submenu is cleaned out, as it should be.
[originally from svn r10022]
Instead of setting a must_close_session flag and having special code
in the message loop to check it, we'll schedule the call to
close_session using the new top-level callback system.
[originally from svn r10021]
I've removed the ad-hoc front-end bodgery in the Windows and GTK ports
to arrange for term_paste to be called at the right moments, and
instead, terminal.c itself deals with knowing when to send the next
chunk of pasted data using a combination of timers and the new
top-level callback mechanism.
As a happy side effect, it's now all in one place so I can actually
understand what it's doing! It turns out that what all that confusing
code was up to is: send a line of pasted data, and delay sending the
next line until either a CR or LF is returned from the server
(typically indicating that the pasted text has been received and
echoed) or 450ms elapse, whichever comes first.
[originally from svn r10020]
This is a little like schedule_timer, in that the callback you provide
will be run from the top-level message loop of whatever application
you're in; but unlike the timer mechanism, it will happen
_immediately_.
The aim is to provide a general way to avoid re-entrance of code, in
cases where just _doing_ the thing you want done is liable to trigger
a confusing recursive call to the function in which you came to the
decision to do it; instead, you just request a top-level callback at
the message loop's earliest convenience, and do it then.
[originally from svn r10019]
error with pr->c NULL, in which case calling sshfwd_unclean_close on
it will dereference NULL and segfault. Write an alternative error
handling path for that possibility.
(I don't know if it's the only way, but one way this can happen is if
you're doing dynamic forwarding and the socket error occurs during
SOCKS negotiation, in which case no SSH channel has been set up yet
because we haven't yet found out what we want to put in the
direct-tcpip channel open message.)
[originally from svn r10018]
pscp.c when I did the big revamp in r9279: I assumed that in any SCP
connection we would be the first to send EOF, but in fact this isn't
true - doing downloads with old-SCP, EOF is initiated by the server,
so we were spuriously reporting an error for 'unexpected' EOF when
everything had gone fine. Thanks to Nathan Phelan for the report.
[originally from svn r10016]
[r9279 == 947962e0b9]
handle. Revert that when we hackily call it from mkfiles.pl, so that
if I have a need to insert diagnostics in the latter they won't go
into the end of sbcsdat.c.
[originally from svn r10013]
than fonts. I broke this in r9559 when I added the option for 'both',
because the internal representation got offset by one so as to change
from a boolean to two bitfields and I must have confused myself about
what the default should be.
[originally from svn r10008]
[r9559 == bc6e0952ef]
bn_restore_invariant (and the many loops that duplicate it) leaves a
single zero word in a bignum representing 0, whereas the constant
'Zero' does not have any data words at all. Cope with this in
bignum_cmp.
(It would be a better plan to decide on one representation and stick
with it, but this is the less disruptive fix for the moment.)
[originally from svn r9996]
now check that all the modular functions (modpow, modinv, modmul,
bigdivmod) have nonzero moduli, and that modinv also has a nonzero
thing to try to invert.
[originally from svn r9987]
PuTTY does not trim a colon suffix off the hostname if it contains
_more than one_ colon. This allows IPv6 literals to be entered.
(Really we need to do a much bigger revamp of all uses of hostnames to
arrange that square-bracketed IPv6 literals work consistently, but
this at least removes a regression over 0.62.)
[originally from svn r9983]
[r9214 == a1f3b7a358]
not so silly in the 1990s and before I implemented scrollback
compression, but it's been a ridiculously low default for a while now.
[originally from svn r9982]
within the same string that destfname points to the start of, so
freeing it causes at best a double-free of destfname and more likely a
free of something that isn't even the start of an allocated block.
[originally from svn r9974]
[r9916 == cc4f38df14]
Simon Tatham
Jacob Nevins