quill icon indicating copy to clipboard operation
quill copied to clipboard

creating a file_handler throws when the file path contains multibyte characters.

Open Heal-Bot opened this issue 2 years ago • 4 comments

I've noticed that if I try to create a file_handler with multibyte characters quill throws an exception trying to convert the std::filesystem::path into a std::string. This happens when the path is taken from QString::toStdWString(), std::string expanded to std::wstring, and std::filesystem::path constructed from raw bytes, std::string, std::wstring, etc.

Using Windows 11, MSVC 2022 (17.7.0), Quill's latest master branch commit.

Example Code:

  quill::FileHandlerConfig fileConfig;
  fileConfig.set_append_to_filename(quill::FilenameAppend::StartDateTime);
  fileConfig.set_open_mode('w');
  fileConfig.set_pattern(formatPattern.toStdString(), "%D %H:%M:%S.%Qms");
  fileConfig.set_timezone(quill::Timezone::LocalTime);

  std::string path("C:/Кузьма/log.txt"); // soufflé works, ทt throws, ç works

  int size_needed = MultiByteToWideChar(CP_UTF8, 0, &path[0], (int)path.size(), NULL, 0);
  std::wstring widePath( size_needed, 0 );
  MultiByteToWideChar(CP_UTF8, 0, &path[0], (int) path.size(), &widePath[0], size_needed);

  //std::wstring widePath = quill::detail::s2ws(path); same outcome

  mxFile_handler = quill::file_handler(widePath, fileConfig);

Stack Trace:

1  RaiseException                                                                      KERNELBASE               0x7ffc3981531c 
2  CxxThrowException                                                                   VCRUNTIME140             0x7ffc29e96720 
3  std::_Throw_system_error_from_std_win_error                                         chrono               64  0x7ff6c8b7f67a 
4  std::_Convert_wide_to_narrow<std::char_traits<char>,std::allocator<char>>           chrono               85  0x7ff6c8b7e34b 
5  std::filesystem::_Convert_wide_to<std::char_traits<char>,std::allocator<char>,char> filesystem           164 0x7ff6c8b7e259 
6  quill::file_handler                                                                 Quill.cpp            65  0x7ff6c8b7f978 

Heal-Bot avatar Sep 28 '23 21:09 Heal-Bot

I've made a quick, naive, example of using path::wstring() instead of path::string() that fixes the issue, but calls s2ws repeatedly as needed when interacting with other parts of Quill. https://github.com/Heal-Bot/quill/commit/fc0d595f419d16b7dbe6aa94ddffb3c145cf878c

I'm happy to polish this if you want to delegate it. However, I didn't see any contribution guidelines so I would need some guidance on how you want to navigate existing APIs, s2ws usage, and interactions with the current unit tests.

Heal-Bot avatar Sep 28 '23 21:09 Heal-Bot

Hey, sorry for the late response.

I can confirm that your example is not working.

I would prefer to keep the code handling wide strings minimal as it is windows specific.

I was not really expecting a wide characters stored into a std::string as in your example.

Would it be possible to use std::wstring for the file path and not QString ?

I have tested the following two examples using master and they work :

  1. provide the path as std::wstring
int main()
{
  quill::start();

  quill::FileHandlerConfig fileConfig;
  fileConfig.set_append_to_filename(quill::FilenameAppend::StartDateTime);
  fileConfig.set_open_mode('w');
  fileConfig.set_timezone(quill::Timezone::LocalTime);

  std::wstring path(L"C:/Кузьма/log.txt");
  auto mxFile_handler = quill::file_handler(path, fileConfig);
  auto logger = quill::create_logger("main", std::move(mxFile_handler));
  LOG_INFO(logger, "test");
}
  1. Convert an std::string that contains only ascii characters to a std::wstring
  quill::start();

  quill::FileHandlerConfig fileConfig;
  fileConfig.set_append_to_filename(quill::FilenameAppend::StartDateTime);
  fileConfig.set_open_mode('w');
  fileConfig.set_timezone(quill::Timezone::LocalTime);

  std::string path("C:/testlol/log.txt");

  int size_needed = MultiByteToWideChar(CP_UTF8, 0, &path[0], (int)path.size(), NULL, 0);
  std::wstring widePath( size_needed, 0 );
  MultiByteToWideChar(CP_UTF8, 0, &path[0], (int) path.size(), &widePath[0], size_needed);

  auto mxFile_handler = quill::file_handler(widePath, fileConfig);
  auto logger = quill::create_logger("main", std::move(mxFile_handler));
  LOG_INFO(logger, "test");

odygrd avatar Nov 04 '23 00:11 odygrd

Also can you try this example please ? Does it work ?

  quill::start();

  quill::FileHandlerConfig fileConfig;
  fileConfig.set_append_to_filename(quill::FilenameAppend::StartDateTime);
  fileConfig.set_open_mode('w');
  fileConfig.set_timezone(quill::Timezone::LocalTime);

  std::string utf8Path = "C:/Кузьма/log.txt";
  auto mxFile_handler = quill::file_handler(utf8Path, fileConfig);

  auto logger = quill::create_logger("main", std::move(mxFile_handler));
  LOG_INFO(logger, "test");

odygrd avatar Nov 04 '23 01:11 odygrd

I was saving the .cpp example file as utf8, when i started saving it with encoding utf16 i noticed that the above examples I asked you to try do not work

odygrd avatar Nov 04 '23 13:11 odygrd