diff --git a/GVFS/GVFS.Common/Git/LibGit2Repo.cs b/GVFS/GVFS.Common/Git/LibGit2Repo.cs index dafcc8d54..00bc55e73 100644 --- a/GVFS/GVFS.Common/Git/LibGit2Repo.cs +++ b/GVFS/GVFS.Common/Git/LibGit2Repo.cs @@ -259,18 +259,39 @@ public virtual string GetConfigString(string name) } try { - string value; - Native.ResultCode resultCode = Native.Config.GetString(out value, configHandle, name); - if (resultCode == Native.ResultCode.NotFound) + // git_config_get_string returns a borrowed pointer whose lifetime is tied to the + // config, so libgit2 only allows it on a snapshot (read-only) config. Calling it on + // the live config returned by git_repository_config fails with "get_string called on + // a live config object". Snapshot the config first, then read the string from it. + IntPtr snapshotHandle; + if (Native.Config.Snapshot(out snapshotHandle, configHandle) != Native.ResultCode.Success) { - return null; + throw new LibGit2Exception($"Failed to snapshot config for '{name}': {Native.GetLastError()}"); } - else if (resultCode != Native.ResultCode.Success) + + try { - throw new LibGit2Exception($"Failed to get config value for '{name}': {Native.GetLastError()}"); - } + // git_config_get_string yields a borrowed pointer owned by the (snapshot) + // config, so it is retrieved as an IntPtr and copied manually. Marshalling it + // directly as an out string would make the interop marshaller free the pointer + // with CoTaskMemFree, corrupting libgit2's heap (mismatched allocator). + IntPtr valuePtr; + Native.ResultCode resultCode = Native.Config.GetString(out valuePtr, snapshotHandle, name); + if (resultCode == Native.ResultCode.NotFound) + { + return null; + } + else if (resultCode != Native.ResultCode.Success) + { + throw new LibGit2Exception($"Failed to get config value for '{name}': {Native.GetLastError()}"); + } - return value; + return valuePtr == IntPtr.Zero ? null : Marshal.PtrToStringUTF8(valuePtr); + } + finally + { + Native.Config.Free(snapshotHandle); + } } finally { @@ -585,8 +606,11 @@ public static class Config [DllImport(Git2NativeLibName, EntryPoint = "git_config_open_default")] public static extern ResultCode GetGlobalAndSystemConfig(out IntPtr configHandle); + [DllImport(Git2NativeLibName, EntryPoint = "git_config_snapshot")] + public static extern ResultCode Snapshot(out IntPtr snapshotConfigHandle, IntPtr configHandle); + [DllImport(Git2NativeLibName, EntryPoint = "git_config_get_string")] - public static extern ResultCode GetString(out string value, IntPtr configHandle, string name); + public static extern ResultCode GetString(out IntPtr value, IntPtr configHandle, string name); [DllImport(Git2NativeLibName, EntryPoint = "git_config_get_multivar_foreach")] public static extern ResultCode GetMultivarForeach( diff --git a/GVFS/GVFS.FunctionalTests/Tests/LibGit2ConfigTests.cs b/GVFS/GVFS.FunctionalTests/Tests/LibGit2ConfigTests.cs new file mode 100644 index 000000000..f1ed89acd --- /dev/null +++ b/GVFS/GVFS.FunctionalTests/Tests/LibGit2ConfigTests.cs @@ -0,0 +1,79 @@ +using GVFS.Common.Git; +using GVFS.Common.Tracing; +using GVFS.FunctionalTests.Tools; +using GVFS.Tests.Should; +using NUnit.Framework; +using System.IO; +using GitProcess = GVFS.FunctionalTests.Tools.GitProcess; + +namespace GVFS.FunctionalTests.Tests +{ + /// + /// Exercises the real libgit2 (git2.dll) config-read path in + /// against a plain on-disk git repository. This is a regression guard for the + /// "get_string called on a live config object" failure, which no mock-based unit + /// test can catch because it only manifests through the native P/Invoke. + /// + [TestFixture] + public class LibGit2ConfigTests + { + private const string StringConfigKey = "gvfs.functionaltests-teststring"; + private const string StringConfigValue = "libgit2-value-42"; + private const string BoolConfigKey = "gvfs.functionaltests-testbool"; + private const string MissingConfigKey = "gvfs.functionaltests-missing"; + + private string repoRoot; + + [OneTimeSetUp] + public void CreateRepo() + { + this.repoRoot = Path.Combine(Path.GetTempPath(), "GVFS.LibGit2ConfigTests_" + 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\""); + GitProcess.Invoke(this.repoRoot, $"config {StringConfigKey} {StringConfigValue}"); + GitProcess.Invoke(this.repoRoot, $"config {BoolConfigKey} true"); + } + + [OneTimeTearDown] + public void DeleteRepo() + { + if (this.repoRoot != null) + { + RepositoryHelpers.DeleteTestDirectory(this.repoRoot); + } + } + + [TestCase] + public void GetConfigStringReturnsValueFromLiveConfig() + { + // Before the snapshot fix this threw LibGit2Exception + // ("get_string called on a live config object") and callers silently + // fell back to their default value. + using (LibGit2Repo repo = new LibGit2Repo(NullTracer.Instance, this.repoRoot)) + { + repo.GetConfigString(StringConfigKey).ShouldEqual(StringConfigValue); + } + } + + [TestCase] + public void GetConfigStringReturnsNullWhenKeyMissing() + { + using (LibGit2Repo repo = new LibGit2Repo(NullTracer.Instance, this.repoRoot)) + { + repo.GetConfigString(MissingConfigKey).ShouldBeNull(); + } + } + + [TestCase] + public void GetConfigBoolReturnsValueFromLiveConfig() + { + using (LibGit2Repo repo = new LibGit2Repo(NullTracer.Instance, this.repoRoot)) + { + repo.GetConfigBool(BoolConfigKey).ShouldEqual(true); + } + } + } +}