<filesystem>: system_error thrown from copy_file

[fkutzner]

Describe the bug
When a std::filesystem::path object p contains characters that cannot be represented in the current code page, p.string() throws a system_error because of the conversion failure. This may cause a system_error exception being thrown from a filesystem_error constructor.

These system_error exceptions are not catched in the filesystem_error constructor or the std::filesystem::copy_file function. I think the standard intends copy_file to still throw a filesystem_error in this situation (http://eel.is/c++draft/fs.err.report#2.1). Other functions constructing filesystem_error exceptions are likely affected as well.

Command-line test case
Set the system locale to English (United States)

C:\repro> type fsbug.cpp
#include <filesystem>
#include <iostream>

namespace fs = std::filesystem;

int main() {
  std::wstring const alef = L"\u05D0";

  try {
    fs::copy_file(fs::path{ alef }, fs::path{ L"Foo" });
  }
  catch (fs::filesystem_error const& e) {
    std::cout << e.what() << "\n";
  }
  catch (std::system_error const& e) {
    std::cout << "This line should not be reachable\n";
    std::cout << e.what() << "\n";
  }
  return 0;
}

C:\repro> cl.exe /D _UNICODE /D UNICODE /Zc:wchar_t /std:c++17 /EHsc fsbug.cpp
Microsoft (R) C/C++ Optimizing Compiler Version 19.24.28315 for x86
Copyright (C) Microsoft Corporation.  All rights reserved.

fsbug.cpp
Microsoft (R) Incremental Linker Version 14.24.28315.0
Copyright (C) Microsoft Corporation.  All rights reserved.

/out:fsbug.exe
fsbug.obj

C:\repro> .\fsbug.exe
This line should not be reachable
No mapping for the Unicode character exists in the target multi-byte code page.

Expected behavior
filesystem_error should be thrown by copy_file, not system_error. I think constructing a filesystem_error with paths not representable in the current locale should not cause the construction of the filesystem_error object to fail.

STL version

Microsoft Visual Studio Professional 2019
Version 16.4.3 

(Edit: minor clarifications)

[Chronial]

The relevant codepath is this: copy_file fails and calls _Throw_fs_error:

STL/stl/inc/filesystem

Line 3299 in 3f6c715

_Throw_fs_error("copy_file", _Result._Error, _From, _To);

There, a std::filesystem_error is constructed, with the two paths as arguments. filesystem_error calls its internal helper _Pretty_message to turn its arguments into a message. Following the standard, the given paths are turned into native narrow encoding and added to the message:

STL/stl/inc/filesystem

Lines 1779 to 1780 in 3f6c715

	const string _Path1_str = _Path1.string();
	const string _Path2_str = _Path2.string();

The string() call is where the system_error is thrown.

Unfortunately, it is not clear to me what exactly the root of the bug is, since the standard is not clear on whether string() is allowed to throw on encoding errors.

• There is a mention of characters that can't be converted and it reads to me like they should just have an arbitrary representation instead of an exception being thrown. I would personally not like such an interface – IMHO string() should throw on conversion error instead of silently returning an incorrect result.
• It looks like if an exception is thrown, it should be filesystem_error, not system_error?
• Changing from throwing to not-throwing would probably be a surprising change to users. Changing system_error to filesystem_error might be fine, since that is a subclass of system_error.

But if string() can throw, that is clearly not the desired behavior in the filesystem_error constructor. So it probably shouldn't call string(), but instead do the conversion itself and replace any unrepresentable characters. That seems to be allowed by the standard.

This could be implemented by adding a __std_fs_convert_wide_to_narrow_replace_chars similar to __std_fs_convert_wide_to_narrow but always using WideCharToMultiByte(..., WC_NO_BEST_FIT_CHARS, ...). @StephanTLavavej would you accept a PR for this?

[StephanTLavavej]

It seems like anything would be preferable to being unable to throw a filesystem_error because we can't narrow the paths. If we replace unrepresentable characters with something like '?' which is forbidden in the ordinary parts of paths, that should be a strong indication that the filesystem_error is doing its best to deal with the const char* interface.

We'll definitely consider a PR! We can accept additional functions being added to the separately compiled library, because that's how <filesystem> shipped in the first place - we added them to the import lib (effectively always statically linked) to avoid modifying the DLL's export surface.