Fix MrmGetFilePathFromName to return S_OK with a valid fallback path (supersedes #5828)

dev/MRTCore/mrt/Core/src/MRM.cpp

@@ -16,6 +16,7 @@
 #include "MRM.h"
 
 #include <memory>
+#include <string>
 
 using namespace Microsoft::Resources;
 
@@ -980,6 +981,11 @@ STDAPI MrmGetFilePathFromName(_In_opt_ PCWSTR filename, _Outptr_ PWSTR* filePath
     RETURN_IF_FAILED(SizeTAdd(exeDirCount, 1, &exeDirCount));
     RETURN_IF_FAILED(PathCchRemoveFileSpec(exeDir.get(), exeDirCount));
 
+    // The ParentPathForFileName pass truncates exeDir in place to reach the parent folder, so
+    // preserve the original module directory here for the DefaultFallback pass, which must return
+    // a path under the module directory (the documented behavior).
+    std::wstring moduleDir(exeDir.get());
+
     enum SearchPass
     {
         exeDirForFileName,
@@ -988,6 +994,7 @@ STDAPI MrmGetFilePathFromName(_In_opt_ PCWSTR filename, _Outptr_ PWSTR* filePath
         BaseDirForModulePri,
         exeDirForResourcesPri,
         exeDirForModulePri,
+        DefaultFallback,
         Final,
     };
     SearchPass searchStart = SearchPass::exeDirForFileName;
@@ -1030,7 +1037,10 @@ STDAPI MrmGetFilePathFromName(_In_opt_ PCWSTR filename, _Outptr_ PWSTR* filePath
             case SearchPass::ParentPathForFileName:
                 // move to parent folder of previous search
                 RETURN_IF_FAILED(PathCchRemoveFileSpec(searchDir, searchDirCount));
-                pass = SearchPass::Final;
+                // If the parent search misses, skip the no-FileName passes and fall through to
+                // DefaultFallback so we still return a best-effort path (the documented
+                // always-succeed contract). The loop's increment lands pass on DefaultFallback.
+                pass = SearchPass(SearchPass::DefaultFallback - 1);
                 break;
 
             // Search, given no FileName
@@ -1050,7 +1060,34 @@ STDAPI MrmGetFilePathFromName(_In_opt_ PCWSTR filename, _Outptr_ PWSTR* filePath
                 break;
             case SearchPass::exeDirForModulePri:
                 searchFilename = modulePriFileName;
-                pass = SearchPass::Final;
+                // If this search misses, fall through to DefaultFallback (the next pass) so we
+                // still return a best-effort path (the documented always-succeed contract).
+                break;
+            case SearchPass::DefaultFallback:
+                // No file was found in any prior pass. Return a best-effort path even though it
+                // doesn't exist, matching the documented contract: the provided filename (if any)
+                // or resources.pri, under the base directory (if set) otherwise the module (exe)
+                // directory. Callers (e.g. ResourceManager) tolerate a non-existent path.
+                if (filename == nullptr || *filename == L'\0')
+                {
+                    if (baseDir)
+                    {
+                        searchDir = baseDir.get();
+                        searchDirCount = baseDirCount;
+                    }
+                    else
+                    {
+                        searchDir = moduleDir.data();
+                        searchDirCount = exeDirCount;
+                    }
+                    searchFilename = ResourcesPriFileName;
+                }
+                else
+                {
+                    searchDir = moduleDir.data();
+                    searchDirCount = exeDirCount;
+                    searchFilename = filename;
+                }
                 break;
         }
 
@@ -1074,13 +1111,13 @@ STDAPI MrmGetFilePathFromName(_In_opt_ PCWSTR filename, _Outptr_ PWSTR* filePath
             PATHCCH_ALLOW_LONG_PATHS));
 
         DWORD attributes = GetFileAttributes(outputPath.get());
-        if ((attributes != INVALID_FILE_ATTRIBUTES) && !(attributes & FILE_ATTRIBUTE_DIRECTORY))
+        if ((pass == SearchPass::DefaultFallback) ||
+            ((attributes != INVALID_FILE_ATTRIBUTES) && !(attributes & FILE_ATTRIBUTE_DIRECTORY)))
         {
-            // The file exists. Done.
             *filePath = outputPath.release();
-            return S_OK;
+            break;
         }
     }
 
-    return HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND);
+    return S_OK;
 }

dev/MRTCore/mrt/Core/unittests/MrmTests.cpp

@@ -685,8 +685,20 @@ class BasicTest
         MrmFreeResource(path);
 
         VERIFY_ARE_EQUAL(MrmGetFilePathFromName(L"something.pri", &path), S_OK);
-        // Even if the file doesn't exist, we will still return a path. This is for those don't use PRI for resource.
+        // Even if the file doesn't exist, we still return a best-effort path so apps without a PRI
+        // file can still create a ResourceManager. The path is under the module (exe) directory.
         VERIFY_IS_NOT_NULL(wcsstr(path, L"something.pri"));
+        {
+            wchar_t moduleFilePath[MAX_PATH];
+            VERIFY_ARE_NOT_EQUAL(0u, GetModuleFileNameW(nullptr, moduleFilePath, ARRAYSIZE(moduleFilePath)));
+            PWSTR lastSeparator = wcsrchr(moduleFilePath, L'\\');
+            VERIFY_IS_NOT_NULL(lastSeparator);
+            lastSeparator[1] = L'\0'; // Trim to "<moduleDir>\"
+            wchar_t expectedPath[MAX_PATH];
+            VERIFY_ARE_EQUAL(0, wcscpy_s(expectedPath, ARRAYSIZE(expectedPath), moduleFilePath));
+            VERIFY_ARE_EQUAL(0, wcscat_s(expectedPath, ARRAYSIZE(expectedPath), L"something.pri"));
+            VERIFY_ARE_EQUAL(CSTR_EQUAL, CompareStringOrdinal(path, -1, expectedPath, -1, TRUE));
+        }
         MrmFreeResource(path);
 
         VERIFY_ARE_EQUAL(MrmGetFilePathFromName(nullptr, &path), S_OK);

dev/MRTCore/mrt/Microsoft.Windows.ApplicationModel.Resources/src/Helper.cpp

@@ -87,16 +87,22 @@ HRESULT GetDefaultPriFile(winrt::hstring& filePath)
     bool isPackaged = AppModel::Identity::IsPackagedProcess();
     hr = GetDefaultPriFileForCurentModule(isPackaged, filePath);
 
-    // Sparse-packaged apps have identity but deploy PRI files as loose files; fall back to unpackaged discovery which also searches for "[modulename].pri".
-    if (isPackaged && IsResourceNotFound(hr))
+    // Sparse-packaged apps have identity but deploy PRI files as loose files; fall back to
+    // unpackaged discovery which also searches for "[modulename].pri". MrmGetFilePathFromName now
+    // returns S_OK with a best-effort (possibly non-existent) path, so gate the fallback on whether
+    // the packaged lookup actually resolved to an existing file rather than on IsResourceNotFound(hr).
+    if (isPackaged && (IsResourceNotFound(hr) || (SUCCEEDED(hr) && FAILED(CheckFile(filePath.c_str())))))
     {
-        HRESULT hrFallback = GetDefaultPriFileForCurentModule(false, filePath);
-        if (SUCCEEDED(hrFallback))
+        winrt::hstring fallbackPath;
+        HRESULT hrFallback = GetDefaultPriFileForCurentModule(false, fallbackPath);
+        // Only adopt the unpackaged result if it resolves to an existing file; otherwise keep the
+        // packaged best-effort path so ResourceManager still receives a path (and handles the
+        // not-found case gracefully).
+        if (SUCCEEDED(hrFallback) && SUCCEEDED(CheckFile(fallbackPath.c_str())))
         {
+            filePath = fallbackPath;
             hr = hrFallback;
         }
-        // If the fallback also fails, preserve the original HRESULT (not the fallback's)
-        // for backward compatibility so callers checking for specific errors (e.g., ERROR_FILE_NOT_FOUND) are not broken.
     }
 
     return hr;