Skip to content

Handle multibyte paths on Windows #852

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

Merged
merged 1 commit into from
Feb 6, 2025

Conversation

lxbndr
Copy link
Contributor

@lxbndr lxbndr commented Jan 20, 2025

DispatchIO is used in FoundationNetworking.URLSession for a file reading. The path is taken from URL.withUnsafeFileSystemRepresentation, which looks as expected behavior to me. It is multibyte string in fact, and on the Dispatch side we have to handle this accordingly.

We noted this as crashes/hangs in our product, happening when we tried to POST files with non-ascii characters in the path. We are running with this patch without any issues for a few months already, so it is kind of tested. We're running on 5.10.1 release branch, but as long as withUnsafeFileSystemRepresentation is not changed, everything should be the same.

@lxbndr
Copy link
Contributor Author

lxbndr commented Jan 20, 2025

cc @compnerd

@lxbndr lxbndr force-pushed the win-multibyte-paths branch from addd018 to d8f101e Compare January 24, 2025 14:26
@lxbndr
Copy link
Contributor Author

lxbndr commented Jan 24, 2025

Update:

  • Renamed wide_path to wszPath to match Windows code style
  • Extracted code related to MultiByteToWideChar into a helper. Basically followed this code from swiftlang/swift.
  • Added error handling. That was missing in original PR, and errors were crashing the app/tests. Now we gracefully call cleanup handler in case of error.

@compnerd please take a look

@lxbndr lxbndr force-pushed the win-multibyte-paths branch from d8f101e to 7f6ffb1 Compare January 24, 2025 14:36
@lxbndr lxbndr force-pushed the win-multibyte-paths branch from 7f6ffb1 to 97a4889 Compare February 4, 2025 14:27
@lxbndr
Copy link
Contributor Author

lxbndr commented Feb 4, 2025

Reworked version

  • some type/function aliasing for Win/Non-Win platforms
  • path in dispatch_io_path_data_s is now WCHAR[] on Windows
  • one single conversion from utf-8 to mbcs
  • general flow is same, no new edge cases
  • most significant changes are in dispatch_io_create_with_path part related to path data initialization

cc @compnerd

@lxbndr lxbndr requested a review from compnerd February 4, 2025 14:40
@compnerd
Copy link
Member

compnerd commented Feb 4, 2025

@swift-ci please test

@compnerd
Copy link
Member

compnerd commented Feb 4, 2025

@swift-ci please test Windows platform

@lxbndr lxbndr force-pushed the win-multibyte-paths branch from 97a4889 to ae1524c Compare February 5, 2025 09:35
@lxbndr lxbndr requested a review from compnerd February 5, 2025 14:47
@compnerd
Copy link
Member

compnerd commented Feb 5, 2025

@swift-ci please test

@compnerd
Copy link
Member

compnerd commented Feb 5, 2025

@swift-ci please test Windows platform

@compnerd compnerd merged commit 58fc9c5 into swiftlang:main Feb 6, 2025
2 checks passed
@lxbndr lxbndr deleted the win-multibyte-paths branch February 6, 2025 22:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants