DaanDeMeyer / reproc

A cross-platform (C99/C++11) process library
MIT License
552 stars 65 forks source link

Multi wide character error #48

Closed Milerius closed 3 years ago

Milerius commented 3 years ago

Hello i have the following error when i try to launch a process:

command line: [C:\Users\Public\Documents\atomicDEX-Desktop\cmake-build-debug\bin\assets\tools/mm2/mm2], from directory: [C:\Use
rs\Public\Documents\atomicDEX-Desktop\cmake-build-debug\bin\assets\tools/mm2/]

error when spawning process No mapping for the Unicode character exists in the target multi-byte code page
        options.working_directory = strdup(tools_path.string().c_str());

        SPDLOG_DEBUG("command line: [{}], from directory: [{}]", args[0], options.working_directory);
        const auto ec = m_mm2_instance.start(args, options);
        std::free((void*)options.working_directory);
        if (ec)
        {
            SPDLOG_ERROR("error when spawning process {}", ec.message());
            std::exit(EXIT_FAILURE);
        }
Milerius commented 3 years ago

image

I'm using https://en.cppreference.com/w/cpp/filesystem/temp_directory_path which seems for this specific users to returns a string like: "MM_CONF_PATH=C:\\Users\\YUSUFK~1\\AppData\\Local\\Temp\\MM2.json"

Trying to extend the env with this value but produce 1113 windows error code

 fs::path mm2_cfg_path = (fs::temp_directory_path() / "MM2.json");

        std::ofstream ofs(mm2_cfg_path.string());
        ofs << json_cfg.dump(1, ' ', false, nlohmann::json::error_handler_t::ignore);
        // std::cout << json_cfg.dump() << std::endl;
        ofs.close();
        const std::array<std::string, 1> args = {(tools_path / "mm2").string()};
        reproc::options                  options;
        options.redirect.parent = false;

        options.env.behavior = reproc::env::extend;
        options.env.extra    = std::unordered_map<std::string, std::string>{
            {"MM_CONF_PATH", mm2_cfg_path.string()},
            {"MM_LOG", utils::get_mm2_atomic_dex_current_log_file().string()},
            {"MM_COINS_PATH", (utils::get_current_configs_path() / "coins.json").string()}};

        options.working_directory = strdup(tools_path.string().c_str());
Milerius commented 3 years ago

To reproduce you can create a windows user named: Yusuf KOÇ and try to extend the env with std::filesystem::temp_directory_path() / "file.json"

You will never be able to start a process.

You can also see that Yusuf KOÇ become YUSUFK~1 which is a sugar syntax from Microsoft to avoid unicode hell

DaanDeMeyer commented 3 years ago

Unfortunately, I don't have a windows system at hand anymore. From looking at MM_CONF_PATH, it seems either the '~1' or the double '\' could be causing the issue.

Could you try modifying the path a bit to try and figure out exactly which character sequence is causing the error?

Milerius commented 3 years ago

Unfortunately, I don't have a windows system at hand anymore. From looking at MM_CONF_PATH, it seems either the '~1' or the double '\' could be causing the issue.

Could you try modifying the path a bit to try and figure out exactly which character sequence is causing the error?

Obviously the char: Ç is causing the error, but this is well handled by microsoft atm since it's became YUSUFK~1

DaanDeMeyer commented 3 years ago

Can you try using the u8string() method to convert a path to a string? reproc expects UTF-8 encoded strings as input. I think using string() on Windows gives back UTF-16 strings.

Milerius commented 3 years ago

I confirm if i manually enter in the windows explorer: C:\Users\YUSUFK~1 it expands to C:\Users\Yusuf KOÇ

So it's should be a known syntax btw : https://stresstest.atomicdex.io/ more than 800 users are testing our app right now and using reproc :p

Will test the above solution

Milerius commented 3 years ago

Can you try using the u8string() method to convert a path to a string? reproc expects UTF-8 encoded strings as input. I think using string() on Windows gives back UTF-16 strings.

Even using u8string() produce the same result, logs:

command
 line: C:\Users\Public\Documents\atomicDEX-Desktop\cmake-build-debug\bin\assets\tools/mm2/mm2, from directory: C:\Users\
Public\Documents\atomicDEX-Desktop\cmake-build-debug\bin\assets\tools/mm2/ conf_path: C:\Users\YUSUFK~1\AppData\Local\Te
mp\MM2.json
DaanDeMeyer commented 3 years ago

So if you pass u8"K~1" to reproc as environment value, you get the same error? Also, what happens if you pass u8"KOÇ" instead?

Milerius commented 3 years ago

So if you pass u8"K~1" to reproc as environment value, you get the same error? Also, what happens if you pass u8"KOÇ" instead?

u8"YUSUFK~1" -> works

u8"Yusuf KOÇ" -> works

using generic_u8string() for everything seems to start the process correctly !

DaanDeMeyer commented 3 years ago

Can you post the strings returned by u8string and generic_u8string? I'm curious what the exact difference is there. This might be something to add to reproc's documentation.

Milerius commented 3 years ago

Can you post the strings returned by u8string and generic_u8string? I'm curious what the exact difference is there. This might be something to add to reproc's documentation.

only \\\ become /

DaanDeMeyer commented 3 years ago

UTF conversion and paths are defintely not as well tested as they should be. Feel free to post a PR that adds some of these cases as new UTF-8 and path tests if you have time. That makes sure this keeps working as expected.

Milerius commented 3 years ago

UTF conversion and paths are defintely not as well tested as they should be. Feel free to post a PR that adds some of these cases as new UTF-8 and path tests if you have time. That makes sure this keeps working as expected.

Sure ! thanks a lot for your help man