Don't allow null name or e-mail in PersonIdent

toExternalString, equals and hashCode don't expect them to be null, so
explicitly disallow it in the constructor.

Also fix the documentation of setAuthor and setCommitter in
CommitCommand when specifying name and email as separate arguments.

Bug: 352984
Change-Id: I0ac994ae8e47789d38f7c6e6db55d482f0f1bac3

3 files changed, 47 insertions, 38 deletions

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/T0001_PersonIdentTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/T0001_PersonIdentTest.java
index 34a557bd92..1b7276bf77 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/T0001_PersonIdentTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/T0001_PersonIdentTest.java
@@ -73,4 +73,14 @@ public class T0001_PersonIdentTest {
 		assertEquals("A U Thor <author@example.com> 1142878501 +0230",
 				p.toExternalString());
 	}
+
+	@Test(expected = IllegalArgumentException.class)
+	public void nullForNameShouldThrowIllegalArgumentException() {
+		new PersonIdent(null, "author@example.com");
+	}
+
+	@Test(expected = IllegalArgumentException.class)
+	public void nullForEmailShouldThrowIllegalArgumentException() {
+		new PersonIdent("A U Thor", null);
+	}
 }
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/CommitCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/CommitCommand.java
index a166790a40..ee92083175 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/api/CommitCommand.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/CommitCommand.java
@@ -548,9 +548,8 @@ public class CommitCommand extends GitCommand<RevCommit> {
 
 	/**
 	 * Sets the committer for this {@code commit}. If no committer is explicitly
-	 * specified because this method is never called or called with {@code null}
-	 * value then the committer will be deduced from config info in repository,
-	 * with current time.
+	 * specified because this method is never called then the committer will be
+	 * deduced from config info in repository, with current time.
 	 *
 	 * @param name
 	 *            the name of the committer used for the {@code commit}
@@ -591,9 +590,8 @@ public class CommitCommand extends GitCommand<RevCommit> {
 
 	/**
 	 * Sets the author for this {@code commit}. If no author is explicitly
-	 * specified because this method is never called or called with {@code null}
-	 * value then the author will be set to the committer or to the original
-	 * author when amending.
+	 * specified because this method is never called then the author will be set
+	 * to the committer or to the original author when amending.
 	 *
 	 * @param name
 	 *            the name of the author used for the {@code commit}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/PersonIdent.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/PersonIdent.java
index 1ec8f4c9ea..f81d9c5a84 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/PersonIdent.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/PersonIdent.java
@@ -78,11 +78,7 @@ public class PersonIdent implements Serializable {
 	 * @param repo
 	 */
 	public PersonIdent(final Repository repo) {
-		final UserConfig config = repo.getConfig().get(UserConfig.KEY);
-		name = config.getCommitterName();
-		emailAddress = config.getCommitterEmail();
-		when = SystemReader.getInstance().getCurrentTime();
-		tzOffset = SystemReader.getInstance().getTimezone(when);
+		this(repo.getConfig().get(UserConfig.KEY));
 	}
 
 	/**
@@ -102,10 +98,7 @@ public class PersonIdent implements Serializable {
 	 * @param aEmailAddress
 	 */
 	public PersonIdent(final String aName, final String aEmailAddress) {
-		name = aName;
-		emailAddress = aEmailAddress;
-		when = SystemReader.getInstance().getCurrentTime();
-		tzOffset = SystemReader.getInstance().getTimezone(when);
+		this(aName, aEmailAddress, SystemReader.getInstance().getCurrentTime());
 	}
 
 	/**
@@ -131,10 +124,7 @@ public class PersonIdent implements Serializable {
 	 *            local time
 	 */
 	public PersonIdent(final PersonIdent pi, final Date aWhen) {
-		name = pi.getName();
-		emailAddress = pi.getEmailAddress();
-		when = aWhen.getTime();
-		tzOffset = pi.tzOffset;
+		this(pi.getName(), pi.getEmailAddress(), aWhen.getTime(), pi.tzOffset);
 	}
 
 	/**
@@ -149,43 +139,54 @@ public class PersonIdent implements Serializable {
 	 */
 	public PersonIdent(final String aName, final String aEmailAddress,
 			final Date aWhen, final TimeZone aTZ) {
-		name = aName;
-		emailAddress = aEmailAddress;
-		when = aWhen.getTime();
-		tzOffset = aTZ.getOffset(when) / (60 * 1000);
+		this(aName, aEmailAddress, aWhen.getTime(), aTZ.getOffset(aWhen
+				.getTime()) / (60 * 1000));
 	}
 
 	/**
-	 * Construct a {@link PersonIdent}
+	 * Copy a PersonIdent, but alter the clone's time stamp
 	 *
-	 * @param aName
-	 * @param aEmailAddress
+	 * @param pi
+	 *            original {@link PersonIdent}
 	 * @param aWhen
 	 *            local time stamp
 	 * @param aTZ
 	 *            time zone
 	 */
-	public PersonIdent(final String aName, final String aEmailAddress,
-			final long aWhen, final int aTZ) {
-		name = aName;
-		emailAddress = aEmailAddress;
-		when = aWhen;
-		tzOffset = aTZ;
+	public PersonIdent(final PersonIdent pi, final long aWhen, final int aTZ) {
+		this(pi.getName(), pi.getEmailAddress(), aWhen, aTZ);
+	}
+
+	private PersonIdent(final String aName, final String aEmailAddress,
+			long when) {
+		this(aName, aEmailAddress, when, SystemReader.getInstance()
+				.getTimezone(when));
+	}
+
+	private PersonIdent(final UserConfig config) {
+		this(config.getCommitterName(), config.getCommitterEmail());
 	}
 
 	/**
-	 * Copy a PersonIdent, but alter the clone's time stamp
+	 * Construct a {@link PersonIdent}
 	 *
-	 * @param pi
-	 *            original {@link PersonIdent}
+	 * @param aName
+	 * @param aEmailAddress
 	 * @param aWhen
 	 *            local time stamp
 	 * @param aTZ
 	 *            time zone
 	 */
-	public PersonIdent(final PersonIdent pi, final long aWhen, final int aTZ) {
-		name = pi.getName();
-		emailAddress = pi.getEmailAddress();
+	public PersonIdent(final String aName, final String aEmailAddress,
+			final long aWhen, final int aTZ) {
+		if (aName == null)
+			throw new IllegalArgumentException(
+					"Name of PersonIdent must not be null.");
+		if (aEmailAddress == null)
+			throw new IllegalArgumentException(
+					"E-mail address of PersonIdent must not be null.");
+		name = aName;
+		emailAddress = aEmailAddress;
 		when = aWhen;
 		tzOffset = aTZ;
 	}