From 0897d4a39abbafbf15f90774ae36b86917d9bc92 Mon Sep 17 00:00:00 2001 From: Adeel Mujahid <3840695+am11@users.noreply.github.com> Date: Thu, 19 Nov 2020 23:04:40 +0200 Subject: [PATCH] Use multibyte character path for bundle_probe (#44466) --- .../src/dlls/mscoree/unixinterface.cpp | 2 +- src/coreclr/src/inc/bundle.h | 9 +++--- src/coreclr/src/vm/bundle.cpp | 29 ++++++++++--------- src/installer/corehost/cli/hostmisc/pal.h | 3 -- .../corehost/cli/hostmisc/pal.unix.cpp | 11 ------- .../corehost/cli/hostmisc/pal.windows.cpp | 12 -------- .../cli/hostpolicy/hostpolicy_context.cpp | 15 ++-------- src/installer/corehost/corehost.cpp | 4 +-- .../TestProjects/BundleProbeTester/Program.cs | 2 +- 9 files changed, 26 insertions(+), 61 deletions(-) diff --git a/src/coreclr/src/dlls/mscoree/unixinterface.cpp b/src/coreclr/src/dlls/mscoree/unixinterface.cpp index e23ece9872244..fd8cf506b2344 100644 --- a/src/coreclr/src/dlls/mscoree/unixinterface.cpp +++ b/src/coreclr/src/dlls/mscoree/unixinterface.cpp @@ -220,7 +220,7 @@ int coreclr_initialize( if (bundleProbe != nullptr) { - static Bundle bundle(StringToUnicode(exePath), bundleProbe); + static Bundle bundle(exePath, bundleProbe); Bundle::AppBundle = &bundle; } diff --git a/src/coreclr/src/inc/bundle.h b/src/coreclr/src/inc/bundle.h index c669790350db5..e6c7a0a1dd18f 100644 --- a/src/coreclr/src/inc/bundle.h +++ b/src/coreclr/src/inc/bundle.h @@ -34,20 +34,20 @@ struct BundleFileLocation bool IsValid() const { LIMITED_METHOD_CONTRACT; return Offset != 0; } }; -typedef bool(__stdcall BundleProbe)(LPCWSTR, INT64*, INT64*); +typedef bool(__stdcall BundleProbe)(LPCSTR, INT64*, INT64*); class Bundle { public: - Bundle(LPCWSTR bundlePath, BundleProbe *probe); - BundleFileLocation Probe(LPCWSTR path, bool pathIsBundleRelative = false) const; + Bundle(LPCSTR bundlePath, BundleProbe *probe); + BundleFileLocation Probe(const SString& path, bool pathIsBundleRelative = false) const; const SString &Path() const { LIMITED_METHOD_CONTRACT; return m_path; } const SString &BasePath() const { LIMITED_METHOD_CONTRACT; return m_basePath; } static Bundle* AppBundle; // The BundleInfo for the current app, initialized by coreclr_initialize. static bool AppIsBundle() { LIMITED_METHOD_CONTRACT; return AppBundle != nullptr; } - static BundleFileLocation ProbeAppBundle(LPCWSTR path, bool pathIsBundleRelative = false); + static BundleFileLocation ProbeAppBundle(const SString& path, bool pathIsBundleRelative = false); private: @@ -55,6 +55,7 @@ class Bundle BundleProbe *m_probe; SString m_basePath; // The prefix to denote a path within the bundle + COUNT_T m_basePathLength; }; #endif // _BUNDLE_H_ diff --git a/src/coreclr/src/vm/bundle.cpp b/src/coreclr/src/vm/bundle.cpp index 3a204deb2cfb5..373ab591b432d 100644 --- a/src/coreclr/src/vm/bundle.cpp +++ b/src/coreclr/src/vm/bundle.cpp @@ -30,25 +30,26 @@ const SString &BundleFileLocation::Path() const return Bundle::AppBundle->Path(); } -Bundle::Bundle(LPCWSTR bundlePath, BundleProbe *probe) +Bundle::Bundle(LPCSTR bundlePath, BundleProbe *probe) { STANDARD_VM_CONTRACT; _ASSERTE(probe != nullptr); - m_path.Set(bundlePath); + m_path.SetUTF8(bundlePath); m_probe = probe; // The bundle-base path is the directory containing the single-file bundle. // When the Probe() function searches within the bundle, it masks out the basePath from the assembly-path (if found). - LPCWSTR pos = wcsrchr(bundlePath, DIRECTORY_SEPARATOR_CHAR_W); + LPCSTR pos = strrchr(bundlePath, DIRECTORY_SEPARATOR_CHAR_A); _ASSERTE(pos != nullptr); - size_t baseLen = pos - bundlePath + 1; // Include DIRECTORY_SEPARATOR_CHAR_W in m_basePath - m_basePath.Set(bundlePath, (COUNT_T)baseLen); + size_t baseLen = pos - bundlePath + 1; // Include DIRECTORY_SEPARATOR_CHAR_A in m_basePath + m_basePath.SetUTF8(bundlePath, (COUNT_T)baseLen); + m_basePathLength = (COUNT_T)baseLen; } -BundleFileLocation Bundle::Probe(LPCWSTR path, bool pathIsBundleRelative) const +BundleFileLocation Bundle::Probe(const SString& path, bool pathIsBundleRelative) const { STANDARD_VM_CONTRACT; @@ -59,17 +60,18 @@ BundleFileLocation Bundle::Probe(LPCWSTR path, bool pathIsBundleRelative) const // Bundle.Probe("path/to/exe/lib.dll") => m_probe("lib.dll") // Bundle.Probe("path/to/exe/and/some/more/lib.dll") => m_probe("and/some/more/lib.dll") + StackScratchBuffer scratchBuffer; + LPCSTR utf8Path(path.GetUTF8(scratchBuffer)); + if (!pathIsBundleRelative) { - size_t baseLen = m_basePath.GetCount(); - #ifdef TARGET_UNIX - if (wcsncmp(m_basePath, path, baseLen) == 0) + if (wcsncmp(m_basePath, path, m_basePath.GetCount()) == 0) #else - if (_wcsnicmp(m_basePath, path, baseLen) == 0) + if (_wcsnicmp(m_basePath, path, m_basePath.GetCount()) == 0) #endif // TARGET_UNIX { - path += baseLen; // m_basePath includes count for DIRECTORY_SEPARATOR_CHAR_W + utf8Path += m_basePathLength; // m_basePath includes count for DIRECTORY_SEPARATOR_CHAR_W } else { @@ -78,15 +80,14 @@ BundleFileLocation Bundle::Probe(LPCWSTR path, bool pathIsBundleRelative) const } } - m_probe(path, &loc.Offset, &loc.Size); + m_probe(utf8Path, &loc.Offset, &loc.Size); return loc; } -BundleFileLocation Bundle::ProbeAppBundle(LPCWSTR path, bool pathIsBundleRelative) +BundleFileLocation Bundle::ProbeAppBundle(const SString& path, bool pathIsBundleRelative) { STANDARD_VM_CONTRACT; return AppIsBundle() ? AppBundle->Probe(path, pathIsBundleRelative) : BundleFileLocation::Invalid(); } - diff --git a/src/installer/corehost/cli/hostmisc/pal.h b/src/installer/corehost/cli/hostmisc/pal.h index bf4785fb1a0d7..cdbb42a1a8def 100644 --- a/src/installer/corehost/cli/hostmisc/pal.h +++ b/src/installer/corehost/cli/hostmisc/pal.h @@ -164,7 +164,6 @@ namespace pal inline const char_t* strerror(int errnum) { return ::_wcserror(errnum); } bool pal_utf8string(const string_t& str, std::vector* out); - bool utf8_palstring(const std::string& str, string_t* out); bool pal_clrstring(const string_t& str, std::vector* out); bool clr_palstring(const char* cstr, string_t* out); @@ -223,7 +222,6 @@ namespace pal inline const char_t* strerror(int errnum) { return ::strerror(errnum); } inline bool pal_utf8string(const string_t& str, std::vector* out) { out->assign(str.begin(), str.end()); out->push_back('\0'); return true; } - inline bool utf8_palstring(const std::string& str, string_t* out) { out->assign(str); return true; } inline bool pal_clrstring(const string_t& str, std::vector* out) { return pal_utf8string(str, out); } inline bool clr_palstring(const char* cstr, string_t* out) { out->assign(cstr); return true; } @@ -305,7 +303,6 @@ namespace pal bool get_default_bundle_extraction_base_dir(string_t& extraction_dir); int xtoi(const char_t* input); - bool unicode_palstring(const char16_t* str, pal::string_t* out); bool get_loaded_library(const char_t *library_name, const char *symbol_name, /*out*/ dll_t *dll, /*out*/ string_t *path); bool load_library(const string_t* path, dll_t* dll); diff --git a/src/installer/corehost/cli/hostmisc/pal.unix.cpp b/src/installer/corehost/cli/hostmisc/pal.unix.cpp index 6996618239ebd..20774cb9e11f9 100644 --- a/src/installer/corehost/cli/hostmisc/pal.unix.cpp +++ b/src/installer/corehost/cli/hostmisc/pal.unix.cpp @@ -17,7 +17,6 @@ #include #include #include -#include #include #include "config.h" @@ -266,16 +265,6 @@ int pal::xtoi(const char_t* input) return atoi(input); } -bool pal::unicode_palstring(const char16_t* str, pal::string_t* out) -{ - out->clear(); - - std::wstring_convert, char16_t> conversion; - out->assign(conversion.to_bytes(str)); - - return true; -} - bool pal::is_path_rooted(const pal::string_t& path) { return path.front() == '/'; diff --git a/src/installer/corehost/cli/hostmisc/pal.windows.cpp b/src/installer/corehost/cli/hostmisc/pal.windows.cpp index c1a62a4d74d1e..7ddaab8c35060 100644 --- a/src/installer/corehost/cli/hostmisc/pal.windows.cpp +++ b/src/installer/corehost/cli/hostmisc/pal.windows.cpp @@ -8,7 +8,6 @@ #include #include -#include #include #include @@ -623,11 +622,6 @@ static bool wchar_convert_helper(DWORD code_page, const char* cstr, int len, pal return ::MultiByteToWideChar(code_page, 0, cstr, len, &(*out)[0], out->size()) != 0; } -bool pal::utf8_palstring(const std::string& str, pal::string_t* out) -{ - return wchar_convert_helper(CP_UTF8, &str[0], str.size(), out); -} - bool pal::pal_utf8string(const pal::string_t& str, std::vector* out) { out->clear(); @@ -652,12 +646,6 @@ bool pal::clr_palstring(const char* cstr, pal::string_t* out) return wchar_convert_helper(CP_UTF8, cstr, ::strlen(cstr), out); } -bool pal::unicode_palstring(const char16_t* str, pal::string_t* out) -{ - out->assign((const wchar_t *)str); - return true; -} - // Return if path is valid and file exists, return true and adjust path as appropriate. bool pal::realpath(string_t* path, bool skip_error_logging) { diff --git a/src/installer/corehost/cli/hostpolicy/hostpolicy_context.cpp b/src/installer/corehost/cli/hostpolicy/hostpolicy_context.cpp index 502d3a1b4a66b..48d7cd335dd91 100644 --- a/src/installer/corehost/cli/hostpolicy/hostpolicy_context.cpp +++ b/src/installer/corehost/cli/hostpolicy/hostpolicy_context.cpp @@ -23,18 +23,7 @@ namespace // This function is an API exported to the runtime via the BUNDLE_PROBE property. // This function used by the runtime to probe for bundled assemblies // This function assumes that the currently executing app is a single-file bundle. - // - // bundle_probe recieves its path argument as cha16_t* instead of pal::char_t*, because: - // * The host uses Unicode strings on Windows and UTF8 strings on Unix - // * The runtime uses Unicode strings on all platforms - // * Using a unicode encoded path presents a uniform interface to the runtime - // and minimizes the number if Unicode <-> UTF8 conversions necessary. - // - // The unicode char type is char16_t* instead of whcar_t*, because: - // * wchar_t is 16-bit encoding on Windows while it is 32-bit encoding on most Unix systems - // * The runtime uses 16-bit encoded unicode characters. - - bool STDMETHODCALLTYPE bundle_probe(const char16_t* path, int64_t* offset, int64_t* size) + bool STDMETHODCALLTYPE bundle_probe(const char* path, int64_t* offset, int64_t* size) { if (path == nullptr) { @@ -43,7 +32,7 @@ namespace pal::string_t file_path; - if (!pal::unicode_palstring(path, &file_path)) + if (!pal::clr_palstring(path, &file_path)) { trace::warning(_X("Failure probing contents of the application bundle.")); trace::warning(_X("Failed to convert path [%ls] to UTF8"), path); diff --git a/src/installer/corehost/corehost.cpp b/src/installer/corehost/corehost.cpp index 9ae142c4fe427..0064f8b6a2a9f 100644 --- a/src/installer/corehost/corehost.cpp +++ b/src/installer/corehost/corehost.cpp @@ -53,8 +53,7 @@ bool is_exe_enabled_for_execution(pal::string_t* app_dll) static const char hi_part[] = EMBED_HASH_HI_PART_UTF8; static const char lo_part[] = EMBED_HASH_LO_PART_UTF8; - std::string binding(&embed[0]); - if (!pal::utf8_palstring(binding, app_dll)) + if (!pal::clr_palstring(embed, app_dll)) { trace::error(_X("The managed DLL bound to this executable could not be retrieved from the executable image.")); return false; @@ -65,6 +64,7 @@ bool is_exe_enabled_for_execution(pal::string_t* app_dll) size_t hi_len = (sizeof(hi_part) / sizeof(hi_part[0])) - 1; size_t lo_len = (sizeof(lo_part) / sizeof(lo_part[0])) - 1; + std::string binding(&embed[0]); if ((binding.size() >= (hi_len + lo_len)) && binding.compare(0, hi_len, &hi_part[0]) == 0 && binding.compare(hi_len, lo_len, &lo_part[0]) == 0) diff --git a/src/installer/tests/Assets/TestProjects/BundleProbeTester/Program.cs b/src/installer/tests/Assets/TestProjects/BundleProbeTester/Program.cs index 187e5d4c6fce4..ac963679797cc 100644 --- a/src/installer/tests/Assets/TestProjects/BundleProbeTester/Program.cs +++ b/src/installer/tests/Assets/TestProjects/BundleProbeTester/Program.cs @@ -13,7 +13,7 @@ public static class Program // The bundle-probe callback is only called from native code in the product // Therefore the type on this test is adjusted to circumvent the failure. [UnmanagedFunctionPointer(CallingConvention.StdCall)] - public delegate byte BundleProbeDelegate([MarshalAs(UnmanagedType.LPWStr)] string path, IntPtr size, IntPtr offset); + public delegate byte BundleProbeDelegate([MarshalAs(UnmanagedType.LPUTF8Str)] string path, IntPtr size, IntPtr offset); unsafe static bool Probe(BundleProbeDelegate bundleProbe, string path, bool isExpected) {