Ban dangerous ref names in Windows

Bug: 423551 
Change-Id: I3e71ef1b4a8181f46d2902c9169859f150cd6ad0
Also-By: Robin Stocker <robin@nibor.org>
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

3 files changed, 83 insertions, 17 deletions

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ValidRefNameTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ValidRefNameTest.java
index 08d675bc04..2f8bfb3fdd 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ValidRefNameTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ValidRefNameTest.java
@@ -45,6 +45,8 @@ package org.eclipse.jgit.lib;
 
 import static org.eclipse.jgit.junit.Assert.assertEquals;
 
+import org.eclipse.jgit.junit.MockSystemReader;
+import org.eclipse.jgit.util.SystemReader;
 import org.junit.Test;
 
 public class ValidRefNameTest {
@@ -192,4 +194,25 @@ public class ValidRefNameTest {
 		assertValid(false, "refs/heads/master@{1}");
 		assertValid(false, "refs/heads/master@{1.hour.ago}");
 	}
+
+	@Test
+	public void testWindowsReservedNames() {
+		SystemReader original = SystemReader.getInstance();
+		try {
+			SystemReader.setInstance(new MockSystemReader() {
+				public boolean isWindows() {
+					return true;
+				}
+			});
+			// re-using code from DirCacheCheckoutTest, hence
+			// only testing for one of the special names.
+			assertValid(false, "refs/heads/con");
+			assertValid(false, "refs/con/x");
+			assertValid(false, "con/heads/x");
+			assertValid(true, "refs/heads/conx");
+			assertValid(true, "refs/heads/xcon");
+		} finally {
+			SystemReader.setInstance(original);
+		}
+	}
 }
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 40efc95f88..3f1afd7cc0 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java
@@ -1156,13 +1156,18 @@ public class DirCacheCheckout {
 
 	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$
-		forbidden = new byte[list.length][];
-		for (int i = 0; i < list.length; ++i)
-			forbidden[i] = Constants.encodeASCII(list[i]);
+		return list;
 	}
 
 	private static void checkValidPath(CanonicalTreeParser t)
@@ -1171,6 +1176,33 @@ public class DirCacheCheckout {
 			checkValidPathSegment(i);
 	}
 
+	/**
+	 * Check if path is a valid path for a checked out file name or ref name.
+	 *
+	 * @param path
+	 * @throws InvalidPathException
+	 *             if the path is invalid
+	 * @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;
+
+		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();
@@ -1181,33 +1213,38 @@ public class DirCacheCheckout {
 		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,
-						"/", t.getEntryPathString()); //$NON-NLS-1$
+						JGitText.get().invalidPathContainsSeparator, "/", path); //$NON-NLS-1$
 			if (isWindows) {
 				if (raw[ptr] == '\\')
 					throw new InvalidPathException(
 							JGitText.get().invalidPathContainsSeparator,
-							"\\", t.getEntryPathString()); //$NON-NLS-1$
+							"\\", path); //$NON-NLS-1$
 				if (raw[ptr] == ':')
 					throw new InvalidPathException(
 							JGitText.get().invalidPathContainsSeparator,
-							":", t.getEntryPathString()); //$NON-NLS-1$
+							":", path); //$NON-NLS-1$
 			}
 			ptr++;
 		}
 		// '.' and '..' are invalid here
 		if (ptr - start == 1) {
 			if (raw[start] == '.')
-				throw new InvalidPathException(t.getEntryPathString());
+				throw new InvalidPathException(path);
 		} else if (ptr - start == 2) {
 			if (raw[start] == '.')
 				if (raw[start + 1] == '.')
-					throw new InvalidPathException(t.getEntryPathString());
+					throw new InvalidPathException(path);
 		} else if (ptr - start == 4) {
 			// .git (possibly case insensitive) is disallowed
 			if (raw[start] == '.')
@@ -1216,8 +1253,7 @@ public class DirCacheCheckout {
 							|| (ignCase && raw[start + 2] == 'I'))
 						if (raw[start + 3] == 't'
 								|| (ignCase && raw[start + 3] == 'T'))
-							throw new InvalidPathException(
-									t.getEntryPathString());
+							throw new InvalidPathException(path);
 		}
 		if (isWindows) {
 			// Space or period at end of file name is ignored by Windows.
@@ -1226,12 +1262,10 @@ public class DirCacheCheckout {
 			if (ptr > 0) {
 				if (raw[ptr - 1] == '.')
 					throw new InvalidPathException(
-							JGitText.get().invalidPathPeriodAtEndWindows,
-							t.getEntryPathString());
+							JGitText.get().invalidPathPeriodAtEndWindows, path);
 				if (raw[ptr - 1] == ' ')
 					throw new InvalidPathException(
-							JGitText.get().invalidPathSpaceAtEndWindows,
-							t.getEntryPathString());
+							JGitText.get().invalidPathSpaceAtEndWindows, path);
 			}
 
 			int i;
@@ -1253,8 +1287,7 @@ public class DirCacheCheckout {
 						if (k == len)
 							throw new InvalidPathException(
 									JGitText.get().invalidPathReservedOnWindows,
-									RawParseUtils.decode(forbidden[j]), t
-											.getEntryPathString());
+									RawParseUtils.decode(forbidden[j]), path);
 					}
 				}
 			}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java
index 77734bf6b1..291803ee59 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java
@@ -65,6 +65,8 @@ import java.util.Set;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import org.eclipse.jgit.dircache.DirCache;
+import org.eclipse.jgit.dircache.DirCacheCheckout;
+import org.eclipse.jgit.dircache.InvalidPathException;
 import org.eclipse.jgit.errors.AmbiguousObjectException;
 import org.eclipse.jgit.errors.CorruptObjectException;
 import org.eclipse.jgit.errors.IncorrectObjectTypeException;
@@ -1151,6 +1153,14 @@ public abstract class Repository {
 		if (refName.endsWith(".lock")) //$NON-NLS-1$
 			return false;
 
+		// Borrow logic for filterig out invalid paths. These
+		// are also invalid ref
+		try {
+			DirCacheCheckout.checkValidPath(refName);
+		} catch (InvalidPathException e) {
+			return false;
+		}
+
 		int components = 1;
 		char p = '\0';
 		for (int i = 0; i < len; i++) {