Skip to content

Fixes for unicode path handling#28390

Merged
adrianlizarraga merged 4 commits into
microsoft:mainfrom
jnagi-intel:fix-unicode-paths
May 13, 2026
Merged

Fixes for unicode path handling#28390
adrianlizarraga merged 4 commits into
microsoft:mainfrom
jnagi-intel:fix-unicode-paths

Conversation

@jnagi-intel

@jnagi-intel jnagi-intel commented May 7, 2026

Copy link
Copy Markdown
Contributor

Description

Replace path::string() / bare std::filesystem::path(string) with
PathToUTF8String / ToPathString in two places that handle user-supplied
paths.

Motivation and Context

On Windows, path::string() and std::filesystem::path(std::string) use the
system ANSI code page (CP_ACP). When a model or EPContext output file sits in a
directory with non-ASCII Unicode characters, this corrupts the path and causes:

  • ModelMetadefIdGenerator::GenerateId — throws during session initialization
    ("No mapping for the Unicode character exists in the target multi-byte code page")
  • ModelGenOptions (EPContext options) — constructs a garbled path, failing
    EPContext file creation with ENOENT

@adrianlizarraga adrianlizarraga requested a review from Copilot May 7, 2026 19:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Updates EP context output path handling to correctly interpret UTF-8 session config values as Unicode paths on Windows.

Changes:

  • Add ToPathString() conversion when constructing std::filesystem::path from ep_context_file_path.
  • Add explanatory comments about Windows UTF-8 vs ANSI code page path decoding.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread onnxruntime/core/framework/ep_context_options.cc Outdated
@jnagi-intel jnagi-intel requested a review from Copilot May 11, 2026 18:27
@jnagi-intel jnagi-intel changed the title [EP Context] Use ToPathString() for ep_context_file_path to correctly handle UTF-8 Unicode paths on Windows Fixed for unicode path handling May 11, 2026
@jnagi-intel jnagi-intel changed the title Fixed for unicode path handling Fixes for unicode path handling May 11, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

@adrianlizarraga adrianlizarraga left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be great to add a unit test that uses non-ascii unicode characters for model compilation. Could be placed in this file:

TEST(OrtEpLibrary, PluginEp_GenEpContextModel_ErrorOutputModelExists_AutoGenOutputModelName) {

@jnagi-intel

Copy link
Copy Markdown
Contributor Author

Would be great to add a unit test that uses non-ascii unicode characters for model compilation. Could be placed in this file:

TEST(OrtEpLibrary, PluginEp_GenEpContextModel_ErrorOutputModelExists_AutoGenOutputModelName) {

@adrianlizarraga Added tests for both the fixes.

@adrianlizarraga adrianlizarraga left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more small issue.

Comment thread onnxruntime/test/autoep/test_execution.cc
Comment thread onnxruntime/test/autoep/test_execution.cc
@adrianlizarraga adrianlizarraga merged commit 9b3fb24 into microsoft:main May 13, 2026
89 of 90 checks passed
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.

3 participants