diff --git a/GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerFixture/GitReadAndGitLockTests.cs b/GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerFixture/GitReadAndGitLockTests.cs index af26622b0b..0427576b70 100644 --- a/GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerFixture/GitReadAndGitLockTests.cs +++ b/GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerFixture/GitReadAndGitLockTests.cs @@ -7,6 +7,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.IO; +using System.Text; using System.Threading; namespace GVFS.FunctionalTests.Tests.EnlistmentPerFixture @@ -200,5 +201,502 @@ public void OrphanedGVFSLockIsCleanedUp() // an error about "Waiting for GVFS.FunctionalTests.LockHolder" statusWait.Errors.ShouldEqual(string.Empty); } + + [TestCase("stash", "list")] + [TestCase("stash.exe", "list")] + [TestCase("for-each-repo", "--config=gvfs.tests.alias-repositories status --porcelain")] + [TestCase("checkout", "--detach HEAD")] + [Order(9)] + public void BuiltInWithSameNameAliasStillAcquiresLock(string command, string arguments) + { + string aliasName = command.EndsWith(".exe", StringComparison.Ordinal) ? Path.GetFileNameWithoutExtension(command) : command; + string repositoryConfig = command == "for-each-repo" + ? "-c \"gvfs.tests.alias-repositories=" + this.Enlistment.RepoRoot + "\" " + : string.Empty; + ProcessResult result = this.RunWithLockHolder(repositoryConfig + "-c alias." + aliasName + "=version " + command + " " + arguments); + result.ExitCode.ShouldEqual(0); + result.Errors.ShouldContain(ExpectedStatusWaitingText); + } + + [TestCase("gvfs-lock-outer-alias", "status --porcelain")] + [TestCase("gvfs-lock-outer-alias", "!git status --porcelain")] + [TestCase("gvfs-lock-outer-alias", "gvfs-lock-inner-alias")] + [TestCase("MiXeD-lock-alias", "status --porcelain")] + [TestCase("git-prefixed-lock-alias", "status --porcelain")] + [Order(10)] + public void AliasInnerCommandStillAcquiresLock(string aliasName, string expansion) + { + string command = "-c alias.gvfs-lock-inner-alias=status " + + "-c \"alias." + aliasName + "=" + expansion + "\" " + aliasName; + ProcessResult result = this.RunWithLockHolder(command); + result.ExitCode.ShouldEqual(0); + result.Errors.ShouldContain(ExpectedStatusWaitingText); + } + + [TestCase] + [Order(11)] + public void ReadOnlyAliasDoesNotAcquireAnOuterLock() + { + ProcessResult result = this.RunWithLockHolder( + "-c \"alias.gvfs-read-only-alias=!git rev-parse --show-toplevel\" gvfs-read-only-alias", + shouldWait: false); + result.ExitCode.ShouldEqual(0); + result.Output.ShouldContain(this.Enlistment.RepoRoot.Replace('\\', '/')); + result.Errors.Contains(ExpectedStatusWaitingText).ShouldBeFalse(); + } + + [TestCase] + [Order(12)] + public void UnknownExternalCommandStillAcquiresLock() + { + ProcessResult result = this.RunWithLockHolder("-c help.autocorrect=never gvfs-unknown-lock-command"); + result.ExitCode.ShouldEqual(1); + result.Errors.ShouldContain(ExpectedStatusWaitingText); + } + + [TestCase("git-", false, false)] + [TestCase("GIT-", true, false)] + [TestCase("git-", false, true)] + [Order(13)] + public void ExternalWithSameNameAliasStillAcquiresLock(string prefix, bool uppercaseCommand, bool useExecPath) + { + const string CommandName = "gvfs-lock-external"; + string directory = Path.Combine(this.Enlistment.EnlistmentRoot, nameof(this.ExternalWithSameNameAliasStillAcquiresLock)); + Directory.CreateDirectory(directory); + string scriptPath = Path.Combine(directory, prefix + CommandName); + File.WriteAllText(scriptPath, "#!/bin/sh\necho external-script\nexec git status --porcelain\n"); + try + { + Dictionary environment = new Dictionary(); + if (useExecPath) + { + environment["GIT_EXEC_PATH"] = directory; + } + else + { + environment["PATH"] = directory + Path.PathSeparator + Environment.GetEnvironmentVariable("PATH"); + } + + string invocation = uppercaseCommand ? CommandName.ToUpperInvariant() : CommandName; + ProcessResult result = this.RunWithLockHolder("-c alias." + CommandName + "=version " + invocation, environment); + result.ExitCode.ShouldEqual(0); + result.Output.ShouldContain("external-script"); + result.Errors.ShouldContain(ExpectedStatusWaitingText); + } + finally + { + File.Delete(scriptPath); + Directory.Delete(directory); + } + } + + [TestCase] + [Order(14)] + public void ExternalExecutableWithSameNameAliasStillAcquiresLock() + { + const string CommandName = "gvfs-lock-executable"; + string directory = Path.Combine(this.Enlistment.EnlistmentRoot, nameof(this.ExternalExecutableWithSameNameAliasStillAcquiresLock)); + Directory.CreateDirectory(directory); + string executablePath = Path.Combine(directory, "GIT-" + CommandName + ".EXE"); + File.Copy(Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.System), "cmd.exe"), executablePath); + try + { + Dictionary environment = new Dictionary(); + environment["PATH"] = directory + Path.PathSeparator + Environment.GetEnvironmentVariable("PATH"); + ProcessResult result = this.RunWithLockHolder("-c alias." + CommandName + "=version " + CommandName + " /d /c echo external-command", environment); + result.ExitCode.ShouldEqual(0); + result.Output.ShouldContain("external-command"); + result.Errors.ShouldContain(ExpectedStatusWaitingText); + } + finally + { + File.Delete(executablePath); + Directory.Delete(directory); + } + } + + [TestCase] + [Order(15)] + public void DirectoryNamedAfterAnExternalDoesNotHideAnAlias() + { + const string CommandName = "gvfs-directory-alias"; + string directory = Path.Combine(this.Enlistment.EnlistmentRoot, nameof(this.DirectoryNamedAfterAnExternalDoesNotHideAnAlias)); + string commandDirectory = Path.Combine(directory, "git-" + CommandName); + Directory.CreateDirectory(commandDirectory); + try + { + Dictionary environment = new Dictionary(); + environment["PATH"] = directory + Path.PathSeparator + Environment.GetEnvironmentVariable("PATH"); + ProcessResult result = this.RunWithLockHolder( + "-c alias." + CommandName + "=version " + CommandName, + environment, + shouldWait: false); + result.ExitCode.ShouldEqual(0); + result.Output.ShouldContain("git version"); + } + finally + { + Directory.Delete(commandDirectory); + Directory.Delete(directory); + } + } + + [TestCase] + [Order(16)] + public void PostCommandReleasesOwnedLockAfterExternalDisappears() + { + const string CommandName = "gvfs-lock-race"; + string directory = Path.Combine(this.Enlistment.EnlistmentRoot, nameof(this.PostCommandReleasesOwnedLockAfterExternalDisappears)); + Directory.CreateDirectory(directory); + string executablePath = Path.Combine(directory, "git-" + CommandName); + File.WriteAllText(executablePath, "#!/bin/sh\nexit 99\n"); + string hooksExecutable = Path.Combine(Path.GetDirectoryName(Properties.Settings.Default.PathToGVFS), "GVFS.Hooks.exe"); + Dictionary environment = new Dictionary(); + environment["PATH"] = directory + Path.PathSeparator + Environment.GetEnvironmentVariable("PATH"); + environment["GIT_CONFIG_COUNT"] = "1"; + environment["GIT_CONFIG_KEY_0"] = "alias." + CommandName; + environment["GIT_CONFIG_VALUE_0"] = "checkout --detach HEAD"; + + // The test calls both hooks directly, so it does not depend on Git running post-command + // for git-. + // Keep the Git PID alive so orphan cleanup cannot hide a skipped release. + using (Process git = new Process()) + { + git.StartInfo = this.CreateGitStartInfo("hash-object --stdin", environment); + git.StartInfo.RedirectStandardInput = true; + bool started = false; + try + { + started = git.Start(); + started.ShouldBeTrue(); + string commandArguments = " git-" + CommandName + " --git-pid=" + git.Id; + ProcessStartInfo hook = this.CreateGitStartInfo("pre-command" + commandArguments, environment); + hook.FileName = hooksExecutable; + ProcessResult preCommand = ProcessHelper.Run(hook, timeoutMs: 30000); + preCommand.ExitCode.ShouldEqual(0, preCommand.Output + preCommand.Errors); + this.Enlistment.Status().ShouldContain("GVFS Lock: Held by git git-" + CommandName + " (PID:" + git.Id + ")"); + + File.Delete(executablePath); + hook.Arguments = "post-command" + commandArguments + " --exit_code=1"; + ProcessResult postCommand = ProcessHelper.Run(hook, timeoutMs: 30000); + postCommand.ExitCode.ShouldEqual(0, postCommand.Output + postCommand.Errors); + git.HasExited.ShouldBeFalse(); + this.Enlistment.Status().ShouldContain("GVFS Lock: Free"); + } + finally + { + if (started) + { + git.StandardInput.Close(); + if (!git.WaitForExit(10000)) + { + git.Kill(entireProcessTree: true); + git.WaitForExit(); + Assert.Fail("The parent Git process did not exit."); + } + } + + File.Delete(executablePath); + Directory.Delete(directory); + } + } + } + + [TestCase("\"C:\\invalid quoted path\"")] + [TestCase("C:\\invalid|path")] + [Order(17)] + public void InvalidPathEntryDoesNotPreventAGenuineAlias(string pathEntry) + { + Dictionary environment = new Dictionary(); + environment["PATH"] = pathEntry + Path.PathSeparator + Environment.GetEnvironmentVariable("PATH"); + ProcessResult result = this.RunWithLockHolder( + "-c alias.gvfs-invalid-path-alias=version gvfs-invalid-path-alias", + environment, + shouldWait: false); + result.ExitCode.ShouldEqual(0); + result.Output.ShouldContain("git version"); + } + + [TestCase] + [Order(18)] + public void SubsectionAliasInnerCommandStillAcquiresLock() + { + ProcessResult result = this.RunWithLockHolder( + "-c \"alias.gvfs-subsection-alias.command=status --porcelain\" gvfs-subsection-alias"); + result.ExitCode.ShouldEqual(0); + result.Errors.ShouldContain(ExpectedStatusWaitingText); + } + + [TestCase] + [Order(19)] + public void ConfigFileAliasInnerCommandStillAcquiresLock() + { + const string AliasName = "gvfs-config-file-alias"; + GitHelpers.CheckGitCommandAgainstGVFSRepo(this.Enlistment.RepoRoot, "config --local alias." + AliasName + " \"status --porcelain\""); + try + { + ProcessResult result = this.RunWithLockHolder(AliasName); + result.ExitCode.ShouldEqual(0); + result.Errors.ShouldContain(ExpectedStatusWaitingText); + } + finally + { + GitHelpers.CheckGitCommandAgainstGVFSRepo(this.Enlistment.RepoRoot, "config --local --unset alias." + AliasName); + } + } + + [TestCase] + [Order(20)] + public void SubsectionAliasWithCapitalLettersSkipsOnlyTheOuterLock() + { + ProcessResult result = this.RunWithLockHolder( + "-c \"alias.GVFS-Subsection-Alias.command=status --porcelain\" GVFS-Subsection-Alias"); + result.ExitCode.ShouldEqual(0); + result.Errors.ShouldContain(ExpectedStatusWaitingText); + } + + [TestCase] + [Order(21)] + public void UnreadableAliasConfigStopsTheCommandBeforeTheLock() + { + string hooksExecutable = Path.Combine(Path.GetDirectoryName(Properties.Settings.Default.PathToGVFS), "GVFS.Hooks.exe"); + string configPath = Path.Combine(this.Enlistment.EnlistmentRoot, nameof(this.UnreadableAliasConfigStopsTheCommandBeforeTheLock) + ".gitconfig"); + File.WriteAllText(configPath, "[alias]\n\tgvfs-unreadable = status\n"); + + // The hook reads aliases with the parent Git's executable, so keep a Git process alive. + using (Process git = new Process()) + { + git.StartInfo = this.CreateGitStartInfo("hash-object --stdin"); + git.StartInfo.RedirectStandardInput = true; + bool started = false; + try + { + started = git.Start(); + started.ShouldBeTrue(); + Dictionary environment = new Dictionary(); + environment["GIT_CONFIG_GLOBAL"] = configPath; + ProcessStartInfo hook = this.CreateGitStartInfo("pre-command git-gvfs-unreadable --git-pid=" + git.Id, environment); + hook.FileName = hooksExecutable; + + ProcessResult preCommand; + using (new FileStream(configPath, FileMode.Open, FileAccess.Read, FileShare.None)) + { + preCommand = ProcessHelper.Run(hook, timeoutMs: 30000); + } + + preCommand.ExitCode.ShouldEqual(1, preCommand.Output + preCommand.Errors); + preCommand.Output.ShouldContain( + "Unable to read Git aliases (exit code 128).", + "GVFS cannot tell if 'git gvfs-unreadable' runs an alias or a program, so it stopped the command."); + this.Enlistment.Status().ShouldContain("GVFS Lock: Free"); + } + finally + { + if (started) + { + git.StandardInput.Close(); + if (!git.WaitForExit(10000)) + { + git.Kill(entireProcessTree: true); + git.WaitForExit(); + } + } + + File.Delete(configPath); + } + } + } + + [TestCase("alias..gvfs-dot-prefix.foo", ".gvfs-dot-prefix.foo")] + [TestCase("alias..gvfs-dot-subsection.command", ".gvfs-dot-subsection")] + [Order(22)] + public void DotPrefixedSubsectionAliasInnerCommandStillAcquiresLock(string aliasKey, string aliasName) + { + ProcessResult result = this.RunWithLockHolder( + "-c \"" + aliasKey + "=status --porcelain\" " + aliasName); + result.ExitCode.ShouldEqual(0); + result.Errors.ShouldContain(ExpectedStatusWaitingText); + } + + [TestCase] + [Order(23)] + public void PostCommandForExternalIgnoresAnUnmountedEnlistment() + { + string hooksExecutable = Path.Combine(Path.GetDirectoryName(Properties.Settings.Default.PathToGVFS), "GVFS.Hooks.exe"); + string directory = Path.Combine(Path.GetTempPath(), nameof(this.PostCommandForExternalIgnoresAnUnmountedEnlistment) + "_" + Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(Path.Combine(directory, ".gvfs")); + try + { + ProcessStartInfo hook = this.CreateGitStartInfo("post-command git-gvfs-unmounted --exit_code=0"); + hook.FileName = hooksExecutable; + hook.WorkingDirectory = directory; + + // A false GIT_OPTIONAL_LOCKS makes post-command status skip the lock request. + hook.Environment.Remove("GIT_OPTIONAL_LOCKS"); + ProcessResult external = ProcessHelper.Run(hook, timeoutMs: 30000); + external.ExitCode.ShouldEqual(0, external.Output + external.Errors); + (external.Output + external.Errors).ShouldEqual(string.Empty); + + hook.Arguments = "post-command status --exit_code=0"; + ProcessResult builtIn = ProcessHelper.Run(hook, timeoutMs: 30000); + builtIn.ExitCode.ShouldEqual(1, builtIn.Output + builtIn.Errors); + builtIn.Output.ShouldContain("The repo does not appear to be mounted."); + } + finally + { + Directory.Delete(directory, recursive: true); + } + } + + private ProcessStartInfo CreateGitStartInfo(string command, Dictionary environment = null) + { + ProcessStartInfo startInfo = new ProcessStartInfo(Properties.Settings.Default.PathToGit, command); + startInfo.WorkingDirectory = this.Enlistment.RepoRoot; + startInfo.UseShellExecute = false; + startInfo.RedirectStandardOutput = true; + startInfo.RedirectStandardError = true; + startInfo.Environment["GIT_TERMINAL_PROMPT"] = "0"; + if (environment != null) + { + foreach (KeyValuePair entry in environment) + { + startInfo.Environment[entry.Key] = entry.Value; + } + } + + return startInfo; + } + + private ProcessResult RunWithLockHolder(string command, Dictionary environment = null, bool shouldWait = true) + { + using (Process holder = new Process()) + using (Process git = new Process()) + using (ManualResetEventSlim waiting = new ManualResetEventSlim(false)) + { + holder.StartInfo = new ProcessStartInfo(GitHelpers.LockHolderCommandPath) + { + WorkingDirectory = this.Enlistment.RepoRoot, + UseShellExecute = false, + RedirectStandardInput = true, + RedirectStandardOutput = true, + RedirectStandardError = true, + }; + holder.Start().ShouldBeTrue(); + bool gitStarted = false; + bool completed = false; + bool released = false; + StringBuilder output = new StringBuilder(); + StringBuilder errors = new StringBuilder(); + Thread errorReader = null; + try + { + this.Enlistment.WaitForLock("GVFS.FunctionalTests.LockHolder (PID:" + holder.Id + ")") + .ShouldBeTrue("The expected lock-holder process did not acquire the lock."); + git.StartInfo = this.CreateGitStartInfo(command, environment); + git.OutputDataReceived += (sender, args) => + { + if (args.Data != null) + { + output.AppendLine(args.Data); + } + }; + errorReader = new Thread(() => + { + int character; + while ((character = git.StandardError.Read()) != -1) + { + errors.Append((char)character); + if (!waiting.IsSet && errors.ToString().Contains(ExpectedStatusWaitingText)) + { + waiting.Set(); + } + } + }); + gitStarted = git.Start(); + gitStarted.ShouldBeTrue(); + git.BeginOutputReadLine(); + errorReader.Start(); + + if (shouldWait) + { + waiting.Wait(AcquireGVFSLockTimeout).ShouldBeTrue("Git did not wait for the lock holder."); + this.ReleaseLockHolder(holder); + released = true; + } + + git.WaitForExit(30000).ShouldBeTrue("Git did not finish within the test timeout."); + git.WaitForExit(); + errorReader.Join(10000).ShouldBeTrue("Git stderr did not close."); + if (!shouldWait) + { + holder.HasExited.ShouldBeFalse(); + this.Enlistment.Status().ShouldContain("GVFS.FunctionalTests.LockHolder (PID:" + holder.Id + ")"); + } + + completed = true; + return new ProcessResult(output.ToString(), errors.ToString(), git.ExitCode); + } + finally + { + // Clean up Git and its reader even if the holder release fails. After a test + // failure, report that failure, not a cleanup failure. + try + { + if (!released) + { + this.ReleaseLockHolder(holder, verify: completed); + } + } + finally + { + if (gitStarted && !git.WaitForExit(10000)) + { + git.Kill(entireProcessTree: true); + git.WaitForExit(); + } + + if (errorReader != null && (errorReader.ThreadState & System.Threading.ThreadState.Unstarted) == 0) + { + bool readerClosed = errorReader.Join(10000); + if (completed) + { + readerClosed.ShouldBeTrue("Git stderr did not close during cleanup."); + } + } + } + } + } + } + + private void ReleaseLockHolder(Process holder, bool verify = true) + { + if (!holder.HasExited) + { + try + { + holder.StandardInput.WriteLine("done"); + holder.StandardInput.Close(); + } + catch (IOException) when (!verify || holder.HasExited) + { + // The holder already exited, or cleanup after a failed test is best effort. + } + + if (!holder.WaitForExit(10000)) + { + holder.Kill(entireProcessTree: true); + holder.WaitForExit(); + if (verify) + { + Assert.Fail("The lock-holder process did not exit."); + } + } + } + + if (verify) + { + holder.ExitCode.ShouldEqual(0, holder.StandardError.ReadToEnd()); + } + } } } diff --git a/GVFS/GVFS.Hooks/GitCommandLock.cs b/GVFS/GVFS.Hooks/GitCommandLock.cs new file mode 100644 index 0000000000..ecc32dced7 --- /dev/null +++ b/GVFS/GVFS.Hooks/GitCommandLock.cs @@ -0,0 +1,90 @@ +using GVFS.Common.NamedPipes; +using System; + +namespace GVFS.Hooks +{ + internal static class GitCommandLock + { + private const string CheckMount = "Run 'gvfs status' to check the mount."; + + /// + /// Returns true if Git runs "status" without locks, so the hook only checks that the GVFS + /// lock is free. "--no-optional-locks" sets GIT_OPTIONAL_LOCKS to a false value. A raw + /// "git-status" is an external program, so it never qualifies. + /// + public static bool IsAvailabilityOnly(string[] args, Func getEnvironmentVariable) + { + return string.Equals(args[1], "status", StringComparison.Ordinal) && + (Array.Exists(args, arg => arg.Equals("--no-lock-index", StringComparison.OrdinalIgnoreCase)) || + IsFalseValue(getEnvironmentVariable("GIT_OPTIONAL_LOCKS"))); + } + + public static bool TryCheckOwnership( + int pid, + bool isElevated, + string fullCommand, + Func sendRequest, + out bool ownsLock, + out string error) + { + ownsLock = false; + error = null; + NamedPipeMessages.LockRequest request = new NamedPipeMessages.LockRequest( + pid, isElevated, checkAvailabilityOnly: true, fullCommand, gitCommandSessionId: string.Empty); + NamedPipeMessages.Message message = sendRequest(request.CreateMessage(NamedPipeMessages.AcquireLock.AcquireRequest)); + if (message == null) + { + error = "GVFS returned no lock-ownership response. " + CheckMount; + return false; + } + + NamedPipeMessages.AcquireLock.Response response; + try + { + response = new NamedPipeMessages.AcquireLock.Response(message); + } + catch (Exception ex) when (ex is InvalidOperationException || ex is ArgumentException) + { + error = "GVFS returned an invalid lock-ownership response: " + ex.Message + " " + CheckMount; + return false; + } + + switch (response.Result) + { + case NamedPipeMessages.AcquireLock.AvailableResult: + case NamedPipeMessages.AcquireLock.DenyGVFSResult: + return true; + + case NamedPipeMessages.AcquireLock.DenyGitResult: + if (response.ResponseData == null || response.ResponseData.PID <= 0) + { + error = "GVFS returned a lock owner without a valid process ID. " + CheckMount; + return false; + } + + ownsLock = response.ResponseData.PID == pid; + return true; + + case NamedPipeMessages.AcquireLock.MountNotReadyResult: + error = "GVFS has not finished initializing. " + CheckMount; + return false; + + case NamedPipeMessages.AcquireLock.UnmountInProgressResult: + error = "GVFS is unmounting. " + CheckMount; + return false; + + default: + error = "GVFS returned an unexpected lock-ownership response: " + response.Result + ". " + CheckMount; + return false; + } + } + + private static bool IsFalseValue(string value) + { + return string.Equals(value, "false", StringComparison.OrdinalIgnoreCase) || + string.Equals(value, "no", StringComparison.OrdinalIgnoreCase) || + string.Equals(value, "off", StringComparison.OrdinalIgnoreCase) || + string.Equals(value, "0", StringComparison.OrdinalIgnoreCase); + } + } +} diff --git a/GVFS/GVFS.Hooks/GitCommandResolver.cs b/GVFS/GVFS.Hooks/GitCommandResolver.cs new file mode 100644 index 0000000000..09b0cd43f3 --- /dev/null +++ b/GVFS/GVFS.Hooks/GitCommandResolver.cs @@ -0,0 +1,261 @@ +using GVFS.Common; +using System; +using System.ComponentModel; +using System.Diagnostics; +using System.IO; +using System.Runtime.InteropServices; +using System.Text; + +namespace GVFS.Hooks +{ + internal static class GitCommandResolver + { + public const uint InvalidFileAttributes = 0xFFFFFFFF; + public const string UnableToAccess = "unable to access '"; + + private const uint FileAttributeDirectory = 0x10; + + // Git builds each PATH candidate in a char[MAX_PATH] buffer. + private const int GitMaxPath = 260; + + /// + /// Determines whether Git will expand as an alias. Returns + /// false when GVFS cannot tell: PATH is empty, or the alias config keys cannot be read or + /// are incomplete. The caller must not guess, because an outer lock on a genuine alias + /// blocks the commands of that alias. + /// + public static bool TryIsAlias( + string hookCommand, + Func readAliasKeys, + string searchPath, + Func getFileAttributes, + out bool isAlias, + out string error) + { + isAlias = false; + error = null; + + // Git supplies canonical, unprefixed names for built-ins, and git- for external attempts. + if (!hookCommand.StartsWith("git-", StringComparison.Ordinal) || + hookCommand.IndexOfAny(new[] { '\\', '/' }) >= 0) + { + return true; + } + + string command = hookCommand.Substring("git-".Length); + if (string.IsNullOrEmpty(searchPath)) + { + // Git adds its exec-path to PATH before it runs the hook, so this is a defensive check. + error = $"Unable to resolve Git command '{command}': the inherited PATH is empty."; + return false; + } + + if (!TryFindProgramOnPath(hookCommand, searchPath, getFileAttributes, out bool programFound, out error)) + { + return false; + } + + if (programFound) + { + return true; + } + + ProcessResult result; + try + { + result = readAliasKeys(); + } + catch (Exception ex) when (ex is Win32Exception || ex is InvalidOperationException || + ex is ArgumentException || ex is IOException || ex is UnauthorizedAccessException) + { + error = $"Unable to read Git aliases: {FormatError(ex)}"; + return false; + } + + // git config exits with 1 when no key matches. It exits with 128 when it cannot read a + // config file, after warnings that name the file. + bool noAliases = result.ExitCode == 1 && string.IsNullOrEmpty(result.Output) && + (result.Errors ?? string.Empty).IndexOf(UnableToAccess, StringComparison.Ordinal) < 0; + if (result.ExitCode != 0 && !noAliases) + { + error = $"Unable to read Git aliases (exit code {result.ExitCode}). {result.Errors}".TrimEnd(); + return false; + } + + foreach (string key in (result.Output ?? string.Empty).Split('\n')) + { + if (IsAliasKeyFor(key, command)) + { + isAlias = true; + return true; + } + } + + return true; + } + + public static string GetProcessPath(int processId) + { + try + { + using (Process process = Process.GetProcessById(processId)) + { + string path = process.MainModule?.FileName; + if (!string.IsNullOrEmpty(path)) + { + return path; + } + } + } + catch (Exception ex) when (ex is ArgumentException || ex is Win32Exception || ex is InvalidOperationException) + { + throw new InvalidOperationException($"Unable to find the executable of the parent Git process (PID {processId}): {ex.Message}", ex); + } + + throw new InvalidOperationException($"Unable to find the executable of the parent Git process (PID {processId})."); + } + + public static ProcessStartInfo CreateAliasKeyQuery(string gitPath) + { + ProcessStartInfo processInfo = new ProcessStartInfo(gitPath); + processInfo.UseShellExecute = false; + + // The query reads no input. Do not give it the hook's standard input. + processInfo.RedirectStandardInput = true; + processInfo.RedirectStandardOutput = true; + processInfo.RedirectStandardError = true; + + // Git writes config names in UTF-8, whatever the console code page is. + processInfo.StandardOutputEncoding = Encoding.UTF8; + processInfo.StandardErrorEncoding = Encoding.UTF8; + processInfo.ArgumentList.Add("config"); + processInfo.ArgumentList.Add("--name-only"); + processInfo.ArgumentList.Add("--get-regexp"); + processInfo.ArgumentList.Add(@"^alias\."); + processInfo.Environment["COMMAND_HOOK_LOCK"] = "true"; + return processInfo; + } + + public static uint GetFileAttributes(string path) + { + return NativeMethods.GetFileAttributesW(path); + } + + /// + /// Mirrors config_alias_cb in Git's alias.c. Like parse_config_key, it splits the text + /// after "alias." at the last dot into a subsection and a variable name. If the + /// subsection is empty, the variable name is the alias name, matched for any ASCII case. + /// If the variable name is "command", the alias name is the subsection, which must match + /// exactly. Otherwise the alias name is all of the text after "alias.", matched for any + /// ASCII case. Thus "alias..review.foo" names ".review.foo". + /// + private static bool IsAliasKeyFor(string key, string command) + { + const string Section = "alias."; + if (!key.StartsWith(Section, StringComparison.Ordinal)) + { + return false; + } + + string name = key.Substring(Section.Length); + int lastDot = name.LastIndexOf('.'); + if (lastDot >= 0) + { + string subsection = name.Substring(0, lastDot); + string variable = name.Substring(lastDot + 1); + if (subsection.Length == 0) + { + name = variable; + } + else if (string.Equals(variable, "command", StringComparison.Ordinal)) + { + return string.Equals(subsection, command, StringComparison.Ordinal); + } + } + + return name.Length > 0 && EqualsIgnoringAsciiCase(name, command); + } + + private static bool EqualsIgnoringAsciiCase(string left, string right) + { + if (left.Length != right.Length) + { + return false; + } + + for (int i = 0; i < left.Length; ++i) + { + if (ToAsciiLower(left[i]) != ToAsciiLower(right[i])) + { + return false; + } + } + + return true; + } + + private static char ToAsciiLower(char c) + { + return c >= 'A' && c <= 'Z' ? (char)(c + ('a' - 'A')) : c; + } + + /// + /// Mirrors path_lookup and lookup_prog in Git for Windows (compat/mingw.c). Git checks + /// "<entry>\<command>.exe" for each nonempty PATH entry, then the same path without + /// ".exe". It uses _waccess, which fails for any file that it cannot access. Git builds + /// the path in a char[MAX_PATH] buffer and cuts a longer path. Returns false for a path + /// that Git cuts, because the conversion of a cut path is not reproduced here. + /// + private static bool TryFindProgramOnPath( + string hookCommand, + string searchPath, + Func getFileAttributes, + out bool found, + out string error) + { + found = false; + error = null; + bool isExe = hookCommand.EndsWith(".exe", StringComparison.OrdinalIgnoreCase); + foreach (string directory in searchPath.Split(';')) + { + if (directory.Length == 0) + { + continue; + } + + string path = directory + "\\" + hookCommand + ".exe"; + if (Encoding.UTF8.GetByteCount(path) >= GitMaxPath) + { + error = $"Unable to resolve Git command '{hookCommand.Substring("git-".Length)}': the PATH entry '{directory}' is too long for Git's path buffer."; + return false; + } + + if (!isExe && getFileAttributes(path) != InvalidFileAttributes) + { + found = true; + return true; + } + + uint attributes = getFileAttributes(path.Substring(0, path.Length - ".exe".Length)); + if (attributes != InvalidFileAttributes && (attributes & FileAttributeDirectory) == 0) + { + found = true; + return true; + } + } + + return true; + } + + private static string FormatError(Exception error) + { + return $"{error.GetType().Name} (0x{error.HResult:X8}): {error.Message}"; + } + + private static class NativeMethods + { + [DllImport("kernel32.dll", CharSet = CharSet.Unicode, SetLastError = true)] + public static extern uint GetFileAttributesW(string fileName); + } + } +} diff --git a/GVFS/GVFS.Hooks/KnownGitCommands.cs b/GVFS/GVFS.Hooks/KnownGitCommands.cs deleted file mode 100644 index a392eace3c..0000000000 --- a/GVFS/GVFS.Hooks/KnownGitCommands.cs +++ /dev/null @@ -1,150 +0,0 @@ -using System.Collections.Generic; - -namespace GVFS.Hooks -{ - internal static class KnownGitCommands - { - private static HashSet knownCommands = new HashSet() - { - "add", - "am", - "annotate", - "apply", - "archive", - "bisect--helper", - "blame", - "branch", - "bundle", - "cat-file", - "check-attr", - "check-ignore", - "check-mailmap", - "check-ref-format", - "checkout", - "checkout-index", - "cherry", - "cherry-pick", - "clean", - "clone", - "column", - "commit", - "commit-tree", - "config", - "count-objects", - "credential", - "describe", - "diff", - "diff-files", - "diff-index", - "diff-tree", - "fast-export", - "fetch", - "fetch-pack", - "fmt-merge-msg", - "for-each-ref", - "format-patch", - "fsck", - "fsck-objects", - "gc", - "get-tar-commit-id", - "grep", - "hash-object", - "help", - "index-pack", - "init", - "init-db", - "interpret-trailers", - "log", - "ls-files", - "ls-remote", - "ls-tree", - "mailinfo", - "mailsplit", - "merge", - "merge-base", - "merge-file", - "merge-index", - "merge-ours", - "merge-recursive", - "merge-recursive-ours", - "merge-recursive-theirs", - "merge-subtree", - "merge-tree", - "mktag", - "mktree", - "mv", - "name-rev", - "notes", - "pack-objects", - "pack-redundant", - "pack-refs", - "patch-id", - "pickaxe", - "prune", - "prune-packed", - "pull", - "push", - "read-tree", - "rebase", - "rebase--helper", - "receive-pack", - "reflog", - "remote", - "remote-ext", - "remote-fd", - "repack", - "replace", - "rerere", - "reset", - "restore", - "rev-list", - "rev-parse", - "revert", - "rm", - "send-pack", - "shortlog", - "show", - "show-branch", - "show-ref", - "stage", - "status", - "stripspace", - "switch", - "symbolic-ref", - "tag", - "unpack-file", - "unpack-objects", - "update-index", - "update-ref", - "update-server-info", - "upload-archive", - "upload-archive--writer", - "var", - "verify-commit", - "verify-pack", - "verify-tag", - "version", - "whatchanged", - "worktree", - "write-tree", - - // Externals - "bisect", - "filter-branch", - "gui", - "merge-octopus", - "merge-one-file", - "merge-resolve", - "mergetool", - "parse-remote", - "quiltimport", - "rebase", - "submodule", - }; - - public static bool Contains(string gitCommand) - { - return knownCommands.Contains(gitCommand); - } - } -} diff --git a/GVFS/GVFS.Hooks/Program.CommandLocking.cs b/GVFS/GVFS.Hooks/Program.CommandLocking.cs index d535373325..b016a7d9ea 100644 --- a/GVFS/GVFS.Hooks/Program.CommandLocking.cs +++ b/GVFS/GVFS.Hooks/Program.CommandLocking.cs @@ -1,6 +1,5 @@ using GVFS.Common; -using GVFS.Common.Git; -using GVFS.Common.Tracing; +using System; using System.Linq; namespace GVFS.Hooks @@ -15,10 +14,41 @@ public partial class Program // Ref and object writes alone do not require this lock. Index refreshes do. // Keep arbitrary Git dispatchers locked because nested command hooks can be suppressed. // Excluded readers can observe concurrent working-tree changes. They do not receive snapshot isolation. - internal static bool ShouldLock(string[] args) + internal static bool TryShouldLock( + string[] args, + Func readGitAliasKeys, + string searchPath, + Func getFileAttributes, + out bool shouldLock, + out string error) { + error = null; string gitCommand = GetGitCommand(args); + // Git reports a built-in by its name. A git- prefix means that Git found no built-in + // and will run a program or expand an alias, so the read-only list does not apply. + // git-submodule and git-web--browse are programs that Git ships in its exec-path, + // which Git puts first on PATH. They keep their exemptions. + if (args[1].StartsWith("git-", StringComparison.Ordinal) && + gitCommand != "submodule" && + gitCommand != "web--browse") + { + shouldLock = true; + if (!GitCommandResolver.TryIsAlias(args[1], readGitAliasKeys, searchPath, getFileAttributes, out bool isAlias, out error)) + { + return false; + } + + shouldLock = !isAlias; + return true; + } + + shouldLock = ShouldLockBuiltIn(gitCommand, args); + return true; + } + + private static bool ShouldLockBuiltIn(string gitCommand, string[] args) + { switch (gitCommand) { // Keep these alphabetically sorted @@ -94,12 +124,6 @@ internal static bool ShouldLock(string[] args) return false; } - if (!KnownGitCommands.Contains(gitCommand) && - IsAlias(gitCommand)) - { - return false; - } - return true; } @@ -113,21 +137,5 @@ private static string GetGitCommand(string[] args) return command; } - - private static bool IsAlias(string command) - { - string aliasKey = "alias." + command; - ProcessResult result = GitConfigReadRetry.InvokeResult( - aliasKey, - () => ProcessHelper.Run("git", "config --get " + aliasKey), - (ProcessResult processResult) => - processResult.ExitCode != 0 && - GitConfigReadRetry.IsTransientFileAccessError(processResult.Errors, lineMustContain: "unable to access"), - NullTracer.Instance, - GitConfigReadRetry.Delay, - ConfigReadMaxAttempts); - - return !string.IsNullOrEmpty(result.Output); - } } } diff --git a/GVFS/GVFS.Hooks/Program.cs b/GVFS/GVFS.Hooks/Program.cs index 3093ab15d9..600ee823a7 100644 --- a/GVFS/GVFS.Hooks/Program.cs +++ b/GVFS/GVFS.Hooks/Program.cs @@ -253,22 +253,74 @@ private static void RunLockRequest(string[] args, bool unattended, LockRequestDe { try { - if (ShouldLock(args)) + Lazy parentPid = new Lazy(() => GetParentPid(args)); + bool checkOwnership = GetHookType(args) == PostCommandHook && + args[1].StartsWith("git-", StringComparison.Ordinal); + bool shouldLock; + if (checkOwnership) + { + shouldLock = true; + } + else if (!TryShouldLock( + args, + () => ReadGitAliasKeys(parentPid.Value), + Environment.GetEnvironmentVariable("PATH"), + GitCommandResolver.GetFileAttributes, + out shouldLock, + out string resolveError)) + { + ExitWithError( + resolveError, + $"GVFS cannot tell if 'git {args[1].Substring("git-".Length)}' runs an alias or a program, so it stopped the command.", + "Run the command again. If this error continues, correct the problem that the first line describes."); + } + + if (shouldLock) { using (NamedPipeClient pipeClient = new NamedPipeClient(enlistmentPipename)) { if (!pipeClient.Connect()) { + if (checkOwnership) + { + // Locks exist only in the mount process, so there is nothing to release. + return; + } + ExitWithError("The repo does not appear to be mounted. Use 'gvfs status' to check."); } - int pid = GetParentPid(args); + int pid = parentPid.Value; if (pid == Program.InvalidProcessId || !GVFSHooksPlatform.IsProcessActive(pid)) { ExitWithError("GVFS.Hooks: Unable to find parent git.exe process " + "(PID: " + pid + ")."); } + if (checkOwnership) + { + // Executable and alias config can change after acquisition. + if (!GitCommandLock.TryCheckOwnership( + pid, + GVFSHooksPlatform.IsElevated(), + GenerateFullCommand(args), + request => + { + pipeClient.SendRequest(request); + return pipeClient.ReadResponse(); + }, + out bool ownsLock, + out string error)) + { + ExitWithError(error); + } + + if (!ownsLock) + { + return; + } + } + requestToRun(unattended, args, pid, pipeClient); } } @@ -380,14 +432,7 @@ private static bool CheckGVFSLockAvailabilityOnly(string[] args) { try { - // Don't acquire the GVFS lock if the git command is not acquiring locks. - // This enables tools to run status commands without to the index and - // blocking other commands from running. The git argument - // "--no-optional-locks" results in a 'negative' - // value GIT_OPTIONAL_LOCKS environment variable. - return GetGitCommand(args).Equals("status", StringComparison.OrdinalIgnoreCase) && - (args.Any(arg => arg.Equals("--no-lock-index", StringComparison.OrdinalIgnoreCase)) || - IsGitEnvVarDisabled("GIT_OPTIONAL_LOCKS")); + return GitCommandLock.IsAvailabilityOnly(args, Environment.GetEnvironmentVariable); } catch (Exception e) { @@ -410,26 +455,22 @@ private static string BuildUpdatePlaceholderFailureMessage(List fileList return message; } - private static bool IsGitEnvVarDisabled(string envVar) + private static string GetHookType(string[] args) { - string envVarValue = Environment.GetEnvironmentVariable(envVar); - if (!string.IsNullOrEmpty(envVarValue)) - { - if (string.Equals(envVarValue, "false", StringComparison.OrdinalIgnoreCase) || - string.Equals(envVarValue, "no", StringComparison.OrdinalIgnoreCase) || - string.Equals(envVarValue, "off", StringComparison.OrdinalIgnoreCase) || - string.Equals(envVarValue, "0", StringComparison.OrdinalIgnoreCase)) - { - return true; - } - } - - return false; + return args[0].ToLowerInvariant(); } - private static string GetHookType(string[] args) + private static ProcessResult ReadGitAliasKeys(int parentPid) { - return args[0].ToLowerInvariant(); + string gitPath = GitCommandResolver.GetProcessPath(parentPid); + return GitConfigReadRetry.InvokeResult( + "alias keys", + () => ProcessHelper.Run(GitCommandResolver.CreateAliasKeyQuery(gitPath)), + (ProcessResult result) => + GitConfigReadRetry.IsTransientFileAccessError(result.Errors, lineMustContain: GitCommandResolver.UnableToAccess), + NullTracer.Instance, + GitConfigReadRetry.Delay, + ConfigReadMaxAttempts); } /// diff --git a/GVFS/GVFS.UnitTests/GVFS.UnitTests.csproj b/GVFS/GVFS.UnitTests/GVFS.UnitTests.csproj index 1d88885cf4..906a6955e5 100644 --- a/GVFS/GVFS.UnitTests/GVFS.UnitTests.csproj +++ b/GVFS/GVFS.UnitTests/GVFS.UnitTests.csproj @@ -27,8 +27,11 @@ - - Hooks\KnownGitCommands.cs + + Hooks\GitCommandLock.cs + + + Hooks\GitCommandResolver.cs Hooks\Program.CommandLocking.cs diff --git a/GVFS/GVFS.UnitTests/Hooks/CommandLockingTests.cs b/GVFS/GVFS.UnitTests/Hooks/CommandLockingTests.cs index 6b198795b3..433ee028e6 100644 --- a/GVFS/GVFS.UnitTests/Hooks/CommandLockingTests.cs +++ b/GVFS/GVFS.UnitTests/Hooks/CommandLockingTests.cs @@ -1,5 +1,7 @@ +using GVFS.Common; using GVFS.Tests.Should; using NUnit.Framework; +using System.Collections.Generic; using System.Linq; namespace GVFS.UnitTests.Hooks @@ -9,7 +11,6 @@ public class CommandLockingTests { [TestCase("annotate")] [TestCase("annotate --contents mock-file HEAD")] - [TestCase("git-annotate")] [TestCase("blame")] [TestCase("branch")] [TestCase("cat-file")] @@ -57,6 +58,7 @@ public class CommandLockingTests [TestCase("show-branch --reflog")] [TestCase("show-ref")] [TestCase("submodule")] + [TestCase("git-submodule")] [TestCase("symbolic-ref")] [TestCase("tag")] [TestCase("unpack-objects")] @@ -68,11 +70,11 @@ public class CommandLockingTests [TestCase("verify-tag --format=%(refname) mock-tag")] [TestCase("version")] [TestCase("web--browse")] + [TestCase("git-web--browse")] [TestCase("worktree list")] [TestCase("worktree list --porcelain")] [TestCase("worktree list --porcelain -z")] [TestCase("worktree list --verbose --expire=now")] - [TestCase("git-worktree list")] [TestCase("WORKTREE LIST")] [TestCase("worktree --git-pid=123 list")] public void CommandsWithoutLockPreserveTheirPolicy(string command) @@ -115,16 +117,90 @@ public void CommandsWithoutLockPreserveTheirPolicy(string command) [TestCase("worktree prune")] [TestCase("worktree repair")] [TestCase("worktree unknown list")] - [TestCase("git-worktree add list")] [TestCase("write-tree")] public void CommandsWithLockPreserveTheirPolicy(string command) { AssertLockPolicy(command, shouldLock: true); } + [TestCase("git-annotate")] + [TestCase("git-DIFF")] + [TestCase("git-LOG")] + [TestCase("git-worktree list")] + [TestCase("git-worktree add list")] + [TestCase("git-reset --soft")] + public void ExternalsNamedAfterBuiltInsAcquireLock(string command) + { + string hookCommand = command.Split(' ')[0]; + string[] args = CreatePreCommandArgs(command); + List candidates = new List(); + + GVFS.Hooks.Program.TryShouldLock( + args, + () => throw new AssertionException("A program must not query aliases."), + @"mock:\tools", + path => + { + candidates.Add(path); + return 0x80; + }, + out bool shouldLock, + out string error).ShouldBeTrue(); + + shouldLock.ShouldBeTrue(string.Join(" ", args)); + error.ShouldBeNull(); + candidates.ShouldMatchInOrder(@"mock:\tools\" + hookCommand + ".exe"); + } + + [TestCase("git-LOG")] + [TestCase("git-co")] + public void AliasesWithoutAProgramSkipTheOuterLock(string hookCommand) + { + int queries = 0; + GVFS.Hooks.Program.TryShouldLock( + CreatePreCommandArgs(hookCommand), + () => + { + ++queries; + return new ProcessResult("alias." + hookCommand.Substring("git-".Length).ToLowerInvariant() + "\n", string.Empty, 0); + }, + @"mock:\tools", + path => GVFS.Hooks.GitCommandResolver.InvalidFileAttributes, + out bool shouldLock, + out string error).ShouldBeTrue(); + + queries.ShouldEqual(1); + shouldLock.ShouldBeFalse(); + error.ShouldBeNull(); + } + + [TestCase] + public void AliasQueryFailuresAreReturnedToTheHook() + { + GVFS.Hooks.Program.TryShouldLock( + CreatePreCommandArgs("git-MiXeD"), + () => new ProcessResult(string.Empty, "fatal: bad config", 128), + @"mock:\tools", + path => GVFS.Hooks.GitCommandResolver.InvalidFileAttributes, + out bool shouldLock, + out string error).ShouldBeFalse(); + + shouldLock.ShouldBeTrue(); + error.ShouldContain("exit code 128", "fatal: bad config"); + } + + private static string[] CreatePreCommandArgs(string command) + { + return new[] { "pre-command" }.Concat(command.Split(' ')).Concat(new[] { "--git-pid=456" }).ToArray(); + } + private static void AssertLockPolicy(string command, bool shouldLock) { - foreach (string hook in new[] { "pre-command", "post-command" }) + // The hook sends post-command for git- to the lock-ownership check, not to this policy. + string[] hooks = command.StartsWith("git-", System.StringComparison.Ordinal) + ? new[] { "pre-command" } + : new[] { "pre-command", "post-command" }; + foreach (string hook in hooks) { string[] args = new[] { hook } .Concat(command.Split(' ')) @@ -133,7 +209,15 @@ private static void AssertLockPolicy(string command, bool shouldLock) : new[] { "--git-pid=456" }) .ToArray(); - GVFS.Hooks.Program.ShouldLock(args).ShouldEqual(shouldLock, string.Join(" ", args)); + GVFS.Hooks.Program.TryShouldLock( + args, + () => throw new AssertionException("Unexpected Git alias list for " + string.Join(" ", args)), + @"mock:\tools", + path => throw new AssertionException("Unexpected PATH search for " + path), + out bool actual, + out string error).ShouldBeTrue(error); + + actual.ShouldEqual(shouldLock, string.Join(" ", args)); } } } diff --git a/GVFS/GVFS.UnitTests/Hooks/GitCommandLockTests.cs b/GVFS/GVFS.UnitTests/Hooks/GitCommandLockTests.cs new file mode 100644 index 0000000000..88ed7a49a3 --- /dev/null +++ b/GVFS/GVFS.UnitTests/Hooks/GitCommandLockTests.cs @@ -0,0 +1,115 @@ +using GVFS.Common.NamedPipes; +using GVFS.Hooks; +using GVFS.Tests.Should; +using GVFS.UnitTests.Category; +using NUnit.Framework; + +namespace GVFS.UnitTests.Hooks +{ + [TestFixture] + public class GitCommandLockTests + { + [TestCase(new[] { "post-command", "status", "--no-lock-index" }, null, true)] + [TestCase(new[] { "post-command", "status", "--NO-LOCK-INDEX" }, null, true)] + [TestCase(new[] { "pre-command", "status" }, "0", true)] + [TestCase(new[] { "pre-command", "status" }, "Off", true)] + [TestCase(new[] { "pre-command", "status" }, "1", false)] + [TestCase(new[] { "pre-command", "status" }, null, false)] + [TestCase(new[] { "post-command", "git-status", "--no-lock-index" }, "0", false)] + [TestCase(new[] { "post-command", "STATUS", "--no-lock-index" }, "0", false)] + [TestCase(new[] { "post-command", "log", "--no-lock-index" }, "0", false)] + public void OnlyCanonicalStatusWithoutLocksIsAvailabilityOnly(string[] args, string optionalLocks, bool expected) + { + GitCommandLock.IsAvailabilityOnly( + args, + name => name == "GIT_OPTIONAL_LOCKS" ? optionalLocks : null).ShouldEqual(expected); + } + + [TestCase(123, true)] + [TestCase(456, false)] + public void OwnershipComparesTheTypedPidWithoutAcquiring(int ownerPid, bool expectedOwner) + { + int requests = 0; + GitCommandLock.TryCheckOwnership( + 123, + isElevated: false, + "git race", + request => + { + ++requests; + request.Header.ShouldEqual("AcquireLock"); + NamedPipeMessages.LockData data = new NamedPipeMessages.LockRequest(request.Body).RequestData; + data.PID.ShouldEqual(123); + data.CheckAvailabilityOnly.ShouldBeTrue(); + data.ParsedCommand.ShouldEqual("git race"); + return new NamedPipeMessages.AcquireLock.Response( + "LockDeniedGit", + new NamedPipeMessages.LockData(ownerPid, false, false, "git original", string.Empty)).CreateMessage(); + }, + out bool ownsLock, + out string error).ShouldBeTrue(); + + requests.ShouldEqual(1); + ownsLock.ShouldEqual(expectedOwner); + error.ShouldBeNull(); + } + + [TestCase("LockAvailable")] + [TestCase("LockDeniedGVFS")] + public void UnownedLocksNeedNoRelease(string result) + { + GitCommandLock.TryCheckOwnership( + 123, false, "git alias", + request => new NamedPipeMessages.Message(result, null), + out bool ownsLock, + out string error).ShouldBeTrue(); + + ownsLock.ShouldBeFalse(); + error.ShouldBeNull(); + } + + [TestCase("MountNotReady", "not finished initializing")] + [TestCase("UnmountInProgress", "unmounting")] + [TestCase("LockAcquired", "unexpected lock-ownership response")] + [TestCase("UnknownResponse", "unexpected lock-ownership response")] + [TestCase("LockDeniedGit", "without a valid process ID")] + public void InvalidOwnershipResponsesProduceADiagnostic(string result, string expectedError) + { + GitCommandLock.TryCheckOwnership( + 123, false, "git alias", + request => new NamedPipeMessages.Message(result, null), + out bool ownsLock, + out string error).ShouldBeFalse(); + + ownsLock.ShouldBeFalse(); + error.ShouldContain(expectedError); + } + + [TestCase] + public void MissingOwnershipResponseProducesADiagnostic() + { + GitCommandLock.TryCheckOwnership( + 123, false, "git alias", request => null, + out bool ownsLock, + out string error).ShouldBeFalse(); + + ownsLock.ShouldBeFalse(); + error.ShouldContain("no lock-ownership response"); + } + + [TestCase("malformed")] + [TestCase("123|false|false|-1|x|0|")] + [Category(CategoryConstants.ExceptionExpected)] + public void MalformedOwnershipResponseProducesADiagnostic(string body) + { + GitCommandLock.TryCheckOwnership( + 123, false, "git alias", + request => new NamedPipeMessages.Message("LockDeniedGit", body), + out bool ownsLock, + out string error).ShouldBeFalse(); + + ownsLock.ShouldBeFalse(); + error.ShouldContain("invalid lock-ownership response"); + } + } +} diff --git a/GVFS/GVFS.UnitTests/Hooks/GitCommandResolverTests.cs b/GVFS/GVFS.UnitTests/Hooks/GitCommandResolverTests.cs new file mode 100644 index 0000000000..2121165ba6 --- /dev/null +++ b/GVFS/GVFS.UnitTests/Hooks/GitCommandResolverTests.cs @@ -0,0 +1,386 @@ +using GVFS.Common; +using GVFS.Hooks; +using GVFS.Tests.Should; +using GVFS.UnitTests.Category; +using NUnit.Framework; +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.IO; +using Win32Exception = System.ComponentModel.Win32Exception; + +namespace GVFS.UnitTests.Hooks +{ + [TestFixture] + public class GitCommandResolverTests + { + private const string SearchPath = @"mock:\git\libexec;mock:\tools"; + private const uint NormalFile = 0x80; + private const uint Directory = 0x10; + private const string UnreadConfigErrors = + "warning: unable to access '.git/config': Permission denied\n" + + "warning: unable to access '.git/config': Permission denied\n" + + "fatal: unknown error occurred while reading the configuration files\n"; + + [TestCase("stash")] + [TestCase("for-each-repo")] + [TestCase("checkout")] + [TestCase("update-index")] + [TestCase("status")] + public void BuiltInsDoNotQueryAliasesOrSearchPath(string command) + { + GitCommandResolver.TryIsAlias( + command, + () => throw new AssertionException("A built-in must not query aliases."), + SearchPath, + path => throw new AssertionException("A built-in must not search PATH."), + out bool isAlias, + out string error).ShouldBeTrue(); + + isAlias.ShouldBeFalse(); + error.ShouldBeNull(); + } + + [TestCase("git-tools/command")] + [TestCase(@"git-tools\command")] + public void ExternalPathsDoNotQueryAliases(string command) + { + GitCommandResolver.TryIsAlias( + command, + () => throw new AssertionException("An external path must not query aliases."), + SearchPath, + path => throw new AssertionException("An external path must not search PATH."), + out bool isAlias, + out string error).ShouldBeTrue(); + + isAlias.ShouldBeFalse(); + error.ShouldBeNull(); + } + + [TestCase("git-co", "alias.st\nalias.co\nalias.co\n")] + [TestCase("git-CO", "alias.co\n")] + [TestCase("git-myAlias", "alias.myalias\n")] + [TestCase("git-sx", "alias.sx.command\n")] + [TestCase("git-MiXeD", "alias.MiXeD.command\n")] + [TestCase("git-a.b", "alias.a.b.command\n")] + [TestCase("git-mixed.foo", "alias.MiXeD.foo\n")] + [TestCase("git-CAF\u00e9.FOO", "alias.Caf\u00e9.foo\n")] + [TestCase("git-caf\u00e9", "alias.caf\u00e9.command\n")] + [TestCase("git-es", "alias..es\n")] + [TestCase("git-ES", "alias..es\n")] + [TestCase("git-command", "alias..command\n")] + [TestCase("git-.review.foo", "alias..review.foo\n")] + [TestCase("git-.REVIEW.FOO", "alias..review.foo\n")] + [TestCase("git-.review", "alias..review.command\n")] + [TestCase("git-git-prefixed", "alias.git-prefixed\n")] + public void AliasKeysThatGitExpandsSkipTheOuterLock(string hookCommand, string aliasKeys) + { + int queries = 0; + GitCommandResolver.TryIsAlias( + hookCommand, + () => + { + ++queries; + return new ProcessResult(aliasKeys, string.Empty, 0); + }, + SearchPath, + path => GitCommandResolver.InvalidFileAttributes, + out bool isAlias, + out string error).ShouldBeTrue(); + + queries.ShouldEqual(1); + isAlias.ShouldBeTrue(); + error.ShouldBeNull(); + } + + [TestCase("git-SX", "alias.sx.command\n", 0)] + [TestCase("git-zzfoO", "alias.zzfoo.command\n", 0)] + [TestCase("git-sx.command", "alias.sx.command\n", 0)] + [TestCase("git-A.B", "alias.a.b.command\n", 0)] + [TestCase("git-co", "alias.st\nalias.co-other\n", 0)] + [TestCase("git-co", "alias.\n", 0)] + [TestCase("git-", "alias.\nalias..\n", 0)] + [TestCase("git-.command", "alias..command\n", 0)] + [TestCase("git-review.foo", "alias..review.foo\n", 0)] + [TestCase("git-.REVIEW", "alias..review.command\n", 0)] + [TestCase("git-review", "alias..review.command\n", 0)] + [TestCase("git-CAF\u00c9.FOO", "alias.Caf\u00e9.foo\n", 0)] + [TestCase("git-co", "", 1)] + public void AliasKeysThatGitDoesNotExpandKeepTheLock(string hookCommand, string aliasKeys, int exitCode) + { + GitCommandResolver.TryIsAlias( + hookCommand, + () => new ProcessResult(aliasKeys, "13:00:00.000000 git.c:1 trace: built-in: git config\n", exitCode), + SearchPath, + path => GitCommandResolver.InvalidFileAttributes, + out bool isAlias, + out string error).ShouldBeTrue(); + + isAlias.ShouldBeFalse(); + error.ShouldBeNull(); + } + + [TestCase("git-co", @"mock:\git\libexec\git-co.exe")] + [TestCase("git-co", @"mock:\tools\git-co")] + [TestCase("git-Co", @"mock:\tools\GIT-CO.EXE")] + [TestCase("git-co.EXE", @"mock:\tools\git-co.EXE")] + [TestCase("git-git-prefixed", @"mock:\tools\git-git-prefixed")] + public void ProgramsTakePrecedenceWithoutAnAliasQuery(string hookCommand, string programPath) + { + GitCommandResolver.TryIsAlias( + hookCommand, + () => throw new AssertionException("A program must not query aliases."), + SearchPath, + path => string.Equals(path, programPath, StringComparison.OrdinalIgnoreCase) ? NormalFile : GitCommandResolver.InvalidFileAttributes, + out bool isAlias, + out string error).ShouldBeTrue(); + + isAlias.ShouldBeFalse(); + error.ShouldBeNull(); + } + + [TestCase(@"mock:\tools\git-co.exe", false)] + [TestCase(@"mock:\tools\git-co", true)] + public void DirectoriesMatchOnlyTheExeCandidate(string directoryPath, bool isAliasExpected) + { + GitCommandResolver.TryIsAlias( + "git-co", + () => new ProcessResult("alias.co\n", string.Empty, 0), + @"mock:\tools", + path => string.Equals(path, directoryPath, StringComparison.Ordinal) ? Directory : GitCommandResolver.InvalidFileAttributes, + out bool isAlias, + out string error).ShouldBeTrue(); + + isAlias.ShouldEqual(isAliasExpected); + error.ShouldBeNull(); + } + + [TestCase(128)] + [TestCase(1)] + public void UnreadConfigIsAFailure(int exitCode) + { + GitCommandResolver.TryIsAlias( + "git-co", + () => new ProcessResult(string.Empty, UnreadConfigErrors, exitCode), + SearchPath, + path => GitCommandResolver.InvalidFileAttributes, + out bool isAlias, + out string error).ShouldBeFalse(); + + isAlias.ShouldBeFalse(); + error.ShouldContain("Unable to read Git aliases (exit code " + exitCode + ")", "unable to access '.git/config'"); + } + + [TestCase(128, "")] + [TestCase(128, "alias.co\n")] + [TestCase(1, "alias.co\n")] + public void FailedAliasQueriesNeverGrantAnAliasExemption(int exitCode, string output) + { + GitCommandResolver.TryIsAlias( + "git-co", + () => new ProcessResult(output, "config read failed", exitCode), + SearchPath, + path => GitCommandResolver.InvalidFileAttributes, + out bool isAlias, + out string error).ShouldBeFalse(); + + isAlias.ShouldBeFalse(); + error.ShouldContain("Unable to read Git aliases", "exit code " + exitCode, "config read failed"); + } + + [TestCase] + [Category(CategoryConstants.ExceptionExpected)] + public void AliasQueryFailuresProducePreciseDiagnostics() + { + foreach (Exception failure in new Exception[] + { + new Win32Exception(5), + new InvalidOperationException("No executable path"), + new ArgumentException("No parent process"), + new IOException("Read failed"), + new UnauthorizedAccessException("Access denied"), + }) + { + GitCommandResolver.TryIsAlias( + "git-co", + () => throw failure, + SearchPath, + path => GitCommandResolver.InvalidFileAttributes, + out bool isAlias, + out string error).ShouldBeFalse(); + + isAlias.ShouldBeFalse(); + error.ShouldContain("Unable to read Git aliases", failure.GetType().Name, failure.HResult.ToString("X8"), failure.Message); + error.Contains(" at GVFS.").ShouldBeFalse(); + } + } + + [TestCase] + [Category(CategoryConstants.ExceptionExpected)] + public void MissingParentProcessIsNamedInTheError() + { + InvalidOperationException failure = Assert.Throws(() => GitCommandResolver.GetProcessPath(-1)); + failure.Message.ShouldContain("parent Git process (PID -1)"); + } + + [TestCase(null)] + [TestCase("")] + public void MissingSearchPathIsAFailure(string searchPath) + { + GitCommandResolver.TryIsAlias( + "git-co", + () => throw new AssertionException("An empty PATH must not query aliases."), + searchPath, + path => throw new AssertionException("An empty PATH must not be searched."), + out bool isAlias, + out string error).ShouldBeFalse(); + + isAlias.ShouldBeFalse(); + error.ShouldContain("inherited PATH is empty"); + } + + [TestCase] + public void SearchPathUsesRawEntriesAndSkipsEmptyEntries() + { + List candidates = new List(); + GitCommandResolver.TryIsAlias( + "git-co", + () => new ProcessResult("alias.co\n", string.Empty, 0), + @"; spaced ;relative tools;;mock:\tools\;", + path => + { + candidates.Add(path); + return GitCommandResolver.InvalidFileAttributes; + }, + out bool isAlias, + out string error).ShouldBeTrue(); + + candidates.ShouldMatchInOrder( + @" spaced \git-co.exe", + @" spaced \git-co", + @"relative tools\git-co.exe", + @"relative tools\git-co", + @"mock:\tools\\git-co.exe", + @"mock:\tools\\git-co"); + isAlias.ShouldBeTrue(); + error.ShouldBeNull(); + } + + [TestCase] + public void CandidatesThatFitGitsPathBufferAreChecked() + { + // Git builds "\.exe" in a 260-byte buffer. 259 bytes and the NUL fit. + string directory = @"mock:\" + new string('d', 242); + List candidates = new List(); + GitCommandResolver.TryIsAlias( + "git-co", + () => new ProcessResult("alias.co\n", string.Empty, 0), + directory, + path => + { + candidates.Add(path); + return GitCommandResolver.InvalidFileAttributes; + }, + out bool isAlias, + out string error).ShouldBeTrue(); + + candidates.ShouldMatchInOrder(directory + @"\git-co.exe", directory + @"\git-co"); + isAlias.ShouldBeTrue(); + } + + [TestCase(243, "")] + [TestCase(129, "\u00e9")] + public void CandidatesLongerThanGitsPathBufferAreFailures(int directoryLength, string filler) + { + string directory = @"mock:\" + (filler.Length == 0 ? new string('d', directoryLength) : string.Concat(System.Linq.Enumerable.Repeat(filler, directoryLength))); + GitCommandResolver.TryIsAlias( + "git-co", + () => throw new AssertionException("A failed PATH search must not query aliases."), + @"mock:\tools-without-co;" + directory + @";mock:\tools", + path => path.StartsWith(@"mock:\tools\", StringComparison.Ordinal) ? NormalFile : GitCommandResolver.InvalidFileAttributes, + out bool isAlias, + out string error).ShouldBeFalse(); + + isAlias.ShouldBeFalse(); + error.ShouldContain("is too long for Git's path buffer"); + } + + [TestCase] + public void ProgramsFoundBeforeALongEntryDoNotFail() + { + GitCommandResolver.TryIsAlias( + "git-co", + () => throw new AssertionException("A program must not query aliases."), + @"mock:\tools;mock:\" + new string('d', 300), + path => path == @"mock:\tools\git-co.exe" ? NormalFile : GitCommandResolver.InvalidFileAttributes, + out bool isAlias, + out string error).ShouldBeTrue(); + + isAlias.ShouldBeFalse(); + error.ShouldBeNull(); + } + + [TestCase] + public void ExecutableSuffixDoesNotAddASecondExtension() + { + List candidates = new List(); + GitCommandResolver.TryIsAlias( + "git-co.EXE", + () => new ProcessResult("alias.co.exe\n", string.Empty, 0), + @"mock:\tools", + path => + { + candidates.Add(path); + return GitCommandResolver.InvalidFileAttributes; + }, + out bool isAlias, + out string error).ShouldBeTrue(); + + candidates.ShouldMatchInOrder(@"mock:\tools\git-co.EXE"); + isAlias.ShouldBeTrue(); + error.ShouldBeNull(); + } + + [TestCase] + public void GetFileAttributesFailsForPathsThatGitCannotAccess() + { + string directory = Path.Combine(Path.GetTempPath(), "GitCommandResolverTests_" + Guid.NewGuid().ToString("N")); + System.IO.Directory.CreateDirectory(directory); + try + { + string file = Path.Combine(directory, "git-co"); + File.WriteAllText(file, string.Empty); + + (GitCommandResolver.GetFileAttributes(file) & Directory).ShouldEqual(0u); + (GitCommandResolver.GetFileAttributes(directory) & Directory).ShouldEqual(Directory); + GitCommandResolver.GetFileAttributes(Path.Combine(directory, "missing", "git-co")).ShouldEqual(GitCommandResolver.InvalidFileAttributes); + GitCommandResolver.GetFileAttributes("\"" + directory + "\"\\git-co").ShouldEqual(GitCommandResolver.InvalidFileAttributes); + } + finally + { + System.IO.Directory.Delete(directory, recursive: true); + } + } + + [TestCase] + public void CurrentProcessPathUsesTheSuppliedPid() + { + GitCommandResolver.GetProcessPath(Environment.ProcessId).ShouldEqual(Environment.ProcessPath); + } + + [TestCase] + public void AliasKeyQueryUsesTheParentGit() + { + const string ParentGit = @"mock:\custom git\bin\git.exe"; + ProcessStartInfo query = GitCommandResolver.CreateAliasKeyQuery(ParentGit); + + query.FileName.ShouldEqual(ParentGit); + query.UseShellExecute.ShouldBeFalse(); + query.RedirectStandardOutput.ShouldBeTrue(); + query.RedirectStandardError.ShouldBeTrue(); + query.ArgumentList.ShouldMatchInOrder("config", "--name-only", "--get-regexp", @"^alias\."); + query.StandardOutputEncoding.WebName.ShouldEqual("utf-8"); + query.Environment["COMMAND_HOOK_LOCK"].ShouldEqual("true"); + } + } +} diff --git a/docs/faq.md b/docs/faq.md index 5026fe6411..35b8cf01b0 100644 --- a/docs/faq.md +++ b/docs/faq.md @@ -25,6 +25,24 @@ you can help optimize Git command performance for developers in the repository by limiting the number of files Git needs to consider for many common operations. +### Do Git aliases change command locks? + +If Git runs a built-in command or an external `git-` program, an +alias with the same name does not remove the GVFS lock requirement. For +example, `alias.stash` does not change the lock requirement for +`git stash`. + +If Git expands an alias, VFS for Git does not lock the alias itself. +This applies to both alias forms: `alias.` and +`[alias ""] command`. Each command in the alias follows its normal +lock policy. For example, an alias that runs `checkout` still requires +the lock for `checkout`. + +If VFS for Git cannot read the Git configuration to find aliases, it +stops the command with an error. Run the command again. If the error +continues, run `git config --get-regexp ^alias\.` to check your Git +configuration. + ### Why the name change? This project was formerly known as GVFS (Git Virtual File System). It is