Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Heal-Bot opened this issue Sep 28, 2023 · 4 comments
Labels
bug Something isn't working enhancement New feature or request

Comments

@Heal-Bot
Copy link

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
Copy link
Author

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. Heal-Bot@fc0d595

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.

@odygrd
Copy link
Owner

odygrd commented Nov 4, 2023

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
Copy link
Owner

odygrd commented Nov 4, 2023

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 odygrd added bug Something isn't working enhancement New feature or request and removed bug Something isn't working enhancement New feature or request labels Nov 4, 2023
@odygrd
Copy link
Owner

odygrd commented Nov 4, 2023

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 odygrd added bug Something isn't working enhancement New feature or request labels Nov 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants