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>

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/DirCacheCheckoutMaliciousPathTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/DirCacheCheckoutMaliciousPathTest.java
index b3219cddb5d1c4708a3a6d9c0eb12a27909ab4fe..d29a75ef710a405229d1fdf51574ba464f4d59c7 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/DirCacheCheckoutMaliciousPathTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/DirCacheCheckoutMaliciousPathTest.java
@@ -405,7 +405,7 @@ public class DirCacheCheckoutMaliciousPathTest extends RepositoryTestCase {
                } catch (InvalidPathException e) {
                        if (good)
                                throw e;
-                       assertTrue(e.getMessage().startsWith("Invalid path: "));
+                       assertTrue(e.getMessage().startsWith("Invalid path"));
                }
        }
 

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 474b8f349c705eada0cc4377c69f233fdc562582..bb67c127a797b55d6e723d87634f44920c5ca985 100644 (file)
--- 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 103d6183b071dd062e3349a06e5ef38a44d6d075..f8c8442ff860c7d33dab7ce0b44751fae8a33335 100644 (file)
--- 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 698636c8cf9464e61bf1bba0fa065fff9a0bf0b6..50d1c4ca387ac63fe4c9f1f57695e6cd61fe8e98 100644 (file)
--- 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 b2c27a483c8d0b14fed144f18ceba882ed03fd99..f9700a1ff41bd9ff692febbd9719b10af5d1ebe5 100644 (file)
--- 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;