From 4feffb3bf58d9a42be13624ef8ecb1775a40eb8d Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Wed, 29 Apr 2015 21:56:28 -0700 Subject: [PATCH] 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 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 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 --- .../eclipse/jgit/lib/ObjectCheckerTest.java | 41 ++++++++++++++++++- .../org/eclipse/jgit/lib/ObjectChecker.java | 21 ++++++++++ .../jgit/transport/BaseReceivePack.java | 4 ++ .../jgit/transport/TransferConfig.java | 4 ++ 4 files changed, 69 insertions(+), 1 deletion(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ObjectCheckerTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ObjectCheckerTest.java index c6578ccfa..274757d95 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ObjectCheckerTest.java +++ b/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 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 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 diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectChecker.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectChecker.java index 8435c9a64..359b59295 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectChecker.java +++ b/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. + *

+ * 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. *

@@ -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; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java index b7a599bad..cf1d92e8e 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java +++ b/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); } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransferConfig.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransferConfig.java index 1de91a57e..60043fff7 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransferConfig.java +++ b/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); }