More helpful InvalidPathException messages (include reason)

Instead of just a generic "Invalid path: $path", add a reason for the
cases where it's not obvious what the problem is (e.g. "aux" being
reserved on Windows).

Bug: 413915
Change-Id: Ia6436bd2560e4f049c92d9aac907cb87348605e0
Signed-off-by: Robin Stocker <robin@nibor.org>

4 files changed, 51 insertions, 23 deletions

diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
index 474b8f349c..bb67c127a7 100644
--- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
+++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
@@ -270,6 +270,10 @@ invalidObject=Invalid {0} {1}:{2}
 invalidOldIdSent=invalid old id sent
 invalidPacketLineHeader=Invalid packet line header: {0}
 invalidPath=Invalid path: {0}
+invalidPathContainsSeparator=Invalid path (contains separator ''{0}''): {1}
+invalidPathPeriodAtEndWindows=Invalid path (period at end is ignored by Windows): {0}
+invalidPathSpaceAtEndWindows=Invalid path (space at end is ignored by Windows): {0}
+invalidPathReservedOnWindows=Invalid path (''{0}'' is reserved on Windows): {1}
 invalidReflogRevision=Invalid reflog revision: {0}
 invalidRefName=Invalid ref name: {0}
 invalidRemote=Invalid remote: {0}
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 103d6183b0..f8c8442ff8 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java
@@ -76,6 +76,7 @@ import org.eclipse.jgit.treewalk.WorkingTreeOptions;
 import org.eclipse.jgit.treewalk.filter.PathFilter;
 import org.eclipse.jgit.util.FS;
 import org.eclipse.jgit.util.FileUtils;
+import org.eclipse.jgit.util.RawParseUtils;
 import org.eclipse.jgit.util.SystemReader;
 import org.eclipse.jgit.util.io.AutoCRLFOutputStream;
 
@@ -306,8 +307,7 @@ public class DirCacheCheckout {
 	void processEntry(CanonicalTreeParser m, DirCacheBuildIterator i,
 			WorkingTreeIterator f) throws IOException {
 		if (m != null) {
-			if (!isValidPath(m))
-				throw new InvalidPathException(m.getEntryPathString());
+			checkValidPath(m);
 			// There is an entry in the merge commit. Means: we want to update
 			// what's currently in the index and working-tree to that one
 			if (i == null) {
@@ -522,8 +522,8 @@ public class DirCacheCheckout {
 
 		String name = walk.getPathString();
 
-		if (m != null && !isValidPath(m))
-			throw new InvalidPathException(m.getEntryPathString());
+		if (m != null)
+			checkValidPath(m);
 
 		if (i == null && m == null && h == null) {
 			// File/Directory conflict case #20
@@ -1157,14 +1157,14 @@ public class DirCacheCheckout {
 			forbidden[i] = Constants.encodeASCII(list[i]);
 	}
 
-	private static boolean isValidPath(CanonicalTreeParser t) {
+	private static void checkValidPath(CanonicalTreeParser t)
+			throws InvalidPathException {
 		for (CanonicalTreeParser i = t; i != null; i = i.getParent())
-			if (!isValidPathSegment(i))
-				return false;
-		return true;
+			checkValidPathSegment(i);
 	}
 
-	private static boolean isValidPathSegment(CanonicalTreeParser t) {
+	private static void checkValidPathSegment(CanonicalTreeParser t)
+			throws InvalidPathException {
 		boolean isWindows = SystemReader.getInstance().isWindows();
 		boolean isOSX = SystemReader.getInstance().isMacOS();
 		boolean ignCase = isOSX || isWindows;
@@ -1177,23 +1177,29 @@ public class DirCacheCheckout {
 		int start = ptr;
 		while (ptr < end) {
 			if (raw[ptr] == '/')
-				return false;
+				throw new InvalidPathException(
+						JGitText.get().invalidPathContainsSeparator,
+						"/", t.getEntryPathString()); //$NON-NLS-1$
 			if (isWindows) {
 				if (raw[ptr] == '\\')
-					return false;
+					throw new InvalidPathException(
+							JGitText.get().invalidPathContainsSeparator,
+							"\\", t.getEntryPathString()); //$NON-NLS-1$
 				if (raw[ptr] == ':')
-					return false;
+					throw new InvalidPathException(
+							JGitText.get().invalidPathContainsSeparator,
+							":", t.getEntryPathString()); //$NON-NLS-1$
 			}
 			ptr++;
 		}
-		// '.' and '.'' are invalid here
+		// '.' and '..' are invalid here
 		if (ptr - start == 1) {
 			if (raw[start] == '.')
-				return false;
+				throw new InvalidPathException(t.getEntryPathString());
 		} else if (ptr - start == 2) {
 			if (raw[start] == '.')
 				if (raw[start + 1] == '.')
-					return false;
+					throw new InvalidPathException(t.getEntryPathString());
 		} else if (ptr - start == 4) {
 			// .git (possibly case insensitive) is disallowed
 			if (raw[start] == '.')
@@ -1202,15 +1208,24 @@ public class DirCacheCheckout {
 							|| (ignCase && raw[start + 2] == 'I'))
 						if (raw[start + 3] == 't'
 								|| (ignCase && raw[start + 3] == 'T'))
-							return false;
+							throw new InvalidPathException(
+									t.getEntryPathString());
 		}
 		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] == '.' || raw[ptr - 1] == ' ')
-					return false;
+			if (ptr > 0) {
+				if (raw[ptr - 1] == '.')
+					throw new InvalidPathException(
+							JGitText.get().invalidPathPeriodAtEndWindows,
+							t.getEntryPathString());
+				if (raw[ptr - 1] == ' ')
+					throw new InvalidPathException(
+							JGitText.get().invalidPathSpaceAtEndWindows,
+							t.getEntryPathString());
+			}
+
 			int i;
 			// Bad names, eliminate suffix first
 			for (i = start; i < ptr; ++i)
@@ -1228,13 +1243,14 @@ public class DirCacheCheckout {
 								break;
 						}
 						if (k == len)
-							return false;
+							throw new InvalidPathException(
+									JGitText.get().invalidPathReservedOnWindows,
+									RawParseUtils.decode(forbidden[j]), t
+											.getEntryPathString());
 					}
 				}
 			}
 		}
-
-		return true;
 	}
 
 	private static byte toUpper(byte b) {
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/InvalidPathException.java b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/InvalidPathException.java
index 698636c8cf..50d1c4ca38 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/InvalidPathException.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/InvalidPathException.java
@@ -60,6 +60,10 @@ public class InvalidPathException extends IllegalArgumentException {
 	 * @param path
 	 */
 	public InvalidPathException(String path) {
-		super(MessageFormat.format(JGitText.get().invalidPath, path));
+		this(JGitText.get().invalidPath, path);
+	}
+
+	InvalidPathException(String messagePattern, Object... arguments) {
+		super(MessageFormat.format(messagePattern, arguments));
 	}
 }
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
index b2c27a483c..f9700a1ff4 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
@@ -332,6 +332,10 @@ public class JGitText extends TranslationBundle {
 	/***/ public String invalidOldIdSent;
 	/***/ public String invalidPacketLineHeader;
 	/***/ public String invalidPath;
+	/***/ public String invalidPathContainsSeparator;
+	/***/ public String invalidPathPeriodAtEndWindows;
+	/***/ public String invalidPathSpaceAtEndWindows;
+	/***/ public String invalidPathReservedOnWindows;
 	/***/ public String invalidReflogRevision;
 	/***/ public String invalidRefName;
 	/***/ public String invalidRemote;