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

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 e3509ae31fe70837b05642536cf2528366927cba..3e9195eea3ec8bb19b369c9920eabca92b9460a3 100644 (file)
--- 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("a<b>c: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 6e8188248c68d447555ca1438cb8938d523ac23b..39f071c98edb1c11ca0eeb742399907c95ec6736 100644 (file)
--- 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.
+        * <p>
+        * 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.
         *