8000 Don't build a fake signature · github/libgit2sharp@53f880c · GitHub
[go: up one dir, main page]
Skip to content
This repository was archived by the owner on Feb 3, 2023. It is now read-only.

Commit 53f880c

Browse files
carlosmncarlosmn
committed
Don't build a fake signature
When there is no user information in the configuration files, BuildSignature() sets "unkown" as the name and uses the environment variables to create the e-mail. This seems peculiar, since it means a consumer of the library cannot use this method to figure out if this configuration does exist but has to do it themselves. As far as I can tell, this started as a way to provide something to the reflog when the user has not set up the information as it's more transient data and progress matters more than accuracy in that case. This is now handled by libgit2 itself, so there is no need for this behaviour inside the library. This leaves us with the curious case of this fake signature only being provided to a consumer of the library when this behaviour was due to some internal users. It also makes it harder than it needs to be to know if the signature the library provided is accurate. Resolve this situation by returning null when there is no signature configured. The internal users can then move to use a method which throws a message mentioning the lack of a necessary signature.
1 parent 01b9a62 commit 53f880c

File tree

3 files changed

+20
-45
lines changed

3 files changed

+20
-45
lines changed

‎LibGit2Sharp/Configuration.cs‎

Lines changed: 16 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -743,12 +743,8 @@ private static ConfigurationEntry<string> BuildConfigEntry(IntPtr entryPtr)
743743
}
744744

745745
/// <summary>
746-
/// Builds a <see cref="Signature"/> based on current configuration.
747-
/// <para>
748-
/// Name is populated from the user.name setting, and is "unknown" if unspecified.
749-
/// Email is populated from the user.email setting, and is built from
750-
/// <see cref="Environment.UserName"/> and <see cref="Environment.UserDomainName"/> if unspecified.
751-
/// </para>
746+
/// Builds a <see cref="Signature"/> based on current configuration. If it is not found or
747+
/// some configuration is missing, <code>null</code> is returned.
752748
/// <para>
753749
/// The same escalation logic than in git.git will be used when looking for the key in the config files:
754750
/// - local: the Git file in the current repository
@@ -758,51 +754,30 @@ private static ConfigurationEntry<string> BuildConfigEntry(IntPtr entryPtr)
758754
/// </para>
759755
/// </summary>
760756
/// <param name="now">The timestamp to use for the <see cref="Signature"/>.</param>
761-
/// <returns>The signature.</returns>
757+
/// <returns>The signature or null if no user identity can be found in the configuration.</returns>
762758
public virtual Signature BuildSignature(DateTimeOffset now)
763759
{
764-
return BuildSignature(now, false);
765-
}
766-
767-
internal Signature BuildSignature(DateTimeOffset now, bool shouldThrowIfNotFound)
768-
{
769-
const string userNameKey = "user.name";
770-
var name = this.GetValueOrDefault<string>(userNameKey);
771-
var normalizedName = NormalizeUserSetting(shouldThrowIfNotFound,
772-
userNameKey,
773-
name,
774-
() => "unknown");
775-
776-
const string userEmailKey = "user.email";
777-
var email = this.GetValueOrDefault<string>(userEmailKey);
778-
var normalizedEmail = NormalizeUserSetting(shouldThrowIfNotFound,
779-
userEmailKey,
780-
email,
781-
() => string.Format(CultureInfo.InvariantCulture,
782-
"{0}@{1}",
783-
Environment.UserName,
784-
Environment.UserDomainName));
760+
var name = this.GetValueOrDefault<string>("user.name");
761+
var email = this.GetValueOrDefault<string>("user.email");
785762

786-
return new Signature(normalizedName, normalizedEmail, now);
787-
}
788-
789-
private string NormalizeUserSetting(bool shouldThrowIfNotFound, string entryName, string currentValue, Func<string> defaultValue)
790-
{
791-
if (!string.IsNullOrEmpty(currentValue))
763+
if (string.IsNullOrEmpty(name) || string.IsNullOrEmpty(email))
792764
{
793-
return currentValue;
765+
return null;
794766
}
795767

796-
string message = string.Format("Configuration value '{0}' is missing or invalid.", entryName);
768+
return new Signature(name, email, now);
769+
}
797770

798-
if (shouldThrowIfNotFound)
771+
internal Signature BuildSignatureOrThrow(DateTimeOffset now)
772+
{
773+
var signature = BuildSignature(now);
774+
if (signature == null)
799775
{
800-
throw new LibGit2SharpException(message);
776+
throw new LibGit2SharpException("This overload requires 'user.name' and 'user.email' to be set. " +
777+
"Use a different overload or set those variables in the configuation");
801778
}
802779< 8000 code class="diff-text syntax-highlighted-line">

803-
Log.Write(LogLevel.Warning, message);
804-
805-
return defaultValue();
780+
return signature;
806781
}
807782

808783
private ConfigurationSafeHandle Snapshot()

‎LibGit2Sharp/NoteCollection.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ internal static string UnCanonicalizeName(string name)
173173
/// <returns>The note which was just saved.</returns>
174174
public virtual Note Add(ObjectId targetId, string message, string @namespace)
175175
{
176-
Signature author = repo.Config.BuildSignature(DateTimeOffset.Now, true);
176+
Signature author = repo.Config.BuildSignatureOrThrow(DateTimeOffset.Now);
177177

178178
return Add(targetId, message, author, author, @namespace);
179179
}
@@ -212,7 +212,7 @@ public virtual Note Add(ObjectId targetId, string message, Signature author, Sig
212212
/// <param name="namespace">The namespace on which the note will be removed. It can be either a canonical namespace or an abbreviated namespace ('refs/notes/myNamespace' or just 'myNamespace').</param>
213213
public virtual void Remove(ObjectId targetId, string @namespace)
214214
{
215-
Signature author = repo.Config.BuildSignature(DateTimeOffset.Now, true);
215+
Signature author = repo.Config.BuildSignatureOrThrow(DateTimeOffset.Now);
216216

217217
Remove(targetId, author, author, @namespace);
218218
}

‎LibGit2Sharp/RepositoryExtensions.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ public static Commit Commit(this IRepository repository, string message)
236236
/// <returns>The generated <see cref="LibGit2Sharp.Commit"/>.</returns>
237237
public static Commit Commit(this IRepository repository, string message, CommitOptions options)
238238
{
239-
Signature author = repository.Config.BuildSignature(DateTimeOffset.Now, true);
239+
Signature author = repository.Config.BuildSignatureOrThrow(DateTimeOffset.Now);
240240

241241
return repository.Commit(message, author, options);
242242
}
@@ -270,7 +270,7 @@ public static Commit Commit(this IRepository repository, string message, Signatu
270270
/// <returns>The generated <see cref="LibGit2Sharp.Commit"/>.</returns>
271271
public static Commit Commit(this IRepository repository, string message, Signature author, CommitOptions options)
272272
{
273-
Signature committer = repository.Config.BuildSignature(DateTimeOffset.Now, true);
273+
Signature committer = repository.Config.BuildSignatureOrThrow(DateTimeOffset.Now);
274274

275275
return repository.Commit(message, author, committer, options);
276276
}

0 commit comments

Comments
 (0)
0