When we create a socket with socket() (in try_connect, sk_newlistener, and
ipv4_is_local_addr) also call SetHandleInformation to disable handle
inheritance for this socket. This fixes dup-sessions-dont-close.
This commit also updates the dumps of Plink's and PSCP's help output,
adding the -proxycmd option to both and the -shareexists option to
Plink.
(Or rather, _re_-adding the latter, since it was introduced in error
by commit 07af4ed10 due to a branch management error and hastily
removed again in 29e8c24f9. This time it really does match reality.)
These reflect the fact that I'm increasingly preferring to make a
release-candidate build a few days in advance of the actual release,
and give the team time to do a bit of testing on it before putting it
up on the live website. Hence, I can't _quite_ fill in everything in
the website announcement ahead of time - the release date has to wait
until we know what it is, which means I need to have 'now fill in the
release date' as part of the go-live checklist.
Also, I've provided a better bob command for doing a release build
(putting it somewhere safe to begin with, rather than leaving it
precariously in my normal build directory). And while I'm here, I've
standardised all my placeholder version numbers to X.YZ; previously
half of them said 0.XX :-)
I had accidentally included the experimental "XT" app class (the
GtkApplication-based packaging of Unix PuTTY/pterm for OS X) among the
things that should still be built even when GTK is absent. That's
definitely wrong.
Several places in the docs were labelled for review because they need
to change when 0.68's feature set comes in - no SSH-2 to SSH-1
fallback any more, but on the other hand, Unix Pageant now exists.
When a handle socket is in THAWING state and handle_socket_unfreeze is
gradually passing the backlogged data on to the plug, the plug might
suddenly turn round and close the socket in the course of handling
plug_receive(), which means that handle_socket_unfreeze had better be
careful not to have had everything vanish out from under it when that
call returns. To solve this, I've added a 'deferred close' flag which
handle_socket_unfreeze can set around its call to plug_receive, and
handle_socket_close will detect that and not actually free the socket,
instead leaving that for handle_socket_unfreeze to do under its own
control.
(In a XXX-REVIEW-BEFORE-RELEASE form.)
Also, note the effect of compilation with different Visual Studio
versions on Windows version compatibility in the source README, for the
sake of having it written down somewhere.
They were there mainly to distinguish from 16-bit Windows, which hasn't
been a thing since before a noticeable fraction of the userbase were
born, probably. These days the obvious comparison is with 64-bit
Windows.
Also tweak some wording to reflect that official PuTTY executables are
not necessarily 32-bit any more, and add some XXX-REVIEW-BEFORE-RELEASE
in the same vein.
We responded to a local error writing out the destination file by
going into a mode where we just looped round receiving and throwing
away data until our count of the file size reached the file size we
expected - but once we were in that mode, we never actually
incremented the count!
xfer_download_data could return actuallen as either 0 or -1 to
indicate EOF. Now it's always 0, and scp_recv_filedata actually checks
for that case and reports an error.
When called with -V to ask for our version, return 0 rather than 1.
This is the usual behaviour observed by ssh(1) and other Unix commands.
Also use exit() rather than cleanup_exit() in pscp.c and psftp.c ; at
this point we have nothing to cleanup!
It's obvious to the trained eye whether GTK PuTTY was compiled against
GTK2 or GTK3, but the untrained eye would probably appreciate a little
help, and even the trained eye probably can't tell GTK 3.18 from 3.19
at a glance :-)
Firstly, I had asserted that data would never arrive on a handle
socket in state FREEZING, which is just an error, because FREEZING is
precisely the state of not being quite frozen _yet_ because one last
read is still expected to arrive from the winhandl.c reading subthread
which it's too late to cancel. I meant to assert that it wasn't
FROZEN.
Secondly, when the handle socket was in state FREEZING, I failed to
actually _set_ it to FROZEN.
And thirdly, when the handle socket starts thawing again (i.e. there's
now outgoing buffer space so we can start sending our backlogged
data), I forgot to ever call bufchain_consume, so that the same block
of data would get sent repeatedly.
I can only assume that nothing I've ever done has actually exercised
this code!
If we try to interpret a string argument as the name of a key file,
sometimes we it's in circumstances where we _know_ it's a key file, so
we must print an error message and return failure if the file can't be
loaded. Other times it's not, and we just fall back to interpreting
the argument in some other way (e.g. as a pattern match against the
comment or fingerprint of a key already in the agent).
My code dealing with failure returns from the public-key loading
functions were mishandling the latter case, if they identified a file
as existing and looking more or less like some kind of key file but
then it turned out to have a format error; they would try to copy and
return a public key that they didn't actually have. Even if
pageant_pubkey_copy avoided crashing as a result, this would still
inhibit the fallback to treating the input string as some other kind
of pattern match.
I think all of the cases in this switch must have originally said
(shift_state ? 'this' : 'that'), and in all but the VK_NUMPAD5 case
the two options were different, and I left VK_NUMPAD5 containing a
redundant ?: just to make it line up in a nice table with the others.
But now the others all have more options than that because I had to
support Ctrl as well as Shift modifiers, so there's no reason to have
that silly ?: lingering around (and it annoys Coverity).
Avoided referring to some functions and header files that aren't there
in the winelib world (_vsnprintf, _stricmp, SecureZeroMemory,
multimon.h), and worked around a really amazingly annoying issue in
which Winelib objects to you using the type 'fd_set' unless you
included winsock2.h before stdlib.h.
I think an agent sending a string length exceeding the buffer bounds
by less than 4 could have made PuTTY read beyond its own buffer end.
Not that I really think a hostile SSH agent is likely to be attacking
PuTTY, but it's as well to fix these things anyway!
The loops that were supposed to count up the number of buttons in the
variadic argument list forgot to increment the counter.
On the other hand, these functions aren't actually _used_ anywhere in
the current code - looks as if commit 616c837cf was the last time they
were seen - but manual dialog stuff like PuTTYgen might yet find a use
for them in future.
I think these were not strictly necessary, since passing a null
pointer to access(2) would have resulted in EINVAL rather than a
segfault. But it's clearer to put them in (and keeps static checkers a
bit happier).
Coverity observes that sometimes 'struct tm' can have other fields
(e.g. glibc's tm_gmtoff), so it's as well to make sure we initialise
the whole thing to zero.
Thanks, Coverity - I must have been lucky that Unix Pageant in client
mode hasn't so far happened to have this field come out non-NULL, or
else pageant_pubkey_copy would have tried to dupstr a garbage pointer.
This makes it clearer that it doesn't persist beyond this block, and
would have made it much more obvious that the assignment to it removed
in the previous commit was pointless.
Assignments that are overwritten shortly afterwards and never used,
and a completely unused variable. Also, the bogus array access in
testbn.c could have actually accessed one beyond the array limit
(though of course it's only in a test harness).
Now it's always freed in the cleanup epilogue (unless we're returning
it), rather than ad-hoc earlier in the code. That should make it more
reliably freed on error paths.
Bring it into line with the other PuTTY tools: --help goes to stdout,
has a one-line purpose and brief version information, --version ends
with a newline.
(Also, tweak text bring the help message under 80 columns.)
Partly to reassure the user that they got what they asked for, and
partly so that's a clue for us in the logs when we get bug reports.
This involved repurposing platform_psftp_post_option_setup() (no longer
used since e22120fe) as platform_psftp_pre_conn_setup(), and moving it
to after logging is set up.
These must have been absent-mindedly copied from function declarations
of the form 'const type *fn(args)', where the 'const' is meaningful
and describes the data pointed to by the returned pointer, to
functions of the form 'const type fn(args)' where the 'const' is
completely pointless.
Jacob Nevins
Owen Dunn
Simon Tatham