Add fsck.allowInvalidPersonIdent to accept invalid author/committers

A larger than expected number of real-world repositories found on
the Internet contain invalid author, committer and tagger lines
in their history. Many of these seem to be caused by users misusing
the user.name and user.email fields, e.g.:

  [user]
    name = Au Thor <author@example.com>
    email = author@example.com

that some version of Git (or a reimplementation thereof) copied
directly into the object header. These headers are not valid and
are rejected by a strict fsck, making it impossible to transfer
the repository with JGit/EGit.

Another form is an invalid committer line with double negative for
the time zone, e.g.

  committer Au Thor <a@b> 1288373970 --700

The real world is messy. :(

Allow callers and users to weaken the fsck settings to accept these
sorts of breakages if they really want to work on a repo that has
broken history.  Most routines will still function fine, however
commit timestamp sorting in RevWalk may become confused by a corrupt
committer line and sort commits out of order. This is mostly fine if
the corrupted chain is shorter than the slop window.

Change-Id: I6d529542c765c131de590f4f7ef8e7c1c8cb9db9

org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ObjectCheckerTest.java

@@ -120,6 +120,42 @@ public class ObjectCheckerTest {
 		checker.check(Constants.OBJ_COMMIT, data);
 	}
 
+	@Test
+	public void testCommitCorruptAuthor() throws CorruptObjectException {
+		StringBuilder b = new StringBuilder();
+		b.append("tree be9bfa841874ccc9f2ef7c48d0c76226f89b7189\n");
+		b.append("author b <b@c> <b@c> 0 +0000\n");
+		b.append("committer <> 0 +0000\n");
+
+		byte[] data = Constants.encodeASCII(b.toString());
+		try {
+			checker.checkCommit(data);
+			fail("Did not catch corrupt object");
+		} catch (CorruptObjectException e) {
+			assertEquals("invalid author", e.getMessage());
+		}
+		checker.setAllowInvalidPersonIdent(true);
+		checker.checkCommit(data);
+	}
+
+	@Test
+	public void testCommitCorruptCommitter() throws CorruptObjectException {
+		StringBuilder b = new StringBuilder();
+		b.append("tree be9bfa841874ccc9f2ef7c48d0c76226f89b7189\n");
+		b.append("author <> 0 +0000\n");
+		b.append("committer b <b@c> <b@c> 0 +0000\n");
+
+		byte[] data = Constants.encodeASCII(b.toString());
+		try {
+			checker.checkCommit(data);
+			fail("Did not catch corrupt object");
+		} catch (CorruptObjectException e) {
+			assertEquals("invalid committer", e.getMessage());
+		}
+		checker.setAllowInvalidPersonIdent(true);
+		checker.checkCommit(data);
+	}
+
 	@Test
 	public void testValidCommit1Parent() throws CorruptObjectException {
 		final StringBuilder b = new StringBuilder();
@@ -940,7 +976,8 @@ public class ObjectCheckerTest {
 	}
 
 	@Test
-	public void testInvalidTagInvalidTaggerHeader1() {
+	public void testInvalidTagInvalidTaggerHeader1()
+			throws CorruptObjectException {
 		final StringBuilder b = new StringBuilder();
 
 		b.append("object ");
@@ -958,6 +995,8 @@ public class ObjectCheckerTest {
 		} catch (CorruptObjectException e) {
 			assertEquals("invalid tagger", e.getMessage());
 		}
+		checker.setAllowInvalidPersonIdent(true);
+		checker.checkTag(data);
 	}
 
 	@Test

org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectChecker.java

@@ -104,6 +104,8 @@ public class ObjectChecker {
 	private final MutableInteger ptrout = new MutableInteger();
 
 	private boolean allowZeroMode;
+
+	private boolean allowInvalidPersonIdent;
 	private boolean windows;
 	private boolean macosx;
 
@@ -124,6 +126,22 @@ public class ObjectChecker {
 		return this;
 	}
 
+	/**
+	 * Enable accepting invalid author, committer and tagger identities.
+	 * <p>
+	 * Some broken Git versions/libraries allowed users to create commits and
+	 * tags with invalid formatting between the name, email and timestamp.
+	 *
+	 * @param allow
+	 *            if true accept invalid person identity strings.
+	 * @return {@code this}.
+	 * @since 4.0
+	 */
+	public ObjectChecker setAllowInvalidPersonIdent(boolean allow) {
+		allowInvalidPersonIdent = allow;
+		return this;
+	}
+
 	/**
 	 * Restrict trees to only names legal on Windows platforms.
 	 * <p>
@@ -198,6 +216,9 @@ public class ObjectChecker {
 	}
 
 	private int personIdent(final byte[] raw, int ptr) {
+		if (allowInvalidPersonIdent)
+			return nextLF(raw, ptr) - 1;
+
 		final int emailB = nextLF(raw, ptr, '<');
 		if (emailB == ptr || raw[emailB - 1] != '<')
 			return -1;

org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java

@@ -289,6 +289,7 @@ public abstract class BaseReceivePack {
 
 		final boolean checkReceivedObjects;
 		final boolean allowLeadingZeroFileMode;
+		final boolean allowInvalidPersonIdent;
 		final boolean safeForWindows;
 		final boolean safeForMacOS;
 
@@ -306,6 +307,8 @@ public abstract class BaseReceivePack {
 					config.getBoolean("transfer", "fsckobjects", false)); //$NON-NLS-1$ //$NON-NLS-2$
 			allowLeadingZeroFileMode = checkReceivedObjects
 					&& config.getBoolean("fsck", "allowLeadingZeroFileMode", false); //$NON-NLS-1$ //$NON-NLS-2$
+			allowInvalidPersonIdent = checkReceivedObjects
+					&& config.getBoolean("fsck", "allowInvalidPersonIdent", false); //$NON-NLS-1$ //$NON-NLS-2$
 			safeForWindows = checkReceivedObjects
 					&& config.getBoolean("fsck", "safeForWindows", false); //$NON-NLS-1$ //$NON-NLS-2$
 			safeForMacOS = checkReceivedObjects
@@ -326,6 +329,7 @@ public abstract class BaseReceivePack {
 				return null;
 			return new ObjectChecker()
 				.setAllowLeadingZeroFileMode(allowLeadingZeroFileMode)
+				.setAllowInvalidPersonIdent(allowInvalidPersonIdent)
 				.setSafeForWindows(safeForWindows)
 				.setSafeForMacOS(safeForMacOS);
 		}

org.eclipse.jgit/src/org/eclipse/jgit/transport/TransferConfig.java

@@ -67,6 +67,7 @@ public class TransferConfig {
 
 	private final boolean checkReceivedObjects;
 	private final boolean allowLeadingZeroFileMode;
+	private final boolean allowInvalidPersonIdent;
 	private final boolean safeForWindows;
 	private final boolean safeForMacOS;
 	private final boolean allowTipSha1InWant;
@@ -82,6 +83,8 @@ public class TransferConfig {
 				rc.getBoolean("transfer", "fsckobjects", false)); //$NON-NLS-1$ //$NON-NLS-2$
 		allowLeadingZeroFileMode = checkReceivedObjects
 				&& rc.getBoolean("fsck", "allowLeadingZeroFileMode", false); //$NON-NLS-1$ //$NON-NLS-2$
+		allowInvalidPersonIdent = checkReceivedObjects
+				&& rc.getBoolean("fsck", "allowInvalidPersonIdent", false); //$NON-NLS-1$ //$NON-NLS-2$
 		safeForWindows = checkReceivedObjects
 				&& rc.getBoolean("fsck", "safeForWindows", //$NON-NLS-1$ //$NON-NLS-2$
 						SystemReader.getInstance().isWindows());
@@ -113,6 +116,7 @@ public class TransferConfig {
 			return null;
 		return new ObjectChecker()
 			.setAllowLeadingZeroFileMode(allowLeadingZeroFileMode)
+			.setAllowInvalidPersonIdent(allowInvalidPersonIdent)
 			.setSafeForWindows(safeForWindows)
 			.setSafeForMacOS(safeForMacOS);
 	}