When telling front ends to paint the screen, the terminal code treats
the cursor as an attribute applied to the character cell(s) it appears
in. do_paint() detects changes to most such attributes by storing what
it last sent to the front end in term->disptext and comparing that with
what it thinks should be displayed in the window. However, before this
commit the cursor was special. Its last-drawn position was recorded in
special structure members and invalidated parts of the display based on
those. The cursor attributes were treated as "temporary attributes" and
were not saved in term->disptext.
This commit regularizes this and turns the cursor attributes into normal
attributes that are stored in term->disptext. This removes a bunch of
special-case code in do_paint() because now the normal update code
handles the cursor properly, and also removes some members from the
Terminal structure. I hope it will also make future cursor-handling
changes (for instance for input method pre-editing) simpler.
This commit makes the required semantic changes but doesn't make the
rather more pervasive change of actually renaming the attributes from
TATTR_ to ATTR_. That will be in the next commit.
Up-to-date trunk clang has introduced a built-in operator called
_Countof, which is like the 'lenof' macro in this code (returns the
number of elements in a statically-declared array object) but with the
safety advantage that it provokes a compile error if you accidentally
use it on a pointer. In this commit I add a cmake-time check for it,
and conditional on that, switch over the definition of lenof.
This should add a safety check for accidental uses of lenof(pointer).
When I tested it with new clang, this whole code base compiled cleanly
with the new setting, so there aren't currently any such accidents.
clang cites C2y as the source for _Countof: WG14 document N3369
initially proposed it under a different name, and then there was a big
internet survey about naming (in which of course I voted for lenof!),
and document N3469 summarises the results, which show that the name
_Countof and/or countof won. Links:
https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3369.pdfhttps://www.open-std.org/jtc1/sc22/wg14/www/docs/n3469.htm
My reading of N3469 seems to say that there will _either_ be _Countof
by itself, _or_ lowercase 'countof' as a new keyword, but they don't
say which. They say they _don't_ intend to do the same equivocation we
had with _Complex and _Bool, where you have a _Countof keyword and an
optional header file defining a lowercase non-underscore macro
wrapping it. But there hasn't been a new whole draft published since
N3469 yet, so I don't know what will end up in it when there is.
However, as of now, _Countof exists in at least one compiler, and that
seems like enough reason to implement it here. If it becomes 'countof'
in the real standard, then we can always change over later. (And in
that case it would probably make sense to rename the macro throughout
the code base to align with what will become the new standard usage.)
GtkIMContext has focus_in and focus_out methods for telling it when the
corresponding widget gains or loses keyboard focus. It's not obvious to
me why these are necessary, but PuTTY now calls them when it sees
focus-in and focus-out events for the terminal window. Somehow, this
has caused Hangul input to start working in PuTTY. I can't yet
see what I'm typing for lack of proper preedit support, though.
[ECMA-48] section 8.3.27 specifies the format of Device Control String
(DCS) commands which are used for XTGETTCAP and other sequences.
We don't parse DCS commands. This causes this command to wrongly
output some characters:
printf '\033P+q616d\033\\'
Fix that by parsing DCS commands just like other OSC-like commands.
(Apart from the initial characters, DCS has the same format as OSC.)
We also allow 0x07 as sequence terminator which does not seem specified
but a lot of people use it with OSC; it's fine because 0x07 is not
allowed in the OSC/DCS payload.
[ECMA-48]: https://www.ecma-international.org/wp-content/uploads/ECMA-48_2nd_edition_august_1979.pdf
It was a bit far to the right, looking at risk of falling off. Now
moved it as far left as it will go without the top right corner of the
computer monitor peeking out from behind it.
If I'm going to use this as a means of generating bitmap icons at
large sizes, I want it to support all the same modes as the existing
bitmap script. So this adds a mode to the SVG generator that produces
the same black and white colour scheme as the existing monochrome
bitmap icons.
(Plus, who knows, the black and white SVGs might come in useful for
other purposes. Printing as a logo on black-and-white printers springs
to mind.)
The existing monochrome icons aren't greyscale: all colours are
literally either black or white, except for the cardboard box in the
installer icon, which is halftoned. Here I've rendered that box as
mid-grey. When I convert the rendered SVG output to an actual
1-bit (plus alpha) image, I'll have to redo that halftoning.
It looked nasty that the back corner of the monitor didn't line up
exactly with the outline of the system box behind it. Now I choose the
y offset between the two components to ensure it does. Also adjusted
the monitor's depth so that it fits better with the new alignment.
We weren't building _all_ the icons in true-colour mode, because most
don't change anyway. The installer ones do, so let's build them. Works
better with the preview page.
A user reported recently that if you connect to a Telnet server via a
proxy that requires authentication, and enter the auth details
manually in the PuTTY terminal window, then the entire Telnet session
is shown with trust sigils to its left.
This happens because telnet.c calls seat_set_trust_status(false) as
soon as it's called new_connection() to make the Socket. But the
interactive proxy authentication dialogue hasn't happened yet, at that
point. So the proxy resets the trust status to true and asks for a
username and password, and then nothing ever resets it to false,
because telnet.c thought it had already done that.
The solution is to defer the Telnet backend's change of trust status
to when we get the notification that the socket is properly connected,
which arrives via plug_log(PLUGLOG_CONNECT_SUCCESS).
The same bug occurs in raw.c and supdup.c, but not in rlogin.c,
because Rlogin has an initial authentication exchange known to the
protocol, and already delays resetting the trust status until after
that has concluded.
Colin Watson reported that a build failure occurred in the AArch64
Debian build of PuTTY 0.83:
gcc now defaults to enabling branch protection using AArch64 pointer
authentication, if the target architecture version supports it.
Debian's base supported architecture does not, but Armv8.4-A does. So
when I changed the compile flags for enable_dit.c to add
-march=armv8.4-a, it didn't _just_ allow me to write the 'msr dit, %0'
instruction in my asm statement; it also unexpectedly turned on
pointer authentication in the containing function, which caused a
SIGILL when running on a pre-Armv8.4-A CPU, because although the code
correctly skipped the instruction that set DIT, it was already inside
enable_dit() at that point and couldn't avoid going through the
unsupported 'retaa' instruction which tries to check an auth code on
the return address.
An obvious approach would be to add -mbranch-protection=none to the
compile flags for enable_dit.c. Another approach is to leave the
_compiler_ flags alone, and change the architecture in the assembler,
either via a fiddly -Wa,... option or by putting a .arch directive
inside the asm statement. But both have downsides. Turning off branch
protection is fine for the Debian build, but has the unwanted side
effect of turning it off (in that one function) even in builds
targeting a later architecture which _did_ want branch protection. And
changing the assembler's architecture risks changing it _down_ instead
of up, again perhaps invalidating other instructions generated by the
compiler (like if some later security feature is introduced that gcc
also wants to turn on by default).
So instead I've taken the much simpler approach of not bothering to
change the target architecture at all, and instead generating the move
into DIT by hardcoding its actual instruction encoding. This meant I
also had to force the input value into a specific register, but I
don't think that does any harm (not _even_ wasting an extra
instruction in codegen). Now we should avoid interfering with any
security features the compiler wants to turn on or off: all of that
should be independent of the instruction I really wanted.
We used to have a practice of \IM-ing every command-line option for the
index, but haven't kept it up.
Add these for all existing indexed command-line options, plus some
related tidying.
(That's Halibut's non-breaking hyphen.)
Triggered by noticing that the changes in 54f6fefe61 happened to come
out badly in the text-only rendering, but I noticed there were many more
instances in the main docs where non-breaking hyphens would help.
After all these years, this checklist is _still_ hard for me to get
right. In the 0.83 runup this month, I prepared everything about the
RC build in advance, but nothing about the announcements, website
updates etc, and had to do all of that on release day.
So I've completely removed the section "Preparing to make the
release", which was ambiguous about whether it's done in advance or on
the day. Now all the text parts (website, wishlist, announcements) are
folded into the "make a release candidate" section, in the hope that
I'll remember to do them all at the same time, which should mean
- people have a few days to review the text _and_ test the RC build
- because they go together, I also remember to revise the text if a
new RC build is needed (e.g. mention whatever extra fix it has).
The "actual release procedure" section is now down to _only_ the
things I have to do on the day, which is basically uploading
everything, going live, and communicating the release.
Spotted by Coverity: we've just allocated a strbuf to hold the output
of the classical half of the hybrid key exchange, but if that output
isn't generated due to some kind of failure, we forgot to free the
strbuf on exit.
This was introduced in commit e7acb9f6968d482, as a side effect of
also wanting to call wmemchr to find a NUL wide character in the
buffer returned from GetDlgItemTextW. But the previous commit has
superseded that code, so now we don't use wmemchr in this code base
any more. Remove the machinery that provides it, saving a now-useless
cmake configure-time check.
This rewrite, due to SATO Kentaro, uses GetWindowTextLength (which I
hadn't known about) to find the correct size to allocate the buffer
the first time, avoiding the need to keep growing it until a call to
GetDlgItemText doesn't have to truncate the result.
[SGT: the helper function do_filereq_w expects its filename size to be
in characters, not bytes, because it's used both as an index into the
wchar_t buffer and also as nMaxFile in the OPENFILENAMEW structure
which is also documented as measured in characters. So at the call
site it should be measured via lenof rather than sizeof. This patch
has done the same with the char version, which makes no functional
difference but keeps the code more consistent.]
A user reported that the following sequence of events leads to Pageant
crashing:
- load an encrypted key into Pageant for decryption later
- attempt to use the key, so that Pageant prompts for the passphrase
- before entering the passphrase, abort the attempt to use the
key (e.g. by closing the PuTTY that was trying to use it)
- now enter the passphrase at the Pageant prompt, once the need for
it has gone away.
Once the key is decrypted, unblock_requests_for_key() goes through the
linked list of blocked PageantSignOp attached to the private key
record it's just decrypted, and tries to unblock them. The
PageantSignOp belonging to the aborted Pageant request is still linked
on that list, which it shouldn't be, because it's also been freed by
pageant_unregister_client when that traversed the separate linked list
of PageantAsyncOp associated with that client connection. So the
private key's list of blocked requests contained a stale pointer.
Now PageantSignOp's implementation of the PageantAsyncOp free method
makes sure to unlink the signop from any list it's on before freeing
it.
This can come up, for example, if the terminal receives a ^E character
and has an empty answerback string configured.
Without this early return, we append zero bytes to ldisc's ordinary
bufchain input_queue, which is harmless; but we also append a
zero-length record to ldisc's list of (type, length) chunks describing
which parts of the input bufchain should be treated as interactive or
as coming from special dedicated keystrokes (e.g. telling Return apart
from ^M).
That zero-length record is not _immediately_ harmful, but when the
user next presses a key, it will have a different type from the empty
answerback data, so that another chunk record is appended to the list
after the zero-length one. And then ldisc_input_queue_callback goes
into a tight loop, because it keeps trying to consume bytes from the
start of the input bufchain but bounding the size at the length of the
first (type, length) chunk, which is zero. So it consumes 0 bytes,
finds the bufchain still isn't empty, and loops round again.
When retrieving Unicode text from an edit box in the GUI configurer,
we were using plain memchr() to look for a terminating NUL. But of
course you have to use wmemchr() to look for a UTF-16 NUL, or else
memchr() will generate a false positive on the UTF-16 version of (at
least) any ASCII character!
(I also have to provide a fallback implementation of wmemchr for the
w32old builds, which don't have it in the libc they build against.
It's as simple as possible, and we use the libc version where
possible.)
This is a combined cherry-pick of three consecutive commits from main:
b088d77d580b8f7 GTK: hard-code some last-ditch fallback fonts.
7f4cccde2ae53c0 GTK: fixes to the previous font fallback patch.
6155365076c47a8 GTK: switch the default to client-side fonts.
The combined effect is that now PuTTY's built-in default font is
client-side rather than server-side (advantaging Wayland and
disadvantaging legacy GTK1 builds, which seems like a sensible
tradeoff these days), and also, if the configured main font can't be
found, we'll try falling back to either the client- or server-side
default (whichever is available) before giving up completely and
whinging on standard error.
"server:fixed" was a good default when GTK1 was common and non-X11
environments were rare. Now it's the other way round - Wayland is very
common and the GTK1 configuration of PuTTY is legacy - so it's time to
make the default GTK font a client-side one.
Of course, anyone with an existing saved session (including Default
Settings) won't be affected by this change; it only helps new users
without an existing ~/.putty at all. That's why we _also_ need the
fallbacks introduced by the previous couple of commits. But we can at
least start making it sensible for new users.
(I considered keeping the #if, and switching it round so that it tests
GTK_CHECK_VERSION(2,0,0) rather than NOT_X_WINDOWS, i.e. selects the
client-side default whenever client-side fonts _are_ available,
instead of only when server-side fonts _aren't_. That way, in GTK1
builds, the Conf default font would _still_ be "server:fixed". But I
think this is firstly too marginal to worry about, and secondly, it's
more futureproof to make the default the same everywhere: if anyone
still stuck on a GTK1 environment later manages to update it, then
their saved settings are less likely to have had a legacy thing
written into them. And the GTK1 build will still run out of the box
because of the last-ditch fallback mechanism I've just added.)
I'd forgotten that I'd already chosen a default client-side font, for
NOT_X_WINDOWS builds. I should have made the two defaults match! Now
both default font names are defined in the header file.
If the user's choice of fonts can't be instantiated during initial
terminal-window setup, then we now automatically try two fallback
options, "client:Monospace 10" and "server:fixed", and only give a
fatal error if _no_ option allows us to open a terminal window.
Previously, on Wayland, PuTTY and pterm with default configuration
would completely fail to open a terminal window at all, because our
default font configuration is still the trad X11 "server:fixed", and
on Wayland, X11-style server-side bitmap fonts don't exist at all.
Conversely, in the GTK1 build of PuTTY which we're still supporting,
_client-side_ fonts aren't supported, so if a user had configured one
in their normal PuTTY configuration, then the GTK1 version would
similarly fail to launch.
Now both of those cases should work, because the fallbacks include a
client-side font _and_ a server-side one, and I hope that any usable
Pango system will make "Monospace" map to _some_ locally available
font, and that any remotely sensible X server has 'fixed'
I think it would be even better if there was a mechanism for the Conf
to specify a fallback list of fonts. For example, this might be
specified via a new multifont prefix along the lines of
choices:client:Monospace 10:server:fixed
with the semantics that the "choices:" prefix means that the rest of
the string is split up at every other colon to find a list of fonts to
try to make. Then we could not only set PuTTY's default to a list of
possibilities likely to find a usable font everywhere, but also, users
could configure their own list of preferred fallbacks.
But I haven't thought of a good answer to the design question of what
should happen if a Conf font setting looks like that and the user
triggers the GUI font selector! (Also, you'd need to figure out what
happened if a 'choices:' string had another 'choices' in it...)
The cmake script that determines the current git head commit, in order
to bake it into the binaries for development builds from a git
checkout, wasn't working reliably on Windows: sometimes it reported
that the source directory didn't seem to be a git repository, when in
fact it was.
This occurred because gitcommit.cmake starts by trying to work out
_whether_ the source directory is the top level of a git worktree at
all, by the technique of running `git rev-parse --show-toplevel` (to
print the top-level path of the git worktree _containing_ $PWD, if
any) and comparing it with ${CMAKE_SOURCE_DIR}. But the comparison was
done as a plain string, which leads to problems if more than one
string can represent the same actual directory.
On Windows, this can occur for two reasons that I know of. One reason
is related to Windows itself: if you map a network file server path to
a local drive letter, then the same directory is accessible as a UNC
path (e.g. \\hostname\share\subdir) and via the drive letter (e.g.
N:\subdir). And it can happen that CMAKE_SOURCE_DIR and git's output
disagree on which representation to use, causing the string comparison
to return a false negative.
(This can also affect filesystems in a WSL instance, accessed from
native Windows via \\wsl$\instance\path, because Windows implements
that as a network file share even though the network in question is
purely in the imagination of that one machine.)
The other reason is related more specifically to git, because some
versions of Windows git are built on top of MSYS or MINGW or that kind
of shim layer, and model Windows drive letters as subdirectories of a
Unixlike VFS root. So you might also find that the two strings
disagree on whether you're in C:\Users\alice\src\putty or
/c/Users/alice/src/putty.
I think this commit should work around both of these problems. Reading
the man page for `git rev-parse` more carefully, it has an option
`--show-cdup`, which returns a _relative_ path from $PWD to the
top-level directory of the worktree: that is, it will be a sequence of
`../../..` as long as necessary, including length zero. So you can use
that to directly query whether you're at the top of a git worktree: if
`git rev-parse --show-cdup` returns the empty string and a success
status, then you are. (If you're deeper in a worktree it will return a
non-empty string, and if you're not in a worktree at all it will
return a failure status and print an error message to stderr.)
This is a cherry-pick of Stefan Tauner's patch from main, but without
my followup refactoring, since the refactoring seemed to me to have a
(small but easily avoidable) chance of introducing a bug in 0.83.
The only downside of the original patch is that it contains a variable
name telling a lie: 'osc_is_apc' should really read 'this isn't an OSC
but one of APC, SOS and PM'. But we don't actually treat those three
things differently, so the functionality is fine.
(cherry picked from commit b6b95f23e563211437e51322edc9118b63a3ca40)
I didn't like the previous patch setting a flag claiming that two
kinds of thing were APC which aren't in fact APC. So I've made a
little enum to distinguish them in the code. There's an outside chance
we might want to handle some case of these in future, in which case
this makes it easier, but more likely it's just making me feel less
wrong about it.
But I've also folded the two existing flags osc_is_apc and osc_w into
that single enum field, which I think is an improvement.
SOS (Start of string) and PM (privacy message) are opening delimiters defined
in ECMA-48 similar to APC. With this change they are treated exactly like APC,
i.e., like fake OSC sequences that are ignored and not printed.
Signed-off-by: Stefan Tauner <stefan.tauner@artech.at>
