diff --git a/localization/strings/en-US/Resources.resw b/localization/strings/en-US/Resources.resw index 3607f1f0c..ceea86e2b 100644 --- a/localization/strings/en-US/Resources.resw +++ b/localization/strings/en-US/Resources.resw @@ -2554,4 +2554,7 @@ On first run, creates the file with all settings commented out at their defaults Requested load but no input provided. + + Cannot write image to terminal. Use the -o flag or redirect stdout. + diff --git a/src/windows/wslc/commands/ImageSaveCommand.cpp b/src/windows/wslc/commands/ImageSaveCommand.cpp index f4056c894..5159bab6d 100644 --- a/src/windows/wslc/commands/ImageSaveCommand.cpp +++ b/src/windows/wslc/commands/ImageSaveCommand.cpp @@ -28,7 +28,7 @@ std::vector ImageSaveCommand::GetArguments() const { return { Argument::Create(ArgType::ImageId, true), - Argument::Create(ArgType::Output, true), + Argument::Create(ArgType::Output), Argument::Create(ArgType::Session), }; } diff --git a/src/windows/wslc/services/ImageService.cpp b/src/windows/wslc/services/ImageService.cpp index 34d4aadd6..0baf96dd8 100644 --- a/src/windows/wslc/services/ImageService.cpp +++ b/src/windows/wslc/services/ImageService.cpp @@ -233,9 +233,14 @@ void ImageService::Save(wsl::windows::wslc::models::Session& session, const std: CreateFileW(output.c_str(), GENERIC_WRITE, FILE_SHARE_READ, nullptr, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr)}; THROW_LAST_ERROR_IF(!outputFile); + Save(session, image, outputFile.get(), cancelEvent); +} + +void ImageService::Save(wsl::windows::wslc::models::Session& session, const std::string& image, HANDLE outputHandle, HANDLE cancelEvent) +{ wsl::windows::common::HandleConsoleProgressBar progressBar( - outputFile.get(), L"Save in progress.", wsl::windows::common::HandleConsoleProgressBar::Format::FileSize); - THROW_IF_FAILED(session.Get()->SaveImage(ToCOMInputHandle(outputFile.get()), image.c_str(), nullptr, cancelEvent)); + outputHandle, L"Save in progress.", wsl::windows::common::HandleConsoleProgressBar::Format::FileSize); + THROW_IF_FAILED(session.Get()->SaveImage(ToCOMInputHandle(outputHandle), image.c_str(), nullptr, cancelEvent)); } void ImageService::Prune() diff --git a/src/windows/wslc/services/ImageService.h b/src/windows/wslc/services/ImageService.h index 4d97b55e2..1d1e9e0e8 100644 --- a/src/windows/wslc/services/ImageService.h +++ b/src/windows/wslc/services/ImageService.h @@ -37,6 +37,7 @@ class ImageService static wsl::windows::common::wslc_schema::InspectImage Inspect(wsl::windows::wslc::models::Session& session, const std::string& image); static void Pull(wsl::windows::wslc::models::Session& session, const std::string& image, IProgressCallback* callback); static void Save(wsl::windows::wslc::models::Session& session, const std::string& image, const std::wstring& output, HANDLE cancelEvent = nullptr); + static void Save(wsl::windows::wslc::models::Session& session, const std::string& image, HANDLE outputHandle, HANDLE cancelEvent = nullptr); static void Tag(wsl::windows::wslc::models::Session& session, const std::string& sourceImage, const std::string& targetImage); void Push(); void Prune(); diff --git a/src/windows/wslc/tasks/ImageTasks.cpp b/src/windows/wslc/tasks/ImageTasks.cpp index 461e00589..33d662b00 100644 --- a/src/windows/wslc/tasks/ImageTasks.cpp +++ b/src/windows/wslc/tasks/ImageTasks.cpp @@ -23,6 +23,7 @@ Module Name: #include "TableOutput.h" #include "Task.h" #include +#include using namespace wsl::shared; using namespace wsl::windows::common::string; @@ -190,11 +191,24 @@ void SaveImage(CLIExecutionContext& context) { WI_ASSERT(context.Data.Contains(Data::Session)); WI_ASSERT(context.Args.Contains(ArgType::ImageId)); - WI_ASSERT(context.Args.Contains(ArgType::Output)); auto& session = context.Data.Get(); auto& imageId = context.Args.Get(); - auto& output = context.Args.Get(); - services::ImageService::Save(session, WideToMultiByte(imageId), output, context.CreateCancelEvent()); + + if (context.Args.Contains(ArgType::Output)) + { + auto& output = context.Args.Get(); + services::ImageService::Save(session, WideToMultiByte(imageId), output, context.CreateCancelEvent()); + } + else + { + auto stdoutHandle = GetStdHandle(STD_OUTPUT_HANDLE); + if (wsl::windows::common::wslutil::IsConsoleHandle(stdoutHandle)) + { + THROW_HR_WITH_USER_ERROR(E_INVALIDARG, Localization::WSLCCLI_ImageSaveStdoutIsTerminalError()); + } + + services::ImageService::Save(session, WideToMultiByte(imageId), stdoutHandle, context.CreateCancelEvent()); + } } void TagImage(CLIExecutionContext& context) diff --git a/test/windows/wslc/e2e/WSLCE2EImageSaveTests.cpp b/test/windows/wslc/e2e/WSLCE2EImageSaveTests.cpp index 99d6d1c9c..361f493a6 100644 --- a/test/windows/wslc/e2e/WSLCE2EImageSaveTests.cpp +++ b/test/windows/wslc/e2e/WSLCE2EImageSaveTests.cpp @@ -54,12 +54,6 @@ class WSLCE2EImageSaveTests result.Verify({.Stdout = GetHelpMessage(), .Stderr = L"Required argument not provided: 'image'\r\n", .ExitCode = 1}); } - WSLC_TEST_METHOD(WSLCE2E_Image_Save_MissingOutputPath) - { - const auto result = RunWslc(std::format(L"image save {}", DebianImage.NameAndTag())); - result.Verify({.Stderr = L"Required argument not provided: 'output'\r\n", .ExitCode = 1}); - } - WSLC_TEST_METHOD(WSLCE2E_Image_Save_ImageNotFound) { const auto result = RunWslc(std::format(L"image save --output \"{}\" {}", SavedArchivePath.wstring(), InvalidImage.NameAndTag())); @@ -95,6 +89,42 @@ class WSLCE2EImageSaveTests runResult.Verify({.Stdout = L"Hello from saved image!\n", .Stderr = L"", .ExitCode = 0}); } + WSLC_TEST_METHOD(WSLCE2E_Image_Save_ToStdout_Success) + { + const auto result = RunWslcAndRedirectToFile(std::format(L"image save {}", DebianImage.NameAndTag()), SavedArchivePath); + result.Verify({.Stdout = L"", .Stderr = L"", .ExitCode = 0}); + + VERIFY_IS_TRUE(std::filesystem::exists(SavedArchivePath)); + auto sourceFileSize = std::filesystem::file_size(DebianImage.Path); + auto archiveFileSize = std::filesystem::file_size(SavedArchivePath); + VERIFY_ARE_EQUAL(sourceFileSize, archiveFileSize); + } + + WSLC_TEST_METHOD(WSLCE2E_Image_Save_ToTerminal_Fail) + { + const auto result = RunWslcAndRedirectToFile(std::format(L"image save {}", DebianImage.NameAndTag())); + result.Verify( + {.Stderr = L"Cannot write image to terminal. Use the -o flag or redirect stdout.\r\nError code: E_INVALIDARG\r\n", .ExitCode = 1}); + } + + WSLC_TEST_METHOD(WSLCE2E_Image_Save_ToStdout_Load) + { + // Save source image + auto saveResult = RunWslcAndRedirectToFile(std::format(L"image save {}", DebianImage.NameAndTag()), SavedArchivePath); + saveResult.Verify({.Stdout = L"", .Stderr = L"", .ExitCode = 0}); + + // Delete source image + EnsureImageIsDeleted(DebianImage); + + // Load from saved archive + auto loadResult = RunWslc(std::format(L"image load --input \"{}\"", SavedArchivePath.wstring())); + loadResult.Verify({.Stderr = L"", .ExitCode = 0}); + + // Run a container from the loaded image to verify it works + auto runResult = RunWslc(std::format(L"container run --rm {} echo Hello from saved image!", DebianImage.NameAndTag())); + runResult.Verify({.Stdout = L"Hello from saved image!\n", .Stderr = L"", .ExitCode = 0}); + } + private: const TestImage DebianImage = DebianTestImage(); const TestImage& InvalidImage = InvalidTestImage(); diff --git a/test/windows/wslc/e2e/WSLCExecutor.cpp b/test/windows/wslc/e2e/WSLCExecutor.cpp index 3472d4eba..5711713ae 100644 --- a/test/windows/wslc/e2e/WSLCExecutor.cpp +++ b/test/windows/wslc/e2e/WSLCExecutor.cpp @@ -163,6 +163,63 @@ void RunWslcAndVerify(const std::wstring& cmd, const WSLCExecutionResult& expect RunWslc(cmd, elevationType).Verify(expected); } +WSLCExecutionResult RunWslcAndRedirectToFile(const std::wstring& commandLine, std::optional outputPath, ElevationType elevationType) +{ + auto cmd = L"\"" + GetWslcPath() + L"\" " + commandLine; + wsl::windows::common::SubProcess process(nullptr, cmd.c_str()); + + // If running non-elevated we need to keep the token alive until it completes. + wil::unique_handle nonElevatedToken; + if (elevationType == ElevationType::NonElevated) + { + nonElevatedToken = GetNonElevatedPrimaryToken(); + process.SetToken(nonElevatedToken.get()); + } + + auto [parentStderrRead, childStderrWrite] = wsl::windows::common::wslutil::OpenAnonymousPipe(0, true, false); + THROW_IF_WIN32_BOOL_FALSE(SetHandleInformation(childStderrWrite.get(), HANDLE_FLAG_INHERIT, HANDLE_FLAG_INHERIT)); + + wil::unique_hfile redirectedStdout; + HANDLE stdoutHandle = nullptr; + + std::wstring effectiveCommandLine = commandLine; + if (outputPath.has_value()) + { + SECURITY_ATTRIBUTES securityAttributes{}; + securityAttributes.nLength = sizeof(securityAttributes); + securityAttributes.bInheritHandle = TRUE; + redirectedStdout.reset(CreateFileW( + outputPath->c_str(), GENERIC_WRITE, FILE_SHARE_READ, &securityAttributes, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr)); + THROW_LAST_ERROR_IF(!redirectedStdout); + stdoutHandle = redirectedStdout.get(); + effectiveCommandLine = std::format(L"{} > \"{}\"", commandLine, outputPath->wstring()); + } + else + { + // Open CONOUT$ so the child process receives a real console handle regardless of + // how the test runner has configured its own stdout (e.g. piped in CI). This + // makes IsConsoleHandle() return true inside wslc, which is the condition under + // test in WSLCE2E_Image_Save_ToTerminal_Fail. + SECURITY_ATTRIBUTES securityAttributes{}; + securityAttributes.nLength = sizeof(securityAttributes); + securityAttributes.bInheritHandle = TRUE; + redirectedStdout.reset( + CreateFileW(L"CONOUT$", GENERIC_WRITE, FILE_SHARE_READ | FILE_SHARE_WRITE, &securityAttributes, OPEN_EXISTING, 0, nullptr)); + THROW_LAST_ERROR_IF(!redirectedStdout); + stdoutHandle = redirectedStdout.get(); + } + + process.SetStdHandles(nullptr, stdoutHandle, childStderrWrite.get()); + + const auto processHandle = process.Start(); + childStderrWrite.reset(); + + const auto exitCode = wsl::windows::common::SubProcess::GetExitCode(processHandle.get()); + const auto stdErrOutput = wsl::shared::string::MultiByteToWide(ReadToString(parentStderrRead.get())); + + return {.CommandLine = std::move(effectiveCommandLine), .Stdout = L"", .Stderr = stdErrOutput, .ExitCode = exitCode}; +} + std::wstring GetWslcHeader() { std::wstringstream header; diff --git a/test/windows/wslc/e2e/WSLCExecutor.h b/test/windows/wslc/e2e/WSLCExecutor.h index 5f1612f71..c7a4fccba 100644 --- a/test/windows/wslc/e2e/WSLCExecutor.h +++ b/test/windows/wslc/e2e/WSLCExecutor.h @@ -94,6 +94,10 @@ struct WSLCInteractiveSession }; WSLCExecutionResult RunWslc(const std::wstring& commandLine, ElevationType elevationType = ElevationType::Elevated); +WSLCExecutionResult RunWslcAndRedirectToFile( + const std::wstring& commandLine, + std::optional outputPath = std::nullopt, + ElevationType elevationType = ElevationType::Elevated); void RunWslcAndVerify(const std::wstring& cmd, const WSLCExecutionResult& expected, ElevationType elevationType = ElevationType::Elevated); std::wstring GetWslcHeader();