diff --git a/.github/workflows/verify-windows-shim.yml b/.github/workflows/verify-windows-shim.yml index 47600c1..288e2dd 100644 --- a/.github/workflows/verify-windows-shim.yml +++ b/.github/workflows/verify-windows-shim.yml @@ -1,15 +1,14 @@ name: verify windows shim # Rebuilds the Windows shim from dotslash_windows_shim.rs and fails if the -# checked-in .exe does not match, so the source cannot change without the -# regenerated binary. The build is byte-for-byte reproducible (rust-lld with +# checked-in artifacts do not match, so the source cannot change without the +# regenerated files. The build is byte-for-byte reproducible (rust-lld with # /Brepro, plus the pinned toolchain in windows_shim/rust-toolchain.toml), so a # plain `git diff` is a reliable check. Each architecture is built on its own # native runner to avoid cross-linking. # -# The freshly built binary is uploaded as an artifact before the diff check, so -# when the check fails (e.g. the committed binaries are out of date) you can -# download the correct binary from the run's "Artifacts" section and commit it +# The freshly built shim and linker stub are uploaded before the diff check, so +# when the check fails you can download the correct artifacts and commit them # without needing a local Windows machine. on: @@ -55,17 +54,19 @@ jobs: rustup target add "${{ matrix.target }}" - name: Rebuild the shim run: python release.py "${{ matrix.target }}" - - name: Upload the rebuilt shim - # Runs before the diff check so the binary is downloadable even when the - # committed copy is out of date and the job ultimately fails. + - name: Upload the rebuilt artifacts + # This runs before the diff check so corrected files remain downloadable + # when the committed copies are out of date and the job ultimately fails. uses: actions/upload-artifact@v4 with: name: dotslash_windows_shim-${{ matrix.arch }} - path: windows_shim/dotslash_windows_shim-${{ matrix.arch }}.exe + path: | + windows_shim/dotslash_windows_linker_stub.exe + windows_shim/dotslash_windows_shim-${{ matrix.arch }}.exe if-no-files-found: error - - name: Verify the checked-in binary is up to date + - name: Verify the checked-in artifacts are up to date run: | - if ! git diff --exit-code -- 'dotslash_windows_shim-*.exe'; then - echo "::error::Checked-in shim binary is out of date. Download the 'dotslash_windows_shim-${{ matrix.arch }}' artifact from this run and commit it (or run 'py release.py' locally)." + if ! git diff --exit-code -- 'dotslash_windows_linker_stub.exe' 'dotslash_windows_shim-*.exe'; then + echo "::error::Checked-in Windows shim artifacts are out of date. Download 'dotslash_windows_shim-${{ matrix.arch }}' from this run and commit its files, or run 'py release.py' locally." exit 1 fi diff --git a/windows_shim/Cargo.toml b/windows_shim/Cargo.toml index 5b5f92a..ae914ba 100644 --- a/windows_shim/Cargo.toml +++ b/windows_shim/Cargo.toml @@ -27,6 +27,7 @@ opt-level = "z" lto = true codegen-units = 1 panic = "abort" +strip = "symbols" [profile.dev] opt-level = 1 diff --git a/windows_shim/README.md b/windows_shim/README.md index 8f9bd13..8843989 100644 --- a/windows_shim/README.md +++ b/windows_shim/README.md @@ -31,9 +31,7 @@ few kilobytes. ## Release -The checked-in `dotslash_windows_shim-x86_64.exe` and -`dotslash_windows_shim-aarch64.exe` are built from `dotslash_windows_shim.rs`. -Regenerate them on Windows with: +The checked-in `dotslash_windows_shim-x86_64.exe` and `dotslash_windows_shim-aarch64.exe` are built from `dotslash_windows_shim.rs`, and `dotslash_windows_linker_stub.exe` is generated as their linker input. Regenerate all three on Windows with: ```shell py release.py @@ -56,13 +54,7 @@ rebuilds the shim whenever anything under `windows_shim/` changes and fails if the committed binaries are stale, so regenerate and commit them in the same change as any edit to the source or a bump of the pinned toolchain. -If you do not have a Windows machine, let CI build the binaries for you: push -your change (or trigger the workflow manually), then download the -`dotslash_windows_shim-x86_64` and `dotslash_windows_shim-aarch64` artifacts -from the workflow run — each contains the freshly built `.exe`. Because the -build is reproducible, those artifacts are exactly what a local `py release.py` -would produce; commit them into `windows_shim/` and re-run the workflow to -confirm it passes. +If you do not have a Windows machine, let CI build the artifacts for you: push your change (or trigger the workflow manually), then download `dotslash_windows_shim-x86_64` and `dotslash_windows_shim-aarch64` from the workflow run. Each contains the freshly built architecture-specific shim and the shared linker stub. Because the build is reproducible, those files are exactly what a local `py release.py` would produce; commit them into `windows_shim/` and re-run the workflow to confirm it passes. ## Testing diff --git a/windows_shim/dotslash_windows_linker_stub.exe b/windows_shim/dotslash_windows_linker_stub.exe new file mode 100644 index 0000000..5b1bb15 Binary files /dev/null and b/windows_shim/dotslash_windows_linker_stub.exe differ diff --git a/windows_shim/dotslash_windows_shim-aarch64.exe b/windows_shim/dotslash_windows_shim-aarch64.exe index 09d2946..5dbba60 100644 Binary files a/windows_shim/dotslash_windows_shim-aarch64.exe and b/windows_shim/dotslash_windows_shim-aarch64.exe differ diff --git a/windows_shim/dotslash_windows_shim-x86_64.exe b/windows_shim/dotslash_windows_shim-x86_64.exe index 5a989e4..23a6cf5 100644 Binary files a/windows_shim/dotslash_windows_shim-x86_64.exe and b/windows_shim/dotslash_windows_shim-x86_64.exe differ diff --git a/windows_shim/dotslash_windows_shim.rs b/windows_shim/dotslash_windows_shim.rs index 78ac060..60ed35c 100644 --- a/windows_shim/dotslash_windows_shim.rs +++ b/windows_shim/dotslash_windows_shim.rs @@ -22,7 +22,8 @@ #![cfg_attr(feature = "no_std", feature(lang_items))] #![cfg_attr(feature = "no_std", no_std)] #![cfg_attr(feature = "no_std", no_main)] -#![cfg_attr(feature = "no_std", windows_subsystem = "console")] // Set Entrypoint to "mainCRTStartup" +// Select the console subsystem; the no_std entry point is `mainCRTStartup` below. +#![cfg_attr(feature = "no_std", windows_subsystem = "console")] #[allow(clippy::upper_case_acronyms)] type DWORD = u32; @@ -31,25 +32,17 @@ use core::mem; use core::ptr; use core::str; -use windows_sys::Win32::Foundation::CloseHandle; use windows_sys::Win32::Foundation::ERROR_FILE_NOT_FOUND; use windows_sys::Win32::Foundation::GetLastError; use windows_sys::Win32::Foundation::HANDLE; -use windows_sys::Win32::Foundation::HMODULE; -use windows_sys::Win32::Foundation::S_FALSE; -use windows_sys::Win32::Foundation::S_OK; use windows_sys::Win32::Foundation::TRUE; use windows_sys::Win32::Foundation::WAIT_OBJECT_0; -use windows_sys::Win32::Globalization::lstrcatW; -use windows_sys::Win32::Globalization::lstrlenW; use windows_sys::Win32::Storage::FileSystem::WriteFile; use windows_sys::Win32::System::Console::GetStdHandle; use windows_sys::Win32::System::Console::STD_ERROR_HANDLE; use windows_sys::Win32::System::Environment::GetCommandLineW; use windows_sys::Win32::System::LibraryLoader::GetModuleFileNameW; -use windows_sys::Win32::System::LibraryLoader::GetModuleHandleW; use windows_sys::Win32::System::Memory::GetProcessHeap; -use windows_sys::Win32::System::Memory::HEAP_ZERO_MEMORY; use windows_sys::Win32::System::Memory::HeapAlloc; use windows_sys::Win32::System::Memory::HeapFree; use windows_sys::Win32::System::Threading::CreateProcessW; @@ -59,12 +52,8 @@ use windows_sys::Win32::System::Threading::INFINITE; use windows_sys::Win32::System::Threading::PROCESS_INFORMATION; use windows_sys::Win32::System::Threading::STARTUPINFOW; use windows_sys::Win32::System::Threading::WaitForSingleObject; -use windows_sys::Win32::UI::Shell::PathCchRemoveExtension; -use windows_sys::Win32::UI::Shell::PathGetArgsW; -use windows_sys::Win32::UI::Shell::PathQuoteSpacesW; use windows_sys::core::BOOL; use windows_sys::core::PCWSTR; -use windows_sys::core::PWSTR; use windows_sys::w; fn write_stderr(text: &str) -> BOOL { @@ -72,9 +61,9 @@ fn write_stderr(text: &str) -> BOOL { // parameter is not NULL. let mut bytes_written: u32 = 0; unsafe { - let stdout: HANDLE = GetStdHandle(STD_ERROR_HANDLE); + let stderr: HANDLE = GetStdHandle(STD_ERROR_HANDLE); let ok: BOOL = WriteFile( - stdout, /* hFile */ + stderr, /* hFile */ text.as_ptr(), /* lpBuffer */ text.len() as u32, /* nNumberOfBytesToWrite */ &mut bytes_written as *mut u32, /* lpNumberOfBytesWritten */ @@ -85,14 +74,48 @@ fn write_stderr(text: &str) -> BOOL { } fn fatal(text: &str) -> ! { + // Diagnostics are best-effort because there is no useful recovery if + // stderr itself cannot be written. write_stderr("dotslash-windows-shim: "); write_stderr(text); write_stderr("\n"); unsafe { ExitProcess(1) } } -// CreateProcessW's lpCommandLine has a maximum length of 32,767 -// characters. +// Find the raw argument tail without parsing and reconstructing it, so the +// caller's quoting and backslashes reach dotslash unchanged. Only argv[0] is +// scanned: quotes may delimit spans anywhere within it, while spaces and tabs +// terminate it only when outside quotes. +// +// SAFETY: `p` must point to a readable, null-terminated UTF-16 string. +unsafe fn command_line_args(mut p: *const u16) -> *const u16 { + let mut in_quotes = false; + + loop { + let ch = unsafe { *p }; + + if ch == 0 { + return p; + } + + if ch == b'"' as u16 { + in_quotes = !in_quotes; + } else if !in_quotes && (ch == b' ' as u16 || ch == b'\t' as u16) { + break; + } + + p = unsafe { p.add(1) }; + } + + while unsafe { *p } == b' ' as u16 || unsafe { *p } == b'\t' as u16 { + p = unsafe { p.add(1) }; + } + + p +} + +// CreateProcessW's lpCommandLine has a maximum length of 32,767 UTF-16 code +// units, including its terminating null. const BUF_MAX_SIZE: usize = 32767; struct PoorMansString { @@ -101,51 +124,41 @@ struct PoorMansString { } impl PoorMansString { - fn append(&mut self, other: *const u16) { - let other_len = unsafe { lstrlenW(other) as usize }; - if self.len + other_len > BUF_MAX_SIZE { - fatal("Buffer overflow"); - } - unsafe { - // Concatenate other string to self.buf - if lstrcatW(self.buf, other).is_null() { - fatal("string concatenation failed"); + fn append(&mut self, mut other: *const u16) { + while unsafe { *other } != 0 { + if self.len == self.capacity() { + fatal("Buffer overflow"); + } + unsafe { + *self.buf.add(self.len) = *other; + other = other.add(1); } + self.len += 1; } - - self.len = self.len + other_len; + unsafe { *self.buf.add(self.len) = 0 }; } fn capacity(&self) -> usize { + // append() writes a null after every copy, so one code unit is never + // available for content. BUF_MAX_SIZE - 1 } fn new() -> Self { - // Allocate BUF_MAX_SIZE upfront to avoid growing buffers. + // Allocate once at the API's maximum size. Zero-initialization is + // unnecessary because append() writes both content and its terminator. let buf = unsafe { - HeapAlloc( - GetProcessHeap(), - HEAP_ZERO_MEMORY, - BUF_MAX_SIZE * mem::size_of::(), - ) as *mut u16 + HeapAlloc(GetProcessHeap(), 0, BUF_MAX_SIZE * mem::size_of::()) as *mut u16 }; Self { buf, len: 0 } } } -impl Drop for PoorMansString { - fn drop(&mut self) { - unsafe { - HeapFree(GetProcessHeap(), 0, self.buf as *mut _); - } - } -} - fn main_impl() -> ! { let mut ds_cmd = PoorMansString::new(); - // Append "dotslash " to the command string. - ds_cmd.append(w!("dotslash ")); + // Append `dotslash "` to the command string. + ds_cmd.append(w!("dotslash \"")); // Append the DotSlash file path to the command string. // @@ -153,24 +166,18 @@ fn main_impl() -> ! { // command string will happen in-place in the command string. unsafe { let ds_file_ptr = ds_cmd.buf.add(ds_cmd.len); - // Get a handle to this executable. - // - // When passed NULL, GetModuleHandle returns a handle to the file - // used to create the calling process (.exe file). - let handle: HMODULE = GetModuleHandleW(ptr::null_mut()); - // Append the fully qualified path for this executable to the // command string. // // For an executable named `foo.exe` we should now have a command - // string that looks like `dotslash C:\path\to\foo.exe`. + // string that looks like `dotslash "C:\path\to\foo.exe`. // - // GetModuleFileName requires you to keep growing a buffer until it - // fits the path. No need to do this because the command buffer - // is already as large as can be. + // GetModuleFileNameW reports truncation when its buffer is too small. + // Retrying with a larger allocation cannot help because the completed + // CreateProcessW command must fit in this same maximum-sized buffer. let remaining_capacity = ds_cmd.capacity() - ds_cmd.len; let new_len = GetModuleFileNameW( - handle, /* hModule */ + ptr::null_mut(), /* hModule */ ds_file_ptr, /* lpFilename */ remaining_capacity as _, /* nSize */ ) as usize; @@ -179,42 +186,29 @@ fn main_impl() -> ! { } ds_cmd.len += new_len; - // Remove the extension from this executable's full path. - // - // We assume that the DotSlash file is named just like this executable but - // without the `.exe`. + // Remove the final `.exe` extension from this executable's full path. // - // For an executable named `foo.exe` we should now have a command string that - // looks like `dotslash C:\path\to\foo`. + // The shim contract requires it to be named `.exe`, so + // the suffix is unconditionally removed as four UTF-16 code units. No + // case check is needed, which also handles the conventional `.EXE`. // - // PathCchRemoveExtension returns `S_OK` when an extension was found and - // removed. It returns `S_FALSE` when there is no extension. In this case, we'll - // pass this executable as the DotSlash file path. `dotslash` will fail and - // complain that it's not a valid DotSlash file. + // The command now looks like `dotslash "C:\path\to\foo`. // - let found_extension = PathCchRemoveExtension(ds_file_ptr, new_len + 1); - if found_extension != S_OK && found_extension != S_FALSE { - fatal("PathCchRemoveExtension failed."); - } + ds_cmd.len -= 4; - // Quote the entire DotSlash file path if there are spaces. - // - // No need to worry about escaping quotes because those aren't - // allowed in Windows paths. - // - // For an executable named `foo.exe` this is a noop. - // - // For an executable named `foo bar.exe` we should now have a command - // string that looks like `dotslash "C:\path\to\foo bar"`. - PathQuoteSpacesW(ds_file_ptr); + // Always close the quote around the DotSlash file path. Quoting paths + // without spaces is valid, and no escaping is needed because quotes + // are not allowed in Windows paths. + ds_cmd.append(w!("\"")); // Get the arguments that were passed to us. let line_ptr: PCWSTR = GetCommandLineW(); // Skip `argv[0]` and focus on the remaining arguments. - let args_ptr: PWSTR = PathGetArgsW(line_ptr); + let args_ptr = command_line_args(line_ptr); // Append the arguments to the command string if there are any. if *args_ptr != 0 { - // Append a separator for the arguments. + // Normalize the discarded argv[0] separator whitespace to one + // space. The argument tail itself remains untouched. ds_cmd.append(w!(" ")); ds_cmd.append(args_ptr); } @@ -226,6 +220,10 @@ fn main_impl() -> ! { si.cb = mem::size_of::() as DWORD; let mut pi: PROCESS_INFORMATION = unsafe { mem::zeroed() }; + // A null application name makes CreateProcessW resolve the first command + // token (`dotslash`) normally, including through PATH. Handles, the + // environment, and the working directory are inherited so the shim is + // transparent to the child process. let status = unsafe { CreateProcessW( ptr::null_mut(), // lpApplicationName @@ -241,10 +239,12 @@ fn main_impl() -> ! { ) }; - // Once CreateProcessW is called, there is no need to hold onto - // lpCommandLine. - // https://stackoverflow.com/a/31031165 - drop(ds_cmd); + // CreateProcessW has finished reading the command line when it returns, so + // release the maximum-sized buffer before a potentially long child wait. + // Capture a failure code first because cleanup may change the thread's + // last-error value. + let err = unsafe { GetLastError() }; + unsafe { HeapFree(GetProcessHeap(), 0, ds_cmd.buf.cast()) }; if status == TRUE { let res = unsafe { WaitForSingleObject(pi.hProcess, INFINITE) }; @@ -258,14 +258,11 @@ fn main_impl() -> ! { fatal("could not get dotslash command exit code."); } - unsafe { - CloseHandle(pi.hProcess); - CloseHandle(pi.hThread); - ExitProcess(status) - }; + // Process teardown closes both handles in pi. Closing the thread handle + // earlier would add code and an import without materially reducing RSS. + unsafe { ExitProcess(status) }; } - let err = unsafe { GetLastError() }; if err == ERROR_FILE_NOT_FOUND { fatal("dotslash executable not found."); } diff --git a/windows_shim/release.py b/windows_shim/release.py index dc5973c..82218b0 100644 --- a/windows_shim/release.py +++ b/windows_shim/release.py @@ -10,6 +10,7 @@ import os import shutil +import struct import subprocess import sys from pathlib import Path @@ -19,6 +20,36 @@ target_triplets: list[str] = ["x86_64-pc-windows-msvc", "aarch64-pc-windows-msvc"] +def write_linker_stub(path: Path) -> None: + # A PE image starts with an MZ-compatible prefix whose field at offset + # 0x3c points Windows to the PE signature. lld-link's default prefix makes + # the headers spill into a second 512-byte file-alignment block. Supplying + # this minimal valid prefix via /STUB keeps SizeOfHeaders, and therefore the + # complete shim, 512 bytes smaller without changing how Windows starts it. + # + # References: + # - Microsoft documents the PE/COFF format at + # https://learn.microsoft.com/en-us/windows/win32/debug/pe-format. + # - Microsoft documents the /STUB linker option at + # https://learn.microsoft.com/en-us/cpp/build/reference/stub-ms-dos-stub-file-name + # + # The header describes one 69-byte image with a 64-byte header and no + # relocations. Its five-byte payload makes the input a complete executable, + # as required by /STUB; Windows does not execute it when loading the PE. + stub = bytearray(64) + # These fields describe the complete one-page image and its 64-byte header. + struct.pack_into(" None: if not IS_WINDOWS: raise Exception("Only Windows is supported.") @@ -55,17 +86,23 @@ def main(targets: list[str] | None = None) -> None: if not rust_lld.is_file(): raise FileNotFoundError(f"Rust's bundled linker was not found: {rust_lld}") + # Regenerate the checked-in linker input before any selected release binary. + linker_stub = dotslash_windows_shim_root / "dotslash_windows_linker_stub.exe" + write_linker_stub(linker_stub) + rustflags = [ f"-Clinker={rust_lld}", "-Clinker-flavor=lld-link", "-Clink-arg=/DEBUG:NONE", # Avoid an embedded PDB path. "-Clink-arg=/NODEFAULTLIB:msvcrt", # The shim does not use the CRT. "-Clink-arg=/Brepro", # Hash-based timestamps instead of wall-clock time. + "-Clink-arg=/MERGE:.pdata=.rdata", # Both sections are read-only. + f"-Clink-arg=/STUB:{linker_stub}", ] # Ambient RUSTFLAGS could change the measured release layout and break - # reproducibility. Encoded flags also preserve the linker path as a single - # argument when the workspace path contains spaces. + # reproducibility. Encoded flags also preserve the linker and stub paths as + # single arguments when the workspace path contains spaces. build_env = {**os.environ} build_env.pop("RUSTFLAGS", None) build_env["RUSTC_BOOTSTRAP"] = "1" # Required by no_std language items. diff --git a/windows_shim/tests/test.py b/windows_shim/tests/test.py index ce1d671..6193369 100755 --- a/windows_shim/tests/test.py +++ b/windows_shim/tests/test.py @@ -22,6 +22,20 @@ from typing import Final EMPTY_STR_LIST: Final[list[str]] = [] +WINDOWS_SHIM_ROOT: Final[Path] = Path(__file__).resolve().parent.parent + +# CreateProcessW limits lpCommandLine to 32,767 UTF-16 code units, including +# the terminating null character. +# https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw#parameters +MAX_COMMAND_LINE_LENGTH: Final[int] = 32767 + +# Keep size changes visible in review: both increases and improvements should +# update these expectations in the same PR as the regenerated artifacts. +RELEASE_ARTIFACT_SIZES: Final[dict[str, int]] = { + "dotslash_windows_linker_stub.exe": 69, + "dotslash_windows_shim-aarch64.exe": 2560, + "dotslash_windows_shim-x86_64.exe": 2560, +} try: from .fb.ci import set_ci_envs @@ -53,6 +67,20 @@ def test_require_dotslash_windows_shim_env(self) -> None: self.assertTrue(os.path.exists(os.environ["DOTSLASH_WINDOWS_SHIM"])) +class ReleaseArtifactSizeTest(unittest.TestCase): + def test_release_artifact_sizes(self) -> None: + for filename, expected_bytes in RELEASE_ARTIFACT_SIZES.items(): + with self.subTest(filename=filename): + artifact = WINDOWS_SHIM_ROOT / filename + actual_bytes = artifact.stat().st_size + self.assertEqual( + actual_bytes, + expected_bytes, + f"{filename} changed from {expected_bytes} bytes " + f"to {actual_bytes} bytes", + ) + + def generate_dotslash_file(name: str) -> str: spec = { "name": name, @@ -190,29 +218,6 @@ def test_args_none_with_period_in_name(self) -> None: self.assertRegex(ret.stdout, PRINT_ARGS_ARG0) self.assertEqual(ret.returncode, 0) - def test_args_none_with_no_extension(self) -> None: - with move_cwd(self._fixtures): - shutil.move("print_args.exe", "print_args") - - # This executes because CreateProcessW adds an implicit `.exe`. - # PathCchRemoveExtension won't have an extension to remove, - # so we'll pass the exetuable to `dotslash`, which will then - # fail because it's not an actual DotSlash file. - print_args_path = self._fixtures / "print_args" - ret = subprocess.run( - [print_args_path], - capture_output=True, - encoding="utf8", - ) - self.assertEqual( - ret.stderr, - f"dotslash error: problem with `{print_args_path}`\n" - "caused by: failed to read DotSlash file\n" - "caused by: stream did not contain valid UTF-8\n", - ) - self.assertEqual(ret.stdout, "") - self.assertEqual(ret.returncode, 1) - def test_args_none_with_unc_path(self) -> None: ret = subprocess.run( ["\\\\?\\" + str(self._fixtures / "print_args.exe")], @@ -275,6 +280,52 @@ def test_args_simple(self) -> None: self.assertRegex(ret.stdout, PRINT_ARGS_ARG0) self.assertEqual(ret.returncode, 0) + def test_raw_command_lines(self) -> None: + actual_shim = self._fixtures / "print_args.exe" + + # A copy of the shim whose own path contains a space, to check that a + # quoted argv[0] is still skipped correctly. + spaced_manifest = self._fixtures / "print args" + spaced_shim = self._fixtures / "print args.exe" + shutil.copy(self._fixtures / "print_args", spaced_manifest) + shutil.copy(actual_shim, spaced_shim) + + cases = [ + (actual_shim, "print_args.exe", ""), + (actual_shim, "print_args.exe one two", "1:one\n2:two\n"), + (actual_shim, 'print_args.exe "one two"', "1:one two\n"), + (actual_shim, 'print_args.exe "" "a\\\"b" trailing\\', '1:\n2:a"b\n3:trailing\\\n'), + (actual_shim, "print_args.exe\tone\ttwo", "1:one\n2:two\n"), + (actual_shim, 'print" args".exe one', "1:one\n"), + (actual_shim, '"print_args.exe"suffix one', "1:one\n"), + (actual_shim, '"print_args.exe"one', ""), + (spaced_shim, f'"{spaced_shim}" one', "1:one\n"), + (spaced_shim, f'"{spaced_shim}" "one two"', "1:one two\n"), + ] + for shim, raw_command_line, stderr in cases: + with self.subTest(raw_command_line=raw_command_line): + ret = subprocess.run( + raw_command_line, + executable=str(shim), + capture_output=True, + encoding="utf8", + ) + self.assertEqual(ret.stderr, stderr) + self.assertRegex(ret.stdout, PRINT_ARGS_ARG0) + self.assertEqual(ret.returncode, 0) + + def test_args_none_with_uppercase_exe_extension(self) -> None: + uppercase_shim = self._fixtures / "print_args.EXE" + shutil.move(self._fixtures / "print_args.exe", uppercase_shim) + ret = subprocess.run( + [str(uppercase_shim)], + capture_output=True, + encoding="utf8", + ) + self.assertEqual(ret.stderr, "") + self.assertRegex(ret.stdout, PRINT_ARGS_ARG0) + self.assertEqual(ret.returncode, 0) + def test_args_simple_with_unicode_in_name(self) -> None: shutil.move( self._fixtures / "print_args", @@ -367,15 +418,12 @@ def test_args_unicode(self) -> None: self.assertEqual(ret.returncode, 0) def test_args_long_args(self) -> None: - # Windows CreateProcess API has a length limit of 32,768. # The shim will actually use a bit more than the original call: # Original: foo.exe a b c # Shim: dotslash foo a b c # Actually more than the above because "foo" is resolved to an # absolute path. So here we test staying a bit below this limit. - # https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessa#parameters - MAX_COMMAND_LINE = 32768 - long_arg = "x" * (MAX_COMMAND_LINE - 512) + long_arg = "x" * (MAX_COMMAND_LINE_LENGTH - 512) ret = subprocess.run( [str(self._fixtures / "print_args.exe"), long_arg], capture_output=True, @@ -385,6 +433,26 @@ def test_args_long_args(self) -> None: self.assertRegex(ret.stdout, PRINT_ARGS_ARG0) self.assertEqual(ret.returncode, 0) + def test_args_over_command_buffer_boundary(self) -> None: + actual_shim = self._fixtures / "print_args.exe" + manifest = self._fixtures / "print_args" + capacity = MAX_COMMAND_LINE_LENGTH - 1 + fixed_command_length = len('dotslash "') + len(str(manifest)) + len('" ') + overflowing_tail = "x" * (capacity - fixed_command_length + 1) + + ret = subprocess.run( + f"print_args.exe {overflowing_tail}", + executable=str(actual_shim), + capture_output=True, + encoding="utf8", + ) + self.assertEqual( + ret.stderr, + "dotslash-windows-shim: Buffer overflow\n", + ) + self.assertEqual(ret.stdout, "") + self.assertEqual(ret.returncode, 1) + def test_stdin_to_stdout(self) -> None: ret = subprocess.run( [str(self._fixtures / "stdin_to_stdout.exe")], @@ -431,6 +499,23 @@ def test_missing_dotslash(self) -> None: self.assertEqual(ret.stdout, "") self.assertEqual(ret.returncode, 1) + def test_invalid_dotslash_executable(self) -> None: + invalid_bin = self._fixtures / "invalid_bin" + invalid_bin.mkdir() + (invalid_bin / "dotslash.exe").write_bytes(b"not a Windows executable") + with prepend_path(invalid_bin): + ret = subprocess.run( + [str(self._fixtures / "exit_code.exe"), "0"], + capture_output=True, + encoding="utf8", + ) + self.assertEqual( + ret.stderr, + "dotslash-windows-shim: could not execute dotslash command.\n", + ) + self.assertEqual(ret.stdout, "") + self.assertEqual(ret.returncode, 1) + if __name__ == "__main__": unittest.main(verbosity=2)