[pull] master from microsoft:master

AGENTS.md

@@ -199,6 +199,37 @@ Default new gates to `false` and gate the **runtime entry point** into a
 feature, not its build, so the code still compiles and ships (and keeps
 getting exercised) while its behavior stays off by default.
 
+## libgit2 P/Invoke string marshalling (UTF-8, not ANSI)
+
+The libgit2 bindings live in `GVFS.Common/Git/LibGit2Repo.cs` under the
+`Native` class. **libgit2 treats every string it receives and returns as
+UTF-8** — paths, revspecs, config keys and values, and error messages.
+
+The .NET default for a bare `[DllImport]` string parameter is
+`CharSet.Ansi`, which encodes through the Windows ANSI code page and
+**silently corrupts non-ASCII input** (a repo path under a non-English user
+name, a non-ASCII branch name, etc.) — an unmappable character becomes `?`,
+so `git_repository_open` and friends resolve the wrong path or fail. There is
+no `[module: DefaultCharSet]` override in `GVFS.Common`, so the ANSI default
+applies unless each declaration opts out.
+
+When you add a libgit2 P/Invoke:
+
+- Annotate **every** `string` parameter, `out string` parameter, string return
+  value, and string struct field with
+  `[MarshalAs(UnmanagedType.LPUTF8Str)]`.
+- Do **not** use `CharSet.Unicode` — that marshals UTF-16, which libgit2 does
+  not accept (it is the same bug in the other direction).
+- For a function that returns a **borrowed** pointer owned by libgit2 (e.g.
+  `git_config_get_string`), marshal it as `IntPtr` and copy with
+  `Marshal.PtrToStringUTF8(...)`; do not marshal it as `out string`, or the
+  interop marshaller frees libgit2's heap memory with the wrong allocator.
+
+Because the mismatch only manifests through the native call, cover new
+string-carrying paths with a real-libgit2 test that uses a non-ASCII input
+(see `GVFS.FunctionalTests/Tests/LibGit2NonAsciiPathTests.cs`), not a
+mock-based unit test.
+
 ## Coding standards
 
 See [CONTRIBUTING.md](CONTRIBUTING.md) for StyleCop rules, error-handling

GVFS/GVFS.Common/Git/LibGit2Repo.cs

@@ -7,6 +7,24 @@
 
 namespace GVFS.Common.Git
 {
+    /// <summary>
+    /// In-process wrapper over libgit2 (git2.dll) for local Git object, tree, blob, and
+    /// config lookups without spawning git.exe.
+    /// </summary>
+    /// <remarks>
+    /// libgit2 marshalling contract: libgit2 treats EVERY string it receives and returns as
+    /// UTF-8 (paths, revspecs, config keys and values, error messages). The default marshalling
+    /// for a bare <c>[DllImport]</c> string parameter is <see cref="CharSet.Ansi"/>, which encodes
+    /// through the Windows ANSI code page and silently corrupts (or drops, as '?') any non-ASCII
+    /// byte — for example a repository path under a non-English Windows user name. When adding a
+    /// new libgit2 P/Invoke to <see cref="Native"/>, annotate every <c>string</c> parameter,
+    /// <c>out string</c> parameter, string return value, and string struct field with
+    /// <c>[MarshalAs(UnmanagedType.LPUTF8Str)]</c>. Do NOT use <see cref="CharSet.Unicode"/> — that
+    /// marshals UTF-16, which libgit2 does not accept. For a libgit2 function that returns a borrowed
+    /// pointer owned by libgit2 (for example <c>git_config_get_string</c>), marshal it as
+    /// <see cref="IntPtr"/> and copy with <see cref="Marshal.PtrToStringUTF8(IntPtr)"/> so the
+    /// interop marshaller does not free libgit2's heap memory.
+    /// </remarks>
     public class LibGit2Repo : IDisposable
     {
         private bool disposedValue = false;
@@ -572,6 +590,12 @@ protected bool CheckSafeDirectoryConfigForCaseSensitivityIssue(ITracer tracer, s
             return configuredMatchingDirectory != null && TryOpenRepo(configuredMatchingDirectory, out repoHandle) == Native.ResultCode.Success;
         }
 
+        /// <summary>
+        /// Raw libgit2 P/Invoke declarations. See the marshalling contract on
+        /// <see cref="LibGit2Repo"/>: every string parameter/return/field passed to or from
+        /// libgit2 MUST be <c>[MarshalAs(UnmanagedType.LPUTF8Str)]</c> (never the ANSI default,
+        /// never <see cref="CharSet.Unicode"/>).
+        /// </summary>
         public static class Native
         {
             public enum ResultCode : int
@@ -602,7 +626,7 @@ public static GitOid IntPtrToGitOid(IntPtr oidPtr)
             public static extern int Shutdown();
 
             [DllImport(Git2NativeLibName, EntryPoint = "git_revparse_single")]
-            public static extern ResultCode RevParseSingle(out IntPtr objectHandle, IntPtr repoHandle, string oid);
+            public static extern ResultCode RevParseSingle(out IntPtr objectHandle, IntPtr repoHandle, [MarshalAs(UnmanagedType.LPUTF8Str)] string oid);
 
             public static string GetLastError()
             {
@@ -612,7 +636,11 @@ public static string GetLastError()
                     return "Operation was successful";
                 }
 
-                return Marshal.PtrToStructure<GitError>(ptr).Message;
+                // git_error.message is a borrowed UTF-8 char* owned by libgit2. Keep it as an
+                // IntPtr and copy it with Marshal.PtrToStringUTF8 so the interop marshaller never
+                // frees libgit2's heap memory (same pattern as GitConfigEntry).
+                GitError error = Marshal.PtrToStructure<GitError>(ptr);
+                return error.Message == IntPtr.Zero ? null : Marshal.PtrToStringUTF8(error.Message);
             }
 
             [DllImport(Git2NativeLibName, EntryPoint = "git_error_last")]
@@ -621,16 +649,15 @@ public static string GetLastError()
             [StructLayout(LayoutKind.Sequential)]
             private struct GitError
             {
-                [MarshalAs(UnmanagedType.LPStr)]
-                public string Message;
+                public IntPtr Message;
 
                 public int Klass;
             }
 
             public static class Repo
             {
                 [DllImport(Git2NativeLibName, EntryPoint = "git_repository_open")]
-                public static extern ResultCode Open(out IntPtr repoHandle, string path);
+                public static extern ResultCode Open(out IntPtr repoHandle, [MarshalAs(UnmanagedType.LPUTF8Str)] string path);
 
                 [DllImport(Git2NativeLibName, EntryPoint = "git_repository_free")]
                 public static extern void Free(IntPtr repoHandle);
@@ -649,7 +676,7 @@ public static class Odb
                 public static extern void Free(IntPtr odbHandle);
 
                 [DllImport(Git2NativeLibName, EntryPoint = "git_oid_fromstr")]
-                public static extern ResultCode OidFromStr(out GitOid oid, string str);
+                public static extern ResultCode OidFromStr(out GitOid oid, [MarshalAs(UnmanagedType.LPUTF8Str)] string str);
             }
 
             public static class Config
@@ -664,13 +691,13 @@ public static class Config
                 public static extern ResultCode Snapshot(out IntPtr snapshotConfigHandle, IntPtr configHandle);
 
                 [DllImport(Git2NativeLibName, EntryPoint = "git_config_get_string")]
-                public static extern ResultCode GetString(out IntPtr value, IntPtr configHandle, string name);
+                public static extern ResultCode GetString(out IntPtr value, IntPtr configHandle, [MarshalAs(UnmanagedType.LPUTF8Str)] string name);
 
                 [DllImport(Git2NativeLibName, EntryPoint = "git_config_get_multivar_foreach")]
                 public static extern ResultCode GetMultivarForeach(
                     IntPtr configHandle,
-                    string name,
-                    string regex,
+                    [MarshalAs(UnmanagedType.LPUTF8Str)] string name,
+                    [MarshalAs(UnmanagedType.LPUTF8Str)] string regex,
                     GitConfigMultivarCallback callback,
                     IntPtr payload);
 
@@ -719,7 +746,7 @@ private static string MarshalUtf8String(IntPtr ptr)
                 }
 
                 [DllImport(Git2NativeLibName, EntryPoint = "git_config_get_bool")]
-                public static extern ResultCode GetBool(out bool value, IntPtr configHandle, string name);
+                public static extern ResultCode GetBool(out bool value, IntPtr configHandle, [MarshalAs(UnmanagedType.LPUTF8Str)] string name);
 
                 [DllImport(Git2NativeLibName, EntryPoint = "git_config_free")]
                 public static extern void Free(IntPtr configHandle);

GVFS/GVFS.FunctionalTests/Tests/LibGit2NonAsciiPathTests.cs

@@ -0,0 +1,107 @@
+using GVFS.Common.Git;
+using GVFS.Common.Tracing;
+using GVFS.FunctionalTests.Tools;
+using GVFS.Tests.Should;
+using NUnit.Framework;
+using System.IO;
+using System.Runtime.InteropServices;
+using System.Text;
+using GitProcess = GVFS.FunctionalTests.Tools.GitProcess;
+
+namespace GVFS.FunctionalTests.Tests
+{
+    /// <summary>
+    /// Exercises the real libgit2 (git2.dll) open/read path in <see cref="LibGit2Repo"/> against a
+    /// repository whose path and config value contain non-ASCII characters. This is a regression
+    /// guard for the string-marshalling bug where the libgit2 P/Invoke declarations used the
+    /// implicit <see cref="System.Runtime.InteropServices.CharSet.Ansi"/> default instead of UTF-8:
+    /// on a non-UTF-8 Windows code page the ANSI marshaller drops an unmappable character to '?',
+    /// so <c>git_repository_open</c> resolves the wrong path and the constructor throws. No
+    /// mock-based unit test can catch this because it only manifests through the native P/Invoke.
+    /// </summary>
+    [TestFixture]
+    public class LibGit2NonAsciiPathTests
+    {
+        // "café_日本語_☃" — Latin-1 accent, CJK, and a symbol outside every common Windows ANSI
+        // code page. Built from \u escapes so the reproduction does not depend on this source
+        // file's on-disk encoding.
+        private const string NonAsciiToken = "caf\u00e9_\u65e5\u672c\u8a9e_\u2603";
+
+        private const string StringConfigKey = "gvfs.functionaltests-nonascii";
+        private const string NonAsciiConfigValue = "caf\u00e9-\u65e5\u672c\u8a9e-\u2603";
+
+        private string repoRoot;
+
+        [OneTimeSetUp]
+        public void CreateRepo()
+        {
+            // The ANSI default marshalling only corrupts non-ASCII characters when the active
+            // Windows ANSI code page cannot represent them. If the process ANSI code page is
+            // UTF-8 (CP65001, the "Beta: Use Unicode UTF-8" setting), ANSI marshalling already
+            // produces correct UTF-8, so the bug cannot reproduce and this guard is inconclusive.
+            // Log the active code page so a reader of the test results can tell whether this guard
+            // actually ran (e.g. CP1252 on the standard CI runners) or was skipped as inconclusive.
+            const uint CP_UTF8 = 65001;
+            uint activeCodePage = GetACP();
+            TestContext.Progress.WriteLine($"{nameof(LibGit2NonAsciiPathTests)}: active ANSI code page = {activeCodePage}");
+            if (activeCodePage == CP_UTF8)
+            {
+                Assert.Ignore(
+                    $"Active ANSI code page is UTF-8 (CP{CP_UTF8}); ANSI marshalling already produces UTF-8, so the " +
+                    "ANSI-vs-UTF-8 marshalling bug cannot reproduce and this regression guard is inconclusive here. " +
+                    "Run on a non-UTF-8 code page (e.g. CP1252, the standard CI runner default) to exercise it.");
+            }
+
+            this.repoRoot = Path.Combine(
+                Path.GetTempPath(),
+                "GVFS.LibGit2NonAsciiPathTests_" + NonAsciiToken + "_" + Path.GetRandomFileName());
+            Directory.CreateDirectory(this.repoRoot);
+
+            GitProcess.Invoke(this.repoRoot, "init");
+            GitProcess.Invoke(this.repoRoot, "config user.name \"Functional Test User\"");
+            GitProcess.Invoke(this.repoRoot, "config user.email \"functional@test.com\"");
+
+            // Write the non-ASCII config value straight to .git/config as UTF-8 (no BOM) so the
+            // value round-tripped through libgit2 is not confounded by how git.exe would encode a
+            // command-line argument.
+            string configPath = Path.Combine(this.repoRoot, ".git", "config");
+            File.AppendAllText(
+                configPath,
+                $"[gvfs]\n\tfunctionaltests-nonascii = {NonAsciiConfigValue}\n",
+                new UTF8Encoding(encoderShouldEmitUTF8Identifier: false));
+        }
+
+        [OneTimeTearDown]
+        public void DeleteRepo()
+        {
+            if (this.repoRoot != null)
+            {
+                RepositoryHelpers.DeleteTestDirectory(this.repoRoot);
+            }
+        }
+
+        [TestCase]
+        public void OpensRepositoryAtNonAsciiPath()
+        {
+            // Before the LPUTF8Str marshalling fix, git_repository_open received an ANSI-mangled
+            // path, failed, and the constructor threw InvalidDataException.
+            using (LibGit2Repo repo = new LibGit2Repo(NullTracer.Instance, this.repoRoot))
+            {
+                // Reading a known config value proves the RepoHandle is real, not just non-null.
+                repo.GetConfigString("user.email").ShouldEqual("functional@test.com");
+            }
+        }
+
+        [TestCase]
+        public void GetConfigStringReturnsNonAsciiValue()
+        {
+            using (LibGit2Repo repo = new LibGit2Repo(NullTracer.Instance, this.repoRoot))
+            {
+                repo.GetConfigString(StringConfigKey).ShouldEqual(NonAsciiConfigValue);
+            }
+        }
+
+        [DllImport("kernel32.dll")]
+        private static extern uint GetACP();
+    }
+}