From 2f1bd3618da20886b12b8132e9307bc61f5fe039 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Tue, 11 Mar 2014 17:19:37 -0700 Subject: [PATCH 01/12] Permit ObjectChecker to optionally accept leading '0' in trees The leading '0' is a broken mode that although incorrect in the Git canonical tree format was created by a couple of libraries frequently used on a popular Git hosting site. Some projects have these modes stuck in their ancient history and cannot easily repair the damage without a full history rewrite. Optionally permit ObjectChecker to ignore them. Bug: 307291 Change-Id: Ib921dfd77ce757e89280d1c00328a88430daef35 --- .../eclipse/jgit/lib/ObjectCheckerTest.java | 8 +++++++ .../org/eclipse/jgit/lib/ObjectChecker.java | 21 ++++++++++++++++++- 2 files changed, 28 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 380defaa0..4a4e349cf 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 @@ -1108,6 +1108,14 @@ public class ObjectCheckerTest { checker.checkTree(data); } + @Test + public void testAcceptTreeModeWithZero() throws CorruptObjectException { + StringBuilder b = new StringBuilder(); + entry(b, "040000 a"); + checker.setAllowLeadingZeroFileMode(true); + checker.checkTree(Constants.encodeASCII(b.toString())); + } + @Test public void testInvalidTreeModeStartsWithZero1() { final StringBuilder b = new StringBuilder(); 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 bb67befae..14cb52898 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectChecker.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectChecker.java @@ -97,6 +97,25 @@ public class ObjectChecker { private final MutableInteger ptrout = new MutableInteger(); + private boolean allowZeroMode; + + /** + * Enable accepting leading zero mode in tree entries. + *

+ * Some broken Git libraries generated leading zeros in the mode part of + * tree entries. This is technically incorrect but gracefully allowed by + * git-core. JGit rejects such trees by default, but may need to accept + * them on broken histories. + * + * @param allow allow leading zero mode. + * @return {@code this}. + * @since 3.4 + */ + public ObjectChecker setAllowLeadingZeroFileMode(boolean allow) { + allowZeroMode = allow; + return this; + } + /** * Check an object for parsing errors. * @@ -308,7 +327,7 @@ public class ObjectChecker { break; if (c < '0' || c > '7') throw new CorruptObjectException("invalid mode character"); - if (thisMode == 0 && c == '0') + if (thisMode == 0 && c == '0' && !allowZeroMode) throw new CorruptObjectException("mode starts with '0'"); thisMode <<= 3; thisMode += c - '0'; From 77cd1c8cdccffa9b9cef0ec620fbfc91691bab24 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Tue, 11 Mar 2014 17:42:01 -0700 Subject: [PATCH 02/12] Extract path segment check function in ObjectChecker Start pulling out the path segment checking. This will be used later to support DirCacheCheckout verification of paths, after folding that logic into this location. Change-Id: I66eaee5c988eb7d425fb7a708ef6f5419ab77348 --- .../org/eclipse/jgit/lib/ObjectChecker.java | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) 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 14cb52898..6e470c9b2 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectChecker.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectChecker.java @@ -346,15 +346,7 @@ public class ObjectChecker { if (c == '/') throw new CorruptObjectException("name contains '/'"); } - if (thisNameB + 1 == ptr) - throw new CorruptObjectException("zero length name"); - if (raw[thisNameB] == '.') { - final int nameLen = (ptr - 1) - thisNameB; - if (nameLen == 1) - throw new CorruptObjectException("invalid name '.'"); - if (nameLen == 2 && raw[thisNameB + 1] == '.') - throw new CorruptObjectException("invalid name '..'"); - } + checkPathSegment(raw, thisNameB, ptr - 1); if (duplicateName(raw, thisNameB, ptr - 1)) throw new CorruptObjectException("duplicate entry names"); @@ -375,6 +367,19 @@ public class ObjectChecker { } } + private static void checkPathSegment(byte[] raw, int ptr, int end) + throws CorruptObjectException { + if (ptr == end) + throw new CorruptObjectException("zero length name"); + if (raw[ptr] == '.') { + int nameLen = end - ptr; + if (nameLen == 1) + throw new CorruptObjectException("invalid name '.'"); + if (nameLen == 2 && raw[ptr + 1] == '.') + throw new CorruptObjectException("invalid name '..'"); + } + } + /** * Check a blob for errors. * From 09f513cb37bcc0f2fba803acacfed66e527c68ba Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Tue, 11 Mar 2014 20:12:07 -0700 Subject: [PATCH 03/12] Reject '.git' as a tree name in ObjectChecker Using .git as a name in a tree is invalid for most Git repositories. This can confuse clients into thinking there is a submodule or another repository deeper in the tree, which is incorrect. Change-Id: I90a1eaf25d45e91557f3f548b69cdcd8f7cddce1 --- .../eclipse/jgit/lib/ObjectCheckerTest.java | 13 ++++++++++++ .../org/eclipse/jgit/lib/ObjectChecker.java | 21 +++++++++++++++---- 2 files changed, 30 insertions(+), 4 deletions(-) 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 4a4e349cf..e3509ae31 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 @@ -1272,6 +1272,19 @@ public class ObjectCheckerTest { } } + @Test + public void testInvalidTreeNameIsGit() { + StringBuilder b = new StringBuilder(); + entry(b, "100644 .git"); + byte[] data = Constants.encodeASCII(b.toString()); + try { + checker.checkTree(data); + fail("incorrectly accepted an invalid tree"); + } catch (CorruptObjectException e) { + assertEquals("invalid name '.git'", e.getMessage()); + } + } + @Test public void testInvalidTreeTruncatedInName() { final StringBuilder b = new StringBuilder(); 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 6e470c9b2..6e8188248 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectChecker.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectChecker.java @@ -53,6 +53,7 @@ import java.text.MessageFormat; import org.eclipse.jgit.errors.CorruptObjectException; import org.eclipse.jgit.internal.JGitText; import org.eclipse.jgit.util.MutableInteger; +import org.eclipse.jgit.util.RawParseUtils; /** * Verifies that an object is formatted correctly. @@ -372,14 +373,26 @@ public class ObjectChecker { if (ptr == end) throw new CorruptObjectException("zero length name"); if (raw[ptr] == '.') { - int nameLen = end - ptr; - if (nameLen == 1) + switch (end - ptr) { + case 1: throw new CorruptObjectException("invalid name '.'"); - if (nameLen == 2 && raw[ptr + 1] == '.') - throw new CorruptObjectException("invalid name '..'"); + case 2: + if (raw[ptr + 1] == '.') + throw new CorruptObjectException("invalid name '..'"); + break; + case 4: + if (isDotGit(raw, ptr + 1)) + throw new CorruptObjectException(String.format( + "invalid name '%s'", + RawParseUtils.decode(raw, ptr, end))); + } } } + private static boolean isDotGit(byte[] buf, int p) { + return buf[p] == 'g' && buf[p + 1] == 'i' && buf[p + 2] == 't'; + } + /** * Check a blob for errors. * From 5019471ccb5f6283c0bbde6f697631f928fea987 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Tue, 11 Mar 2014 21:19:23 -0700 Subject: [PATCH 04/12] Allow an ObjectChecker to reject special characters for Windows Repositories that are frequently checked out on Windows platforms may need to ensure trees do not contain strange names that cause problems on those systems. Follow the MSDN guidelines and refuse to accept a tree containing a special character, or names that end with " " (space) or "." (dot). Since Windows filesystems are usually case insensitive, also reject mixed case versions of the reserved ".git" name. Change-Id: Ic3042444b1e162c6d01b88c7e6ea39b2a73c4eca --- .../eclipse/jgit/lib/ObjectCheckerTest.java | 85 +++++++++++++++++++ .../org/eclipse/jgit/lib/ObjectChecker.java | 57 ++++++++++++- 2 files changed, 140 insertions(+), 2 deletions(-) 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 e3509ae31..3e9195eea 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 @@ -1034,6 +1034,13 @@ public class ObjectCheckerTest { checker.checkTree(data); } + @Test + public void testValidPosixTree() throws CorruptObjectException { + checkOneName("ac:d|e"); + checkOneName("test "); + checkOneName("test."); + } + @Test public void testValidTreeSorting1() throws CorruptObjectException { final StringBuilder b = new StringBuilder(); @@ -1285,6 +1292,20 @@ public class ObjectCheckerTest { } } + @Test + public void testInvalidTreeNameIsMixedCaseGit() { + StringBuilder b = new StringBuilder(); + entry(b, "100644 .GiT"); + byte[] data = Constants.encodeASCII(b.toString()); + try { + checker.setSafeForWindows(true); + checker.checkTree(data); + fail("incorrectly accepted an invalid tree"); + } catch (CorruptObjectException e) { + assertEquals("invalid name '.GiT'", e.getMessage()); + } + } + @Test public void testInvalidTreeTruncatedInName() { final StringBuilder b = new StringBuilder(); @@ -1413,6 +1434,70 @@ public class ObjectCheckerTest { } } + @Test + public void testRejectSpaceAtEndOnWindows() { + checker.setSafeForWindows(true); + try { + checkOneName("test "); + fail("incorrectly accepted space at end"); + } catch (CorruptObjectException e) { + assertEquals("invalid name ends with ' '", e.getMessage()); + } + } + + @Test + public void testRejectDotAtEndOnWindows() { + checker.setSafeForWindows(true); + try { + checkOneName("test."); + fail("incorrectly accepted dot at end"); + } catch (CorruptObjectException e) { + assertEquals("invalid name ends with '.'", e.getMessage()); + } + } + + @Test + public void testRejectInvalidWindowsCharacters() { + checker.setSafeForWindows(true); + rejectName('<'); + rejectName('>'); + rejectName(':'); + rejectName('"'); + rejectName('/'); + rejectName('\\'); + rejectName('|'); + rejectName('?'); + rejectName('*'); + + for (int i = 1; i <= 31; i++) + rejectName((byte) i); + } + + private void rejectName(char c) { + try { + checkOneName("te" + c + "st"); + fail("incorrectly accepted with " + c); + } catch (CorruptObjectException e) { + assertEquals("name contains '" + c + "'", e.getMessage()); + } + } + + private void rejectName(byte c) { + String h = Integer.toHexString(c); + try { + checkOneName("te" + ((char) c) + "st"); + fail("incorrectly accepted with 0x" + h); + } catch (CorruptObjectException e) { + assertEquals("name contains byte 0x" + h, e.getMessage()); + } + } + + private void checkOneName(String name) throws CorruptObjectException { + StringBuilder b = new StringBuilder(); + entry(b, "100644 " + name); + checker.checkTree(Constants.encodeASCII(b.toString())); + } + private static void entry(final StringBuilder b, final String modeName) { b.append(modeName); b.append('\0'); 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 6e8188248..39f071c98 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectChecker.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectChecker.java @@ -99,6 +99,7 @@ public class ObjectChecker { private final MutableInteger ptrout = new MutableInteger(); private boolean allowZeroMode; + private boolean windows; /** * Enable accepting leading zero mode in tree entries. @@ -117,6 +118,20 @@ public class ObjectChecker { return this; } + /** + * Restrict trees to only names legal on Windows platforms. + *

+ * Also rejects any mixed case forms of reserved names ({@code .git}). + * + * @param win true if Windows name checking should be performed. + * @return {@code this}. + * @since 3.4 + */ + public ObjectChecker setSafeForWindows(boolean win) { + windows = win; + return this; + } + /** * Check an object for parsing errors. * @@ -346,6 +361,13 @@ public class ObjectChecker { break; if (c == '/') throw new CorruptObjectException("name contains '/'"); + if (windows && isInvalidOnWindows(c)) { + if (c > 31) + throw new CorruptObjectException(String.format( + "name contains '%c'", c)); + throw new CorruptObjectException(String.format( + "name contains byte 0x%x", c & 0xff)); + } } checkPathSegment(raw, thisNameB, ptr - 1); if (duplicateName(raw, thisNameB, ptr - 1)) @@ -368,7 +390,7 @@ public class ObjectChecker { } } - private static void checkPathSegment(byte[] raw, int ptr, int end) + private void checkPathSegment(byte[] raw, int ptr, int end) throws CorruptObjectException { if (ptr == end) throw new CorruptObjectException("zero length name"); @@ -387,12 +409,43 @@ public class ObjectChecker { RawParseUtils.decode(raw, ptr, end))); } } + + // Windows ignores space and dot at end of file name. + if (windows && (raw[end - 1] == ' ' || raw[end - 1] == '.')) + throw new CorruptObjectException("invalid name ends with '" + + ((char) raw[end - 1]) + "'"); + } + + private static boolean isInvalidOnWindows(byte c) { + // Windows disallows "special" characters in a path component. + switch (c) { + case '"': + case '*': + case ':': + case '<': + case '>': + case '?': + case '\\': + case '|': + return true; + } + return 1 <= c && c <= 31; } - private static boolean isDotGit(byte[] buf, int p) { + private boolean isDotGit(byte[] buf, int p) { + if (windows) + return toLower(buf[p]) == 'g' + && toLower(buf[p + 1]) == 'i' + && toLower(buf[p + 2]) == 't'; return buf[p] == 'g' && buf[p + 1] == 'i' && buf[p + 2] == 't'; } + private static char toLower(byte b) { + if ('A' <= b && b <= 'Z') + return (char) (b + ('a' - 'A')); + return (char) b; + } + /** * Check a blob for errors. * From ba0f89b4211fd57fe52120ec9a7c3cbaadbadd3b Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Tue, 11 Mar 2014 22:49:36 -0700 Subject: [PATCH 05/12] Reject special Windows device names in ObjectChecker If Windows rejection is enabled reject special device names like NUL and PRN, including NUL.txt. This prevents a tree that might be used on a Windows client from referencing a confusing name. Change-Id: Ic700ea8fa68724509e0357d4b758a41178c4d70c --- .../eclipse/jgit/lib/ObjectCheckerTest.java | 24 +++++++ .../org/eclipse/jgit/lib/ObjectChecker.java | 70 +++++++++++++++++-- 2 files changed, 90 insertions(+), 4 deletions(-) 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 3e9195eea..d434c852e 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 @@ -1039,6 +1039,7 @@ public class ObjectCheckerTest { checkOneName("ac:d|e"); checkOneName("test "); checkOneName("test."); + checkOneName("NUL"); } @Test @@ -1456,6 +1457,29 @@ public class ObjectCheckerTest { } } + @Test + public void testRejectDevicesOnWindows() { + checker.setSafeForWindows(true); + + String[] bad = { "CON", "PRN", "AUX", "NUL", "COM1", "COM2", "COM3", + "COM4", "COM5", "COM6", "COM7", "COM8", "COM9", "LPT1", "LPT2", + "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9" }; + for (String b : bad) { + try { + checkOneName(b); + fail("incorrectly accepted " + b); + } catch (CorruptObjectException e) { + assertEquals("invalid name '" + b + "'", e.getMessage()); + } + try { + checkOneName(b + ".txt"); + fail("incorrectly accepted " + b + ".txt"); + } catch (CorruptObjectException e) { + assertEquals("invalid name '" + b + "'", e.getMessage()); + } + } + } + @Test public void testRejectInvalidWindowsCharacters() { checker.setSafeForWindows(true); 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 39f071c98..260a643e6 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectChecker.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectChecker.java @@ -410,10 +410,68 @@ public class ObjectChecker { } } - // Windows ignores space and dot at end of file name. - if (windows && (raw[end - 1] == ' ' || raw[end - 1] == '.')) - throw new CorruptObjectException("invalid name ends with '" - + ((char) raw[end - 1]) + "'"); + if (windows) { + // Windows ignores space and dot at end of file name. + if (raw[end - 1] == ' ' || raw[end - 1] == '.') + throw new CorruptObjectException("invalid name ends with '" + + ((char) raw[end - 1]) + "'"); + if (end - ptr >= 3) + checkNotWindowsDevice(raw, ptr, end); + } + } + + private static void checkNotWindowsDevice(byte[] raw, int ptr, int end) + throws CorruptObjectException { + switch (toLower(raw[ptr])) { + case 'a': // AUX + if (end - ptr >= 3 + && toLower(raw[ptr + 1]) == 'u' + && toLower(raw[ptr + 2]) == 'x' + && (end - ptr == 3 || raw[ptr + 3] == '.')) + throw new CorruptObjectException("invalid name 'AUX'"); + break; + + case 'c': // CON, COM[1-9] + if (end - ptr >= 3 + && toLower(raw[ptr + 2]) == 'n' + && toLower(raw[ptr + 1]) == 'o' + && (end - ptr == 3 || raw[ptr + 3] == '.')) + throw new CorruptObjectException("invalid name 'CON'"); + if (end - ptr >= 4 + && toLower(raw[ptr + 2]) == 'm' + && toLower(raw[ptr + 1]) == 'o' + && isPositiveDigit(raw[ptr + 3]) + && (end - ptr == 4 || raw[ptr + 4] == '.')) + throw new CorruptObjectException("invalid name 'COM" + + ((char) raw[ptr + 3]) + "'"); + break; + + case 'l': // LPT[1-9] + if (end - ptr >= 4 + && toLower(raw[ptr + 1]) == 'p' + && toLower(raw[ptr + 2]) == 't' + && isPositiveDigit(raw[ptr + 3]) + && (end - ptr == 4 || raw[ptr + 4] == '.')) + throw new CorruptObjectException("invalid name 'LPT" + + ((char) raw[ptr + 3]) + "'"); + break; + + case 'n': // NUL + if (end - ptr >= 3 + && toLower(raw[ptr + 1]) == 'u' + && toLower(raw[ptr + 2]) == 'l' + && (end - ptr == 3 || raw[ptr + 3] == '.')) + throw new CorruptObjectException("invalid name 'NUL'"); + break; + + case 'p': // PRN + if (end - ptr >= 3 + && toLower(raw[ptr + 1]) == 'r' + && toLower(raw[ptr + 2]) == 'n' + && (end - ptr == 3 || raw[ptr + 3] == '.')) + throw new CorruptObjectException("invalid name 'PRN'"); + break; + } } private static boolean isInvalidOnWindows(byte c) { @@ -446,6 +504,10 @@ public class ObjectChecker { return (char) b; } + private static boolean isPositiveDigit(byte b) { + return '1' <= b && b <= '9'; + } + /** * Check a blob for errors. * From ed3879e38964c8d41068c1171161bad2be0d3500 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Wed, 12 Mar 2014 12:38:13 -0700 Subject: [PATCH 06/12] Reject mixed case .git on Mac OS in ObjectChecker Most Mac OS X systems use a case insensitive HFS+ volume. Like Windows ".git" and ".GIT" are the same path and can confuse a Git program into expecting a repository where one does not exist. Change-Id: Iec6ce9e6c2872f8b0850cc6aec023fa0fcb05ae4 --- .../eclipse/jgit/lib/ObjectCheckerTest.java | 16 +++++++++++++++- .../org/eclipse/jgit/lib/ObjectChecker.java | 18 +++++++++++++++++- 2 files changed, 32 insertions(+), 2 deletions(-) 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 d434c852e..8cdb0ae35 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 @@ -1294,7 +1294,7 @@ public class ObjectCheckerTest { } @Test - public void testInvalidTreeNameIsMixedCaseGit() { + public void testInvalidTreeNameIsMixedCaseGitWindows() { StringBuilder b = new StringBuilder(); entry(b, "100644 .GiT"); byte[] data = Constants.encodeASCII(b.toString()); @@ -1307,6 +1307,20 @@ public class ObjectCheckerTest { } } + @Test + public void testInvalidTreeNameIsMixedCaseGitMacOS() { + StringBuilder b = new StringBuilder(); + entry(b, "100644 .GiT"); + byte[] data = Constants.encodeASCII(b.toString()); + try { + checker.setSafeForMacOS(true); + checker.checkTree(data); + fail("incorrectly accepted an invalid tree"); + } catch (CorruptObjectException e) { + assertEquals("invalid name '.GiT'", e.getMessage()); + } + } + @Test public void testInvalidTreeTruncatedInName() { final StringBuilder b = new StringBuilder(); 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 260a643e6..dd430024b 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectChecker.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectChecker.java @@ -100,6 +100,7 @@ public class ObjectChecker { private boolean allowZeroMode; private boolean windows; + private boolean macosx; /** * Enable accepting leading zero mode in tree entries. @@ -132,6 +133,21 @@ public class ObjectChecker { return this; } + /** + * Restrict trees to only names legal on Mac OS X platforms. + *

+ * Rejects any mixed case forms of reserved names ({@code .git}) + * for users working on HFS+ in case-insensitive (default) mode. + * + * @param mac true if Mac OS X name checking should be performed. + * @return {@code this}. + * @since 3.4 + */ + public ObjectChecker setSafeForMacOS(boolean mac) { + macosx = mac; + return this; + } + /** * Check an object for parsing errors. * @@ -491,7 +507,7 @@ public class ObjectChecker { } private boolean isDotGit(byte[] buf, int p) { - if (windows) + if (windows || macosx) return toLower(buf[p]) == 'g' && toLower(buf[p + 1]) == 'i' && toLower(buf[p + 2]) == 't'; From e2f63788470a1cdd2d591505a00dd13ff1bf6a34 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Wed, 12 Mar 2014 12:01:56 -0700 Subject: [PATCH 07/12] Change DirCacheCheckout to verify path using ObjectChecker Reuse the generic logic in ObjectChecker to examine paths. This required extracting the scanner loop to check for bad characters within the path name segment. Change-Id: I02e964d114fb544a0c1657790d5367c3a2b09dff --- .../eclipse/jgit/lib/ObjectCheckerTest.java | 10 ++ .../jgit/dircache/DirCacheCheckout.java | 151 ++++-------------- .../org/eclipse/jgit/lib/ObjectChecker.java | 67 +++++--- 3 files changed, 83 insertions(+), 145 deletions(-) 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 8cdb0ae35..06745650c 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 @@ -1449,6 +1449,16 @@ public class ObjectCheckerTest { } } + @Test + public void testRejectNulInPathSegment() { + try { + checker.checkPathSegment(Constants.encodeASCII("a\u0000b"), 0, 3); + fail("incorrectly accepted NUL in middle of name"); + } catch (CorruptObjectException e) { + assertEquals("name contains byte 0x00", e.getMessage()); + } + } + @Test public void testRejectSpaceAtEndOnWindows() { checker.setSafeForWindows(true); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java index 44b427690..5275b4c89 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java @@ -62,6 +62,7 @@ import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.CoreConfig.AutoCRLF; import org.eclipse.jgit.lib.CoreConfig.SymLinks; import org.eclipse.jgit.lib.FileMode; +import org.eclipse.jgit.lib.ObjectChecker; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectLoader; import org.eclipse.jgit.lib.ObjectReader; @@ -1164,26 +1165,13 @@ public class DirCacheCheckout { entry.setLength((int) ol.getSize()); } - private static byte[][] forbidden; - static { - String[] list = getSortedForbiddenFileNames(); - forbidden = new byte[list.length][]; - for (int i = 0; i < list.length; ++i) - forbidden[i] = Constants.encodeASCII(list[i]); - } - - static String[] getSortedForbiddenFileNames() { - String[] list = new String[] { "AUX", "COM1", "COM2", "COM3", "COM4", //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ //$NON-NLS-5$ - "COM5", "COM6", "COM7", "COM8", "COM9", "CON", "LPT1", "LPT2", //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ //$NON-NLS-5$ //$NON-NLS-6$ //$NON-NLS-7$ //$NON-NLS-8$ - "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9", "NUL", //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ //$NON-NLS-5$ //$NON-NLS-6$ //$NON-NLS-7$ //$NON-NLS-8$ - "PRN" }; //$NON-NLS-1$ - return list; - } - private static void checkValidPath(CanonicalTreeParser t) throws InvalidPathException { + ObjectChecker chk = new ObjectChecker() + .setSafeForWindows(SystemReader.getInstance().isWindows()) + .setSafeForMacOS(SystemReader.getInstance().isMacOS()); for (CanonicalTreeParser i = t; i != null; i = i.getParent()) - checkValidPathSegment(i); + checkValidPathSegment(chk, i); } /** @@ -1195,119 +1183,36 @@ public class DirCacheCheckout { * @since 3.3 */ public static void checkValidPath(String path) throws InvalidPathException { - boolean isWindows = SystemReader.getInstance().isWindows(); - boolean isOSX = SystemReader.getInstance().isMacOS(); - boolean ignCase = isOSX || isWindows; + ObjectChecker chk = new ObjectChecker() + .setSafeForWindows(SystemReader.getInstance().isWindows()) + .setSafeForMacOS(SystemReader.getInstance().isMacOS()); byte[] bytes = Constants.encode(path); int segmentStart = 0; - for (int i = 0; i < bytes.length; i++) { - if (bytes[i] == '/') { - checkValidPathSegment(isWindows, ignCase, bytes, segmentStart, - i, path); - segmentStart = i + 1; - } - } - if (segmentStart < bytes.length) - checkValidPathSegment(isWindows, ignCase, bytes, segmentStart, - bytes.length, path); - } - - private static void checkValidPathSegment(CanonicalTreeParser t) - throws InvalidPathException { - boolean isWindows = SystemReader.getInstance().isWindows(); - boolean isOSX = SystemReader.getInstance().isMacOS(); - boolean ignCase = isOSX || isWindows; - - int ptr = t.getNameOffset(); - byte[] raw = t.getEntryPathBuffer(); - int end = ptr + t.getNameLength(); - - checkValidPathSegment(isWindows, ignCase, raw, ptr, end, - t.getEntryPathString()); - } - - private static void checkValidPathSegment(boolean isWindows, - boolean ignCase, byte[] raw, int ptr, int end, String path) { - // Validate path component at this level of the tree - int start = ptr; - while (ptr < end) { - if (raw[ptr] == '/') - throw new InvalidPathException( - JGitText.get().invalidPathContainsSeparator, "/", path); //$NON-NLS-1$ - if (isWindows) { - if (raw[ptr] == '\\') - throw new InvalidPathException( - JGitText.get().invalidPathContainsSeparator, - "\\", path); //$NON-NLS-1$ - if (raw[ptr] == ':') - throw new InvalidPathException( - JGitText.get().invalidPathContainsSeparator, - ":", path); //$NON-NLS-1$ - } - ptr++; - } - // '.' and '..' are invalid here - if (ptr - start == 1) { - if (raw[start] == '.') - throw new InvalidPathException(path); - } else if (ptr - start == 2) { - if (raw[start] == '.') - if (raw[start + 1] == '.') - throw new InvalidPathException(path); - } else if (ptr - start == 4) { - // .git (possibly case insensitive) is disallowed - if (raw[start] == '.') - if (raw[start + 1] == 'g' || (ignCase && raw[start + 1] == 'G')) - if (raw[start + 2] == 'i' - || (ignCase && raw[start + 2] == 'I')) - if (raw[start + 3] == 't' - || (ignCase && raw[start + 3] == 'T')) - throw new InvalidPathException(path); - } - if (isWindows) { - // Space or period at end of file name is ignored by Windows. - // Treat this as a bad path for now. We may want to handle - // this as case insensitivity in the future. - if (ptr > 0) { - if (raw[ptr - 1] == '.') - throw new InvalidPathException( - JGitText.get().invalidPathPeriodAtEndWindows, path); - if (raw[ptr - 1] == ' ') - throw new InvalidPathException( - JGitText.get().invalidPathSpaceAtEndWindows, path); - } - - int i; - // Bad names, eliminate suffix first - for (i = start; i < ptr; ++i) - if (raw[i] == '.') - break; - int len = i - start; - if (len == 3 || len == 4) { - for (int j = 0; j < forbidden.length; ++j) { - if (forbidden[j].length == len) { - if (toUpper(raw[start]) < forbidden[j][0]) - break; - int k; - for (k = 0; k < len; ++k) { - if (toUpper(raw[start + k]) != forbidden[j][k]) - break; - } - if (k == len) - throw new InvalidPathException( - JGitText.get().invalidPathReservedOnWindows, - RawParseUtils.decode(forbidden[j]), path); - } + try { + for (int i = 0; i < bytes.length; i++) { + if (bytes[i] == '/') { + chk.checkPathSegment(bytes, segmentStart, i); + segmentStart = i + 1; } } + chk.checkPathSegment(bytes, segmentStart, bytes.length); + } catch (CorruptObjectException e) { + throw new InvalidPathException(e.getMessage()); } } - private static byte toUpper(byte b) { - if (b >= 'a' && b <= 'z') - return (byte) (b - ('a' - 'A')); - return b; + private static void checkValidPathSegment(ObjectChecker chk, + CanonicalTreeParser t) throws InvalidPathException { + try { + int ptr = t.getNameOffset(); + int end = ptr + t.getNameLength(); + chk.checkPathSegment(t.getEntryPathBuffer(), ptr, end); + } catch (CorruptObjectException err) { + String path = t.getEntryPathString(); + InvalidPathException i = new InvalidPathException(path); + i.initCause(err); + throw i; + } } - } 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 dd430024b..38fd0a1eb 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectChecker.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectChecker.java @@ -369,44 +369,67 @@ public class ObjectChecker { throw new CorruptObjectException("invalid mode " + thisMode); final int thisNameB = ptr; - for (;;) { - if (ptr == sz) - throw new CorruptObjectException("truncated in name"); - final byte c = raw[ptr++]; - if (c == 0) - break; - if (c == '/') - throw new CorruptObjectException("name contains '/'"); - if (windows && isInvalidOnWindows(c)) { - if (c > 31) - throw new CorruptObjectException(String.format( - "name contains '%c'", c)); - throw new CorruptObjectException(String.format( - "name contains byte 0x%x", c & 0xff)); - } - } - checkPathSegment(raw, thisNameB, ptr - 1); - if (duplicateName(raw, thisNameB, ptr - 1)) + ptr = scanPathSegment(raw, ptr, sz); + if (ptr == sz || raw[ptr] != 0) + throw new CorruptObjectException("truncated in name"); + checkPathSegment2(raw, thisNameB, ptr); + if (duplicateName(raw, thisNameB, ptr)) throw new CorruptObjectException("duplicate entry names"); if (lastNameB != 0) { final int cmp = pathCompare(raw, lastNameB, lastNameE, - lastMode, thisNameB, ptr - 1, thisMode); + lastMode, thisNameB, ptr, thisMode); if (cmp > 0) throw new CorruptObjectException("incorrectly sorted"); } lastNameB = thisNameB; - lastNameE = ptr - 1; + lastNameE = ptr; lastMode = thisMode; - ptr += Constants.OBJECT_ID_LENGTH; + ptr += 1 + Constants.OBJECT_ID_LENGTH; if (ptr > sz) throw new CorruptObjectException("truncated in object id"); } } - private void checkPathSegment(byte[] raw, int ptr, int end) + private int scanPathSegment(byte[] raw, int ptr, int end) + throws CorruptObjectException { + for (; ptr < end; ptr++) { + byte c = raw[ptr]; + if (c == 0) + return ptr; + if (c == '/') + throw new CorruptObjectException("name contains '/'"); + if (windows && isInvalidOnWindows(c)) { + if (c > 31) + throw new CorruptObjectException(String.format( + "name contains '%c'", c)); + throw new CorruptObjectException(String.format( + "name contains byte 0x%x", c & 0xff)); + } + } + return ptr; + } + + /** + * Check tree path entry for validity. + * + * @param raw buffer to scan. + * @param ptr offset to first byte of the name. + * @param end offset to one past last byte of name. + * @throws CorruptObjectException name is invalid. + * @since 3.4 + */ + public void checkPathSegment(byte[] raw, int ptr, int end) + throws CorruptObjectException { + int e = scanPathSegment(raw, ptr, end); + if (e < end && raw[e] == 0) + throw new CorruptObjectException("name contains byte 0x00"); + checkPathSegment2(raw, ptr, end); + } + + private void checkPathSegment2(byte[] raw, int ptr, int end) throws CorruptObjectException { if (ptr == end) throw new CorruptObjectException("zero length name"); From 0aa682fc68a80704160102fc07dea5611c010746 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Wed, 12 Mar 2014 13:59:29 -0700 Subject: [PATCH 08/12] Check for duplicate names after folding case in ObjectChecker Mac OS X and Windows filesystems are generally case insensitive and will fold 'a' and 'A' to the same directory entry. If the checker is enforcing safe semantics for these platforms, track all names and look for duplicates after folding case and normalizing to NFC. Change-Id: I170b6f649a72d6ef322b7254943d4c604a8d25b9 --- .../eclipse/jgit/lib/ObjectCheckerTest.java | 56 +++++++++++++++ .../org/eclipse/jgit/lib/ObjectChecker.java | 70 ++++++++++++++++++- 2 files changed, 125 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 06745650c..3f58b7501 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 @@ -49,6 +49,7 @@ import static java.lang.Long.valueOf; import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; +import java.io.UnsupportedEncodingException; import java.text.MessageFormat; import org.eclipse.jgit.errors.CorruptObjectException; @@ -1449,6 +1450,61 @@ public class ObjectCheckerTest { } } + @Test + public void testInvalidTreeDuplicateNames5() + throws UnsupportedEncodingException { + StringBuilder b = new StringBuilder(); + entry(b, "100644 a"); + entry(b, "100644 A"); + byte[] data = b.toString().getBytes("UTF-8"); + try { + checker.setSafeForWindows(true); + checker.checkTree(data); + fail("incorrectly accepted an invalid tree"); + } catch (CorruptObjectException e) { + assertEquals("duplicate entry names", e.getMessage()); + } + } + + @Test + public void testInvalidTreeDuplicateNames6() + throws UnsupportedEncodingException { + StringBuilder b = new StringBuilder(); + entry(b, "100644 a"); + entry(b, "100644 A"); + byte[] data = b.toString().getBytes("UTF-8"); + try { + checker.setSafeForMacOS(true); + checker.checkTree(data); + fail("incorrectly accepted an invalid tree"); + } catch (CorruptObjectException e) { + assertEquals("duplicate entry names", e.getMessage()); + } + } + + @Test + public void testInvalidTreeDuplicateNames7() + throws UnsupportedEncodingException { + try { + Class.forName("java.text.Normalizer"); + } catch (ClassNotFoundException e) { + // Ignore this test on Java 5 platform. + return; + } + + StringBuilder b = new StringBuilder(); + entry(b, "100644 \u00C1"); + entry(b, "100644 \u004a\u0301"); + byte[] data = b.toString().getBytes("UTF-8"); + try { + checker.setSafeForMacOS(true); + checker.checkTree(data); + fail("incorrectly accepted an invalid tree"); + } catch (CorruptObjectException e) { + assertEquals("duplicate entry names", e.getMessage()); + } + } + @Test public void testRejectNulInPathSegment() { try { 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 38fd0a1eb..1b135a924 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectChecker.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectChecker.java @@ -48,7 +48,12 @@ import static org.eclipse.jgit.util.RawParseUtils.match; import static org.eclipse.jgit.util.RawParseUtils.nextLF; import static org.eclipse.jgit.util.RawParseUtils.parseBase10; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; import java.text.MessageFormat; +import java.util.HashSet; +import java.util.Locale; +import java.util.Set; import org.eclipse.jgit.errors.CorruptObjectException; import org.eclipse.jgit.internal.JGitText; @@ -348,6 +353,9 @@ public class ObjectChecker { final int sz = raw.length; int ptr = 0; int lastNameB = 0, lastNameE = 0, lastMode = 0; + Set normalized = windows || macosx + ? new HashSet() + : null; while (ptr < sz) { int thisMode = 0; @@ -373,7 +381,10 @@ public class ObjectChecker { if (ptr == sz || raw[ptr] != 0) throw new CorruptObjectException("truncated in name"); checkPathSegment2(raw, thisNameB, ptr); - if (duplicateName(raw, thisNameB, ptr)) + if (normalized != null) { + if (normalized.add(normalize(raw, thisNameB, ptr))) + throw new CorruptObjectException("duplicate entry names"); + } else if (duplicateName(raw, thisNameB, ptr)) throw new CorruptObjectException("duplicate entry names"); if (lastNameB != 0) { @@ -558,4 +569,61 @@ public class ObjectChecker { public void checkBlob(final byte[] raw) throws CorruptObjectException { // We can always assume the blob is valid. } + + private String normalize(byte[] raw, int ptr, int end) { + String n = RawParseUtils.decode(raw, ptr, end).toLowerCase(Locale.US); + return macosx ? Normalizer.normalize(n) : n; + } + + private static class Normalizer { + // TODO Simplify invocation to Normalizer after dropping Java 5. + private static final Method normalize; + private static final Object nfc; + static { + Method method; + Object formNfc; + try { + Class formClazz = Class.forName("java.text.Normalizer$Form"); //$NON-NLS-1$ + formNfc = formClazz.getField("NFC").get(null); //$NON-NLS-1$ + method = Class.forName("java.text.Normalizer") //$NON-NLS-1$ + .getMethod("normalize", CharSequence.class, formClazz); //$NON-NLS-1$ + } catch (ClassNotFoundException e) { + method = null; + formNfc = null; + } catch (NoSuchFieldException e) { + method = null; + formNfc = null; + } catch (NoSuchMethodException e) { + method = null; + formNfc = null; + } catch (SecurityException e) { + method = null; + formNfc = null; + } catch (IllegalArgumentException e) { + method = null; + formNfc = null; + } catch (IllegalAccessException e) { + method = null; + formNfc = null; + } + normalize = method; + nfc = formNfc; + } + + static String normalize(String in) { + if (normalize == null) + return in; + try { + return (String) normalize.invoke(null, in, nfc); + } catch (IllegalAccessException e) { + return in; + } catch (InvocationTargetException e) { + if (e.getCause() instanceof RuntimeException) + throw (RuntimeException) e.getCause(); + if (e.getCause() instanceof Error) + throw (Error) e.getCause(); + return in; + } + } + } } From d300609495fc52f19535a8d541ec3a848aad3f5d Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Wed, 12 Mar 2014 16:36:07 -0700 Subject: [PATCH 09/12] Allow ReceivePack callers to configure their own ObjectChecker PackParser permits supplying a specific ObjectChecker instance. Allow this to be passed through ReceivePack, giving the caller more flexibility to configure the implementation. Change-Id: I9440dd25588008626222f33bfd697f57c05b439e --- .../jgit/transport/BaseReceivePack.java | 24 +++++++++++++++---- 1 file changed, 19 insertions(+), 5 deletions(-) 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 68b3262a0..483ac55fa 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java @@ -74,6 +74,7 @@ import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.Config.SectionParser; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.NullProgressMonitor; +import org.eclipse.jgit.lib.ObjectChecker; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectIdSubclassMap; import org.eclipse.jgit.lib.ObjectInserter; @@ -160,7 +161,7 @@ public abstract class BaseReceivePack { private boolean expectDataAfterPackFooter; /** Should an incoming transfer validate objects? */ - private boolean checkReceivedObjects; + private ObjectChecker objectChecker; /** Should an incoming transfer permit create requests? */ private boolean allowCreates; @@ -254,7 +255,7 @@ public abstract class BaseReceivePack { walk = new RevWalk(db); final ReceiveConfig cfg = db.getConfig().get(ReceiveConfig.KEY); - checkReceivedObjects = cfg.checkReceivedObjects; + objectChecker = cfg.checkReceivedObjects ? new ObjectChecker() : null; allowCreates = cfg.allowCreates; allowDeletes = cfg.allowDeletes; allowNonFastForwards = cfg.allowNonFastForwards; @@ -481,16 +482,29 @@ public abstract class BaseReceivePack { * of the connection. */ public boolean isCheckReceivedObjects() { - return checkReceivedObjects; + return objectChecker != null; } /** * @param check * true to enable checking received objects; false to assume all * received objects are valid. + * @see #setObjectChecker(ObjectChecker) */ public void setCheckReceivedObjects(final boolean check) { - checkReceivedObjects = check; + if (check && objectChecker == null) + setObjectChecker(new ObjectChecker()); + else if (!check && objectChecker != null) + setObjectChecker(null); + } + + /** + * @param impl if non-null the object checking instance to verify each + * received object with; null to disable object checking. + * @since 3.4 + */ + public void setObjectChecker(ObjectChecker impl) { + objectChecker = impl; } /** @return true if the client can request refs to be created. */ @@ -983,7 +997,7 @@ public abstract class BaseReceivePack { parser.setCheckEofAfterPackFooter(!biDirectionalPipe && !isExpectDataAfterPackFooter()); parser.setExpectDataAfterPackFooter(isExpectDataAfterPackFooter()); - parser.setObjectChecking(isCheckReceivedObjects()); + parser.setObjectChecker(objectChecker); parser.setLockMessage(lockMsg); parser.setMaxObjectSizeLimit(maxObjectSizeLimit); packLock = parser.parse(receiving, resolving); From a1a5218032a8f068fd0ee62d56acdb6251404583 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Wed, 12 Mar 2014 16:44:59 -0700 Subject: [PATCH 10/12] Default receive.fsckObjects to transfer.fsckObjects ReceivePack should configure itself with receive.fsckObjects, and if not defined, transfer.fsckObjects. This is the order used by git-core. Change-Id: I41f243633dacb606dbcc3132972f63bbaba174d1 --- .../src/org/eclipse/jgit/transport/BaseReceivePack.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) 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 483ac55fa..0c83ece29 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java @@ -284,8 +284,9 @@ public abstract class BaseReceivePack { final boolean allowOfsDelta; ReceiveConfig(final Config config) { - checkReceivedObjects = config.getBoolean("receive", "fsckobjects", //$NON-NLS-1$ //$NON-NLS-2$ - false); + checkReceivedObjects = config.getBoolean( + "receive", "fsckobjects", //$NON-NLS-1$ //$NON-NLS-2$ + config.getBoolean("transfer", "fsckobjects", false)); //$NON-NLS-1$ //$NON-NLS-2$ allowCreates = true; allowDeletes = !config.getBoolean("receive", "denydeletes", false); //$NON-NLS-1$ //$NON-NLS-2$ allowNonFastForwards = !config.getBoolean("receive", //$NON-NLS-1$ From ced58a7cff03654024bd9f154cfea2bd9459d39c Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Wed, 12 Mar 2014 16:53:26 -0700 Subject: [PATCH 11/12] Correct fetch to use fetch.fsckObjects and transfer.fsckObjects git-core configures fsck during fetch using these two variables. JGit use of fsck in fetch predates the usage in git-core and had reused receive.fsckobjects, which does not match behavior. Change-Id: Ie4d5f082258c4b2928c7ecc3780c6363fa587f34 --- .../src/org/eclipse/jgit/transport/TransferConfig.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) 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 3a08cd35d..b00d607ee 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransferConfig.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransferConfig.java @@ -63,7 +63,7 @@ public class TransferConfig { } }; - private final boolean fsckObjects; + private final boolean fetchFsck; private final boolean allowTipSha1InWant; private final String[] hideRefs; @@ -72,7 +72,10 @@ public class TransferConfig { } private TransferConfig(final Config rc) { - fsckObjects = rc.getBoolean("receive", "fsckobjects", false); //$NON-NLS-1$ //$NON-NLS-2$ + fetchFsck = rc.getBoolean( + "fetch", "fsckobjects", //$NON-NLS-1$ //$NON-NLS-2$ + rc.getBoolean("transfer", "fsckobjects", false)); //$NON-NLS-1$ //$NON-NLS-2$ + allowTipSha1InWant = rc.getBoolean( "uploadpack", "allowtipsha1inwant", false); //$NON-NLS-1$ //$NON-NLS-2$ hideRefs = rc.getStringList("uploadpack", null, "hiderefs"); //$NON-NLS-1$ //$NON-NLS-2$ @@ -82,7 +85,7 @@ public class TransferConfig { * @return strictly verify received objects? */ public boolean isFsckObjects() { - return fsckObjects; + return fetchFsck; } /** From 3d412827e55fb43bdc2ae7475a3e54c92a55ad5e Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Wed, 12 Mar 2014 17:04:08 -0700 Subject: [PATCH 12/12] Allow configuration of receive pack's ObjectChecker through fsck.* fsck.allowLeadingZeroFileMode may be set true to permit pushing broken trees with leading '0' in the file mode. fsck.safeForWindows may be set true to require new trees to have only file names that are safe on the Windows platform. fsck.safeForMacOS may be set true to require new trees to have only file names that do not cause collisions or confusion on the Mac OS platform. Change-Id: I1a225c1b3cd13c0d1a0d43fffe79355c501f49b7 --- .../jgit/transport/BaseReceivePack.java | 24 +++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) 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 0c83ece29..ff45b1cda 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java @@ -255,7 +255,7 @@ public abstract class BaseReceivePack { walk = new RevWalk(db); final ReceiveConfig cfg = db.getConfig().get(ReceiveConfig.KEY); - objectChecker = cfg.checkReceivedObjects ? new ObjectChecker() : null; + objectChecker = cfg.newObjectChecker(); allowCreates = cfg.allowCreates; allowDeletes = cfg.allowDeletes; allowNonFastForwards = cfg.allowNonFastForwards; @@ -274,19 +274,26 @@ public abstract class BaseReceivePack { }; final boolean checkReceivedObjects; + final boolean allowLeadingZeroFileMode; + final boolean safeForWindows; + final boolean safeForMacOS; final boolean allowCreates; - final boolean allowDeletes; - final boolean allowNonFastForwards; - final boolean allowOfsDelta; ReceiveConfig(final Config config) { checkReceivedObjects = config.getBoolean( "receive", "fsckobjects", //$NON-NLS-1$ //$NON-NLS-2$ config.getBoolean("transfer", "fsckobjects", false)); //$NON-NLS-1$ //$NON-NLS-2$ + allowLeadingZeroFileMode = checkReceivedObjects + && config.getBoolean("fsck", "allowLeadingZeroFileMode", false); //$NON-NLS-1$ //$NON-NLS-2$ + safeForWindows = checkReceivedObjects + && config.getBoolean("fsck", "safeForWindows", false); //$NON-NLS-1$ //$NON-NLS-2$ + safeForMacOS = checkReceivedObjects + && config.getBoolean("fsck", "safeForMacOS", false); //$NON-NLS-1$ //$NON-NLS-2$ + allowCreates = true; allowDeletes = !config.getBoolean("receive", "denydeletes", false); //$NON-NLS-1$ //$NON-NLS-2$ allowNonFastForwards = !config.getBoolean("receive", //$NON-NLS-1$ @@ -294,6 +301,15 @@ public abstract class BaseReceivePack { allowOfsDelta = config.getBoolean("repack", "usedeltabaseoffset", //$NON-NLS-1$ //$NON-NLS-2$ true); } + + ObjectChecker newObjectChecker() { + if (!checkReceivedObjects) + return null; + return new ObjectChecker() + .setAllowLeadingZeroFileMode(allowLeadingZeroFileMode) + .setSafeForWindows(safeForWindows) + .setSafeForMacOS(safeForMacOS); + } } /**