Win32ColourImpl has problems with tests that redirect stdout

[dthaler]

Describe the bug
Win32ColourImpl saves GetStdHandle(STD_OUTPUT_HANDLE) and then assumes the handle will be valid throughout the test. This assumption is not true if the test uses freopen or _dup2() to redirect stdout, which closes the underlying handle. This results in setTextAttribute() passing an invalid handle to SetConsoleTextAttribute(). If appverif is enabled for handle checking, this hits an app verifier exception.

Expected behavior
Color should work normally, even with application verifier enabled for handle checking, and even when the test being executed redirects stdout and then restores it.

Reproduction steps
Create a test file as follows:

#include <io.h>
#define CATCH_CONFIG_MAIN
#include "catch2\catch.hpp"

TEST_CASE("test", "[test]")
{
    HANDLE original_handle = GetStdHandle(STD_OUTPUT_HANDLE);
    int fdDup = _dup(_fileno(stdout));
    FILE* fp;
    freopen_s(&fp, "stdout.out", "w", stdout); // Redirect stdout to a file.
    printf("Hello world\n");
    _dup2(fdDup, _fileno(stdout)); // Restore stdout to original.
    _close(fdDup);
    HANDLE new_handle = GetStdHandle(STD_OUTPUT_HANDLE);
    assert(new_handle != original_handle);
}

Enable application verifier handle checking: appverif -enable handles -for test.exe
Run "test.exe [test]" under a debugger. E.g., in visual studio set the debugger arguments to "[test]". The use of a filter will force catch to initialize Win32ColourImpl before the test runs, and then try to use the handle after the test completes.

Platform information:
OS: Windows 10
Compiler version: Visual Studio 2019
Catch version: v2.8.0 using the CatchOrg.Catch nuget package

[horenmar]

So, if I understand this correctly, the handle needs to be reloaded before every SetConsoleTextAttribute?

[dthaler]

So, if I understand this correctly, the handle needs to be reloaded before every SetConsoleTextAttribute?

Yes.

[horenmar]

Ok.

I might end up tearing out the Windows cmd colouring support (supporting it together with multireporters is going to be pain and half), but that's an easy enough fix to merge it in the meantime.