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

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);

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();
+		ObjectChecker chk = new ObjectChecker()
-		boolean isOSX = SystemReader.getInstance().isMacOS();
+			.setSafeForWindows(SystemReader.getInstance().isWindows())
-		boolean ignCase = isOSX || isWindows;
+			.setSafeForMacOS(SystemReader.getInstance().isMacOS());
 
 		byte[] bytes = Constants.encode(path);
 		int segmentStart = 0;
+		try {
 			for (int i = 0; i < bytes.length; i++) {
 				if (bytes[i] == '/') {
-				checkValidPathSegment(isWindows, ignCase, bytes, segmentStart,
+					chk.checkPathSegment(bytes, segmentStart, i);
-						i, path);
 					segmentStart = i + 1;
 				}
 			}
-		if (segmentStart < bytes.length)
+			chk.checkPathSegment(bytes, segmentStart, bytes.length);
-			checkValidPathSegment(isWindows, ignCase, bytes, segmentStart,
+		} catch (CorruptObjectException e) {
-					bytes.length, path);
+			throw new InvalidPathException(e.getMessage());
+		}
 	}
 
-	private static void checkValidPathSegment(CanonicalTreeParser t)
+	private static void checkValidPathSegment(ObjectChecker chk,
-			throws InvalidPathException {
+			CanonicalTreeParser t) throws InvalidPathException {
-		boolean isWindows = SystemReader.getInstance().isWindows();
+		try {
-		boolean isOSX = SystemReader.getInstance().isMacOS();
-		boolean ignCase = isOSX || isWindows;
-
 			int ptr = t.getNameOffset();
-		byte[] raw = t.getEntryPathBuffer();
 			int end = ptr + t.getNameLength();
-
+			chk.checkPathSegment(t.getEntryPathBuffer(), ptr, end);
-		checkValidPathSegment(isWindows, ignCase, raw, ptr, end,
+		} catch (CorruptObjectException err) {
-				t.getEntryPathString());
+			String path = t.getEntryPathString();
-	}
+			InvalidPathException i = new InvalidPathException(path);
-
+			i.initCause(err);
-	private static void checkValidPathSegment(boolean isWindows,
+			throw i;
-			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);
-					}
 		}
 	}
-		}
-	}
-
-	private static byte toUpper(byte b) {
-		if (b >= 'a' && b <= 'z')
-			return (byte) (b - ('a' - 'A'));
-		return 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 (;;) {
+			ptr = scanPathSegment(raw, ptr, sz);
-				if (ptr == sz)
+			if (ptr == sz || raw[ptr] != 0)
 				throw new CorruptObjectException("truncated in name");
-				final byte c = raw[ptr++];
+			checkPathSegment2(raw, thisNameB, ptr);
-				if (c == 0)
+			if (duplicateName(raw, thisNameB, ptr))
-					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))
 				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");