Fix ArchGetFileName on windows for file paths with special characters #3545
Conversation
ArchGetFileName called std::filesystem::canonical() with an utf8-encoded std::string, but on windows std::filesystem::path expects an std::string with system encoding. To fix this issue the std::filesystem::path is now created from the utf16-encoded string, which is returned by GetFinalPathNameByHandleW().
|
Do you have a test case that shows the failure state? Simplifying things the function to use the existing |
|
You just need a usd file with a non-ASCII character in it's name, e.g. "würfel.usd". If you try to open it in usdview on windows, std::filesystem::canonical raises an exception. |
|
Filed as internal issue #USD-10702 (This is an automated message. See here for more information.) |
|
/AzurePipelines run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| @@ -96,7 +96,12 @@ int main() | |||
| // Open a file, check that the file path from FILE* handle is matched. | |||
| ARCH_AXIOM((firstFile = ArchOpenFile(firstName.c_str(), "rb")) != NULL); | |||
| std::string filePath = ArchGetFileName(firstFile); | |||
| #if defined(ARCH_OS_WINDOWS) | |||
There was a problem hiding this comment.
I'm curious if there a solution that could use u8path to avoid forking the test behavior around the platform?
There was a problem hiding this comment.
Forking could be avoided using u8path. I didn't use it because it's deprecated in C++20 and may be removed with future versions (C++26 or later). But if this is no problem I would change it.
Description of Change(s)
ArchGetFileName called std::filesystem::canonical() with an utf8-encoded std::string, but on windows std::filesystem::path expects std::string with system encoding. To fix this issue the std::filesystem::path is now created from the utf16-encoded string, which is returned by GetFinalPathNameByHandleW().
Checklist
I have created this PR based on the dev branch
I have followed the coding conventions
I have added unit tests that exercise this functionality (Reference:
testing guidelines)
I have verified that all unit tests pass with the proposed changes
I have submitted a signed Contributor License Agreement (Reference:
Contributor License Agreement instructions)