My motivation at the moment is that I'm trying to share a
git checkout between multiple VMs using VirtualBox's Shared
Folders feature. git in the guest VM isn't able to see the
executable bits from the host due to the VirtualBox/SMB/CIFS
layer. Instead, it thinks text files are non-executable,
unless they have a shebang line. That's a sensible way to
set the flags anyway, so set them like that.
With this commit, there's still one file that isn't handled:
src/shared/GetCommitHash.cmd. It's still marked executable,
but it lacks a shebang line, so the guest thinks it's
non-executable. I'm not sure it should be changed.
So far, I've only tested it on Windows 7. The test passed on that OS:
creating a screen buffer isn't blocked by selection, but writing to an
inactive buffer *is* blocked, regardless of whether the buffer was created
before or after selection began. The use of Mark or SelectAll doesn't
affect behavior.
If we're only examining the table for debugging purposes, then read only
first 1000 fonts and dump them.
If we're reading the font table on XP, then use the undocumented
GetNumberOfConsoleFonts API. Avoid this API on Vista and up, because it's
undocumented. It ought to be stable enough on XP.
When ConEmu has adopted the agent console (e.g. there is a ConEmu tab for
the agent's console), then every font in the table will have the same size.
(For now, anyway. Obviously, the ConEmu author may decide to change the
behavior.)
When `WINPTY_SHOW_CONSOLE=1` is set, and when ConEmu's "Process 'start'"
flag is set, ConEmu will still adopt the winpty-agent console. It seems
that this case is still broken (on Vista and up), because winpty.dll
notices that its pipe clients have the wrong PID. I'm not sure what to do
about this. `WINPTY_SHOW_CONSOLE=1` is strictly intended for debugging
winpty, though, so there's no useful need for ConEmu to adopt the console.
There's no way for me to opt out. I could disable the security check when
I detect ConEmu, but that's hazardous, and I'm not a fan of the API hooking
anyway.
winpty prints a content-free error, "Error creating winpty." in the above
case. The error report could be improved to include the traced() error,
"Security check failed: pipe client pid (7492) does not match agent pid (7416)".
Second part of a fix for
https://github.com/rprichard/winpty/issues/70
The SW_HIDE change fixes part of a ConEmu<->winpty incompatibility. If
WINPTY_SHOW_CONSOLE=1 is set, and ConEmu's "start cmd" flag is on, then
readFontTable can still run forever.
Avoiding a background desktop would fix a ConEmu<->winpty incompatibility
if not for the previous fix. It fixes a clipboard issue and
thread-unsafety issue, but only on Win7 and up. (More work is needed for
XP/Vista.)
See https://github.com/rprichard/winpty/issues/58.
See https://github.com/rprichard/winpty/issues/70.
When I initially wrote this section, there was no `install` target at all,
and I just ran `build/console.exe` right out of my checkout. That's
actually very inconvenient, though, for day-to-day use.
There are three reasons for this change:
* It's consistent with MSYS2, which already renamed console.exe to
winpty.exe.
* winpty.exe is less likely to clash with another program.
* winpty.exe is easier to search for online. If someone unfamiliar with
winpty is asked to run console.exe, they might have a hard time figuring
out where console.exe comes from.
The old behavior can be restored by passing UNIX_ADAPTER_EXE=console.exe
to `make`, or by simply renaming the binary after-the-fact, or with
`alias`, etc.
* Harmonize the subsystem of every binary to CONSOLE. It apparently
doesn't matter for DLLs, and it's easier if they're all the same. I
examined various DLLs on my system, and they're very inconsistent.
(e.g. kernel32.dll is CONSOLE, but shell32.dll is GUI.)
* When using a recent XP-targeting toolset (v120_xp or v140_xp), it's
necessary to explicitly specify the subsystem, so list it in
configurations.gypi.
* node.js changed its common.gypi to disable exception handling[1], so
explicitly turn it back on in winpty.gyp. Placing the setting in
winpty.gyp's 'target_defaults' block doesn't seem to override a setting
in an included file's 'target_defaults' block, so instead enable EH on
each target.
* For consistency with other gyp files I see, use quoted integers instead
of bare integers for settings. Affects the 'RuntimeLibrary' setting.
* Provide a documented way in configurations.gypi for setting the
XP-specific toolsets.
[1] da9eff80a3
MSVC complained that GetVersionEx was deprecated when the /SDL checks were
enabled.
Comment added to WindowsVersion.cc:
Allow use of deprecated functions (i.e. GetVersionEx). We need to use
GetVersionEx for the old MinGW toolchain and with MSVC when it targets XP.
Having two code paths makes code harder to test, and it's not obvious how
to detect the presence of a new enough SDK. (Including ntverp.h and
examining VER_PRODUCTBUILD apparently works, but even then, MinGW-w64 and
MSVC seem to use different version numbers.)
This change avoids the unnecessary round-trip to the ANSI (or OEM?) code
page, and it also allows removing the mbstowcs calls, which simplifies the
code and fixes MSVC /SDL compliance.
In certain situations (e.g. when it has the SeImpersonatePrivilege
privilege), a named pipe server can impersonate a named pipe client and
execute in its security context (e.g. open the client user's files).
There's no reason for a winpty debugserver to need this ability, so
restrict it in the client.
It seems unlikely to me that this commit fixes a genuine security issue,
but it also seems like a decent precaution to take. Clients using winpty
don't connect to the DebugServer named pipe unless the WINPTY_DEBUG
environment variable is set, and Windows restricts the ability to
impersonate named clients (at least as of XP SP2).
The str array need an extra element for the NUL terminator. The previous
code didn't suffer from memory-unsafety, but winpty_snprintf truncated the
last character of the marker, and trying to write NUL to the WinXP console
didn't work. (I suspect WinXP turned the NUL into a space, so
findSyncMarker couldn't find it later.)
* In general, harmonize the debugserver named pipe with the other named
pipe instances: don't let remote users log messages and fail if the
pipe already exists.
Also:
* Removed some obsoleted comments.
* Factor out the Handle class. Rename to OwnedHandle. Merge some
improvements from libwinpty-rewrite, including fixing move-assignment
so that it closes the existing handle, if one exists.
* Set the PIPE_REJECT_REMOTE_CLIENTS flag on Vista and up.
* Set a DACL on named pipes that gives full control to the built-in
administrators, the local system account, and the current security
token's owner SID. This is the same DACL that is used by default,
except that the default also grants read access to the Everyone group
and the anonymous account.
* The createSecurityDescriptorOwnerFullControlEveryoneWrite function is
not currently used (or tested), but I think I'll use it in the debug
server to allow collecting trace output from other accounts on the
machine. (I think I'll make that behavior optional.)
* PIDs are recycled, so include the system time.
* A program running on the same machine could predict the names of pipes
winpty uses and block winpty, causing a denial-of-service. I'm not sure
this is really a concern, but I suppose in principle it is? Try to
guard against it by appending random bytes to the pipe name. The
CreatePrivateNamespace API looks relevant, but I'm not sure it applies
to named pipes, and it's Vista only.
* Rename modifiableWString to vectorWithNulFromString.
* Create a vectorFromString that omits the NUL terminator.
* Stop triply-terminated non-empty environment blocks.
* Stop adding an extra NUL terminator to environment blocks in
handleStartProcessPacket. (env.data() didn't necessarily add a
terminator in C++03, but it does in C++11.)
* Previously, most places in the codebase used "%ls", which relies on the
CRT to do a wide-to-mbcs conversion, and I think(?) that uses locale
settings?
* Do the conversion explicitly using narrowString, which is renamed to
utf8FromWide and moved to a new file, src/shared/StringUtil.cc.
Remove all references to the iostream and sstream headers from code in the
src directory.
This change reduces the sizes of winpty.dll and winpty-agent.exe. For
example:
Cygwin's x86_64-w64-mingw32-g++ 4.9.2 compiler:
- winpty.dll: 622 KiB to 513 KiB
- winpty-agent.exe: 742 KiB to 572 KiB
MSVC 2015, using gyp + configurations.gypi targeting x64:
- winpty.dll: 985 KiB to 545 KiB
- winpty-agent.exe: 1108 KiB to 642 KiB
* Avoid std::stringstream because it's inefficient/bloated.
* Explicitly handle decode errors, by throwing ReadBuffer::DecodeError.
For the time being, this exception can only be thrown in the agent, but
I anticipate it being thrown in the libwinpty code eventually.
* Catch encoding errors a bit better.
* Be more precise about the integer types used for length. e.g.
Use 64 bits on the wire, but otherwise use size_t/uint64_t where
appropriate.
* Replace memcpy with std::copy to make MSVC's /SDL security checks happy.
It's a somewhat silly/arbitrary change. I don't think it will affect
performance, but I haven't benchmarked it.
* This commit changes the RPC protocol that libwinpty uses to communicate
with winpty-agent.
The existing snprintf functions were not C99-conformant:
- The _vsnprintf* functions do not accept all of the C99 specifiers,
like %zu for size_t or (on WinXP) %lld for long long.
- MinGW always provides vsnprintf, but the implementation sometimes just
calls MSVCRT's _vsnprintf*. In that case, it won't handle all C99
specifiers and the return value won't be conformant.
Changes:
- winpty_snprintf accepts C90(?) specifiers, returns -1 on error
(including truncation) and should work with the inttypes.h printf
macros.
- Enable __attribute__((printf)) on the winpty_[v]snprintf functions and
also on trace(). Fix a HANDLE(void*) <-> unsigned int mismatch in
NamedPipe.cc.
- Switch most sprintf() call sites to winpty_snprintf for improved safety
guarantees. (This change gets the code closer to compiling with MSVC's
/sdl flag, which forbids "unsafe" memory/string functions.)
* In agent: if the console is frozen, then calling abort hangs forever,
because abort tries to write to stderr, which blocks. Instead,
use WM_CLOSE to close the console window. If that doesn't work after
waiting a while, then exit.
We need to do the same thing doing normal agent shutdown, so factor out
an agentShutdown function. It's a bit unhappy living in
WinptyAssert.cc, but it works.
* Outside agent: trace an assertion failure message, then delegate to the
ordinary assert() function. If that's somehow turned off, then call
abort().
The gyp file already worked this way, but now the Makefile does too.
The motivation is to let the shared source files have different error
handling behavior (e.g. assert/ASSERT/stderr-print/abort/exception) in
winpty.dll and the other targets.
Currently, when a header file is removed (or renamed), there is a stale
dependency file still referring to the header, and GNU make aborts, because
the header is missing. Attempt to work around this by adding a pattern
rule to generate any header in `src/`. The rule prints a warning message.
The C source file whose header was missing will be recompiled because a
dependency was rebuilt. Once it is, the removed header should be removed
from the dependency file.
(Aside: FWIW, the "ninja" build tool knows directly about depfiles, and it
does not have this problem.)
