Skip to content

Commit 312354b

Browse files
authored
Fix detection of configured GCM helper (git-ecosystem#2349)
Better handling of multiple matching entries (last/first index deviation). Detect empty trailing value to skip addition of redundant guard entry.
2 parents c4be6b2 + 3524619 commit 312354b

2 files changed

Lines changed: 57 additions & 2 deletions

File tree

src/shared/Core.Tests/ApplicationTests.cs

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,55 @@ public async Task Application_ConfigureAsync_EmptyAndGcmWithEmptyAfter_RemovesEx
178178
Assert.Equal(executablePath, actualValues[4]);
179179
}
180180

181+
[Fact]
182+
public async Task Application_ConfigureAsync_MultiGcmWithValidEmpty_DoesNothing()
183+
{
184+
const string emptyHelper = "";
185+
const string executablePath = "/usr/local/share/gcm-core/git-credential-manager";
186+
string key = $"{Constants.GitConfiguration.Credential.SectionName}.{Constants.GitConfiguration.Credential.Helper}";
187+
188+
var context = new TestCommandContext { AppPath = executablePath };
189+
IConfigurableComponent application = new Application(context);
190+
191+
context.Git.Configuration.Global[key] = new List<string>
192+
{
193+
executablePath, emptyHelper, executablePath
194+
};
195+
196+
await application.ConfigureAsync(ConfigurationTarget.User);
197+
198+
Assert.Single(context.Git.Configuration.Global);
199+
Assert.True(context.Git.Configuration.Global.TryGetValue(key, out var actualValues));
200+
Assert.Equal(3, actualValues.Count);
201+
Assert.Equal(executablePath, actualValues[0]);
202+
Assert.Equal(emptyHelper, actualValues[1]);
203+
Assert.Equal(executablePath, actualValues[2]);
204+
}
205+
206+
[Fact]
207+
public async Task Application_ConfigureAsync_EmptyOnly_AddsGcmOnly()
208+
{
209+
const string emptyHelper = "";
210+
const string executablePath = "/usr/local/share/gcm-core/git-credential-manager";
211+
string key = $"{Constants.GitConfiguration.Credential.SectionName}.{Constants.GitConfiguration.Credential.Helper}";
212+
213+
var context = new TestCommandContext { AppPath = executablePath };
214+
IConfigurableComponent application = new Application(context);
215+
216+
context.Git.Configuration.Global[key] = new List<string>
217+
{
218+
emptyHelper
219+
};
220+
221+
await application.ConfigureAsync(ConfigurationTarget.User);
222+
223+
Assert.Single(context.Git.Configuration.Global);
224+
Assert.True(context.Git.Configuration.Global.TryGetValue(key, out var actualValues));
225+
Assert.Equal(2, actualValues.Count);
226+
Assert.Equal(emptyHelper, actualValues[0]);
227+
Assert.Equal(executablePath, actualValues[1]);
228+
}
229+
181230
[Fact]
182231
public async Task Application_UnconfigureAsync_NoHelpers_DoesNothing()
183232
{

src/shared/Core/Application.cs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ Task IConfigurableComponent.ConfigureAsync(ConfigurationTarget target)
204204

205205
// Try to locate an existing app entry with a blank reset/clear entry immediately preceding,
206206
// and no other blank empty/clear entries following (which effectively disable us).
207-
int appIndex = Array.FindIndex(currentValues, x => Context.FileSystem.IsSamePath(x, appPath));
207+
int appIndex = Array.FindLastIndex(currentValues, x => Context.FileSystem.IsSamePath(x, appPath));
208208
int lastEmptyIndex = Array.FindLastIndex(currentValues, string.IsNullOrWhiteSpace);
209209
if (appIndex > 0 && string.IsNullOrWhiteSpace(currentValues[appIndex - 1]) && lastEmptyIndex < appIndex)
210210
{
@@ -217,9 +217,15 @@ Task IConfigurableComponent.ConfigureAsync(ConfigurationTarget target)
217217
// Clear any existing app entries in the configuration
218218
config.UnsetAll(configLevel, helperKey, Regex.Escape(appPath));
219219

220+
// Reload updated helper settings (unset only clears entries in primary file, ignores includes and alternatives)
221+
currentValues = config.GetAll(configLevel, GitConfigurationType.Raw, helperKey).ToArray();
222+
220223
// Add an empty value for `credential.helper`, which has the effect of clearing any helper value
221224
// from any lower-level Git configuration, then add a second value which is the actual executable path.
222-
config.Add(configLevel, helperKey, string.Empty);
225+
if ((currentValues.Length == 0) || !string.IsNullOrWhiteSpace(currentValues.Last()))
226+
{
227+
config.Add(configLevel, helperKey, string.Empty);
228+
}
223229
config.Add(configLevel, helperKey, appPath);
224230
}
225231

0 commit comments

Comments
 (0)