Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions GitBranchStateCache.Tests/Admission/AdmissionGateTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,19 @@ public async Task AdmitAsync_WhenLsRemoteSucceeds_Admits()
Assert.AreEqual("ls-remote", runner.Invocations.Single().Arguments[0]);
}

[TestMethod]
public async Task AdmitAsync_DiscardsTheProbesOutput()
{
// The probe proves a credential by succeeding, and its output is never read. Decoding it would
// let one branch name that is not valid UTF-8 refuse every caller of the repository
// (ktsu-dev/GitBranchStateCache#50).
(AdmissionGate gate, FakeGitRunner runner, _) = Build();

await AdmitAsync(gate);

Assert.IsTrue(runner.Invocations.Single().DiscardStandardOutput);
}

[TestMethod]
public async Task AdmitAsync_PassesTheCredentialThroughTheEnvironmentAndNeverAnArgument()
{
Expand Down
60 changes: 60 additions & 0 deletions GitBranchStateCache.Tests/Git/GitRunnerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,66 @@ public async Task RunAsync_OutputThatIsNotUtf8_IsReportedRatherThanReadAsEmptyOr
Assert.Contains("not valid UTF-8", result.StandardError);
}

[TestMethod]
[OSCondition(OperatingSystems.Linux | OperatingSystems.OSX)]
public async Task RunAsync_OutputThatIsNotUtf8AndLargerThanThePipe_IsReportedPromptlyRatherThanAsATimeout()
{
// A diff-tree with one non-UTF-8 path among enough others to fill the pipe. Once the decoder
// refuses the bad byte, the rest still has to be drained or the process ended, or git blocks on
// the full pipe and the run is reported as a timeout instead (ktsu-dev/GitBranchStateCache#50).
Stopwatch elapsed = Stopwatch.StartNew();

GitResult result = await Build("/bin/sh").RunAsync(
new GitInvocation
{
Arguments = ["-c", "printf 'b\\377.uasset\\0'; head -c 204800 /dev/zero | tr '\\0' a"],
Timeout = TimeSpan.FromSeconds(20),
},
CancellationToken.None);

Assert.IsFalse(result.TimedOut, "The run stalled on a full pipe until it was killed.");
Assert.Contains("not valid UTF-8", result.StandardError);
Assert.IsLessThan(TimeSpan.FromSeconds(10), elapsed.Elapsed);
}

[TestMethod]
[OSCondition(OperatingSystems.Linux | OperatingSystems.OSX)]
public async Task RunAsync_DiscardingStandardOutput_SucceedsWhateverTheOutputHolds()
{
// The admission probe asks only whether ls-remote succeeded. A branch name git allows but
// UTF-8 cannot read must not turn a reachable repository into a refused one.
GitResult result = await Build("/bin/sh").RunAsync(
new GitInvocation
{
Arguments = ["-c", "printf 'deadbeef\\trefs/heads/caf\\351\\n'; head -c 204800 /dev/zero | tr '\\0' a"],
Timeout = TimeSpan.FromSeconds(20),
DiscardStandardOutput = true,
},
CancellationToken.None);

Assert.IsTrue(result.Succeeded, result.StandardError);
Assert.AreEqual(string.Empty, result.StandardOutput);
}

[TestMethod]
[OSCondition(OperatingSystems.Linux | OperatingSystems.OSX)]
public async Task RunAsync_DiscardingStandardOutput_StillReportsStandardError()
{
// What a refused probe says on standard error is what classifies it, so only standard output
// is thrown away.
GitResult result = await Build("/bin/sh").RunAsync(
new GitInvocation
{
Arguments = ["-c", "printf 'fatal: Authentication failed\\n' >&2; exit 128"],
Timeout = TimeSpan.FromSeconds(20),
DiscardStandardOutput = true,
},
CancellationToken.None);

Assert.IsFalse(result.Succeeded);
Assert.Contains("Authentication failed", result.StandardError);
}

private static string ReaderExecutable() => OnWindows ? "cmd.exe" : "/bin/sh";

private static string[] ReaderArguments() => OnWindows
Expand Down
4 changes: 4 additions & 0 deletions GitBranchStateCache/Admission/AdmissionGate.cs
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,10 @@ public async Task<AdmissionOutcome> AdmitAsync(
CredentialScope = upstreamBase,
Authorization = authorization,
Timeout = options.Value.ProbeTimeout,

// Only whether the probe succeeded matters, so a branch name that is not valid UTF-8
// cannot refuse a whole repository (ktsu-dev/GitBranchStateCache#50).
DiscardStandardOutput = true,
},
cancellationToken).ConfigureAwait(false);

Expand Down
11 changes: 11 additions & 0 deletions GitBranchStateCache/Git/GitInvocation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,17 @@ public sealed class GitInvocation
/// </remarks>
public string? Authorization { get; init; }

/// <summary>
/// Gets whether standard output is thrown away unread rather than decoded and returned.
/// </summary>
/// <remarks>
/// For a command whose answer is its exit code alone. Its output is still drained, so the command
/// never blocks on a full pipe, but it is not held to strict UTF-8: a branch name git allows and
/// UTF-8 cannot read would otherwise fail a command whose output nobody looks at
/// (ktsu-dev/GitBranchStateCache#50).
/// </remarks>
public bool DiscardStandardOutput { get; init; }

/// <summary>Gets how long the command may run before it is killed.</summary>
public required TimeSpan Timeout { get; init; }
}
21 changes: 17 additions & 4 deletions GitBranchStateCache/Git/GitRunner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,14 @@ public sealed class GitRunner(IOptions<GitBranchStateCacheOptions> options) : IG
encoderShouldEmitUTF8Identifier: false,
throwOnInvalidBytes: true);

/// <summary>
/// Decodes the output of a run whose standard output is discarded, where only standard error is
/// kept and it is only ever shown or searched for git's own ASCII messages.
/// </summary>
private static readonly Encoding LenientUtf8 = new UTF8Encoding(
encoderShouldEmitUTF8Identifier: false,
throwOnInvalidBytes: false);

/// <inheritdoc />
/// <remarks>
/// Starting, reading and killing the process is <see cref="RunCommand.ExecuteAsync(string, IEnumerable{string}, OutputHandler, CommandOptions, CancellationToken)"/>'s
Expand All @@ -67,10 +75,15 @@ public async Task<GitResult> RunAsync(GitInvocation invocation, CancellationToke

StringBuilder standardOutput = new();
StringBuilder standardError = new();
OutputHandler output = new(
chunk => standardOutput.Append(chunk),
chunk => standardError.Append(chunk),
StrictUtf8);
OutputHandler output = invocation.DiscardStandardOutput
? new(
_ => { },
chunk => standardError.Append(chunk),
LenientUtf8)
: new(
chunk => standardOutput.Append(chunk),
chunk => standardError.Append(chunk),
StrictUtf8);

CommandOptions commandOptions = new()
{
Expand Down
Loading