From d547d0c44b061c0d4e59629588b3074d90747e2d Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Tue, 25 Nov 2014 00:47:48 -0800 Subject: [PATCH] DirCache: Replace isValidPath with DirCacheCheckout.checkValidPath isValidPath is an older simple form of the validation performed by checkValidPath. Use the latter as it more consistently matches git-core's validation rules. By running the same validation as fsck, callers creating an entry for the DirCache are more likely to learn early they are trying to build trees that will fail fsck. Change-Id: Ibf5ac116097156aa05c18e231bc65c0854932eb1 --- .../jgit/dircache/DirCacheEntryTest.java | 8 +++-- .../jgit/dircache/DirCacheCheckout.java | 4 ++- .../eclipse/jgit/dircache/DirCacheEntry.java | 34 +------------------ 3 files changed, 10 insertions(+), 36 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/dircache/DirCacheEntryTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/dircache/DirCacheEntryTest.java index 225ce2a90..e159ed939 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/dircache/DirCacheEntryTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/dircache/DirCacheEntryTest.java @@ -49,7 +49,6 @@ import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; -import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.FileMode; import org.eclipse.jgit.lib.ObjectId; import org.junit.Test; @@ -71,7 +70,12 @@ public class DirCacheEntryTest { } private static boolean isValidPath(final String path) { - return DirCacheEntry.isValidPath(Constants.encode(path)); + try { + DirCacheCheckout.checkValidPath(path); + return true; + } catch (InvalidPathException e) { + return false; + } } @Test 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 80dda8eb8..6bf8ed7d0 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java @@ -1274,7 +1274,9 @@ public class DirCacheCheckout { } chk.checkPathSegment(bytes, segmentStart, bytes.length); } catch (CorruptObjectException e) { - throw new InvalidPathException(e.getMessage()); + InvalidPathException p = new InvalidPathException(path); + p.initCause(e); + throw p; } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheEntry.java b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheEntry.java index 458cd98e4..b6b376ae8 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheEntry.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheEntry.java @@ -64,7 +64,6 @@ import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.util.IO; import org.eclipse.jgit.util.MutableInteger; import org.eclipse.jgit.util.NB; -import org.eclipse.jgit.util.SystemReader; /** * A single file (or stage of a file) in a {@link DirCache}. @@ -265,8 +264,7 @@ public class DirCacheEntry { */ @SuppressWarnings("boxing") public DirCacheEntry(final byte[] newPath, final int stage) { - if (!isValidPath(newPath)) - throw new InvalidPathException(toString(newPath)); + DirCacheCheckout.checkValidPath(toString(newPath)); if (stage < 0 || 3 < stage) throw new IllegalArgumentException(MessageFormat.format( JGitText.get().invalidStageForPath, @@ -725,36 +723,6 @@ public class DirCacheEntry { return Constants.CHARSET.decode(ByteBuffer.wrap(path)).toString(); } - static boolean isValidPath(final byte[] path) { - if (path.length == 0) - return false; // empty path is not permitted. - - boolean componentHasChars = false; - for (final byte c : path) { - switch (c) { - case 0: - return false; // NUL is never allowed within the path. - - case '/': - if (componentHasChars) - componentHasChars = false; - else - return false; - break; - case '\\': - case ':': - // Tree's never have a backslash in them, not even on Windows - // but even there we regard it as an invalid path - if (SystemReader.getInstance().isWindows()) - return false; - //$FALL-THROUGH$ - default: - componentHasChars = true; - } - } - return componentHasChars; - } - static int getMaximumInfoLength(boolean extended) { return extended ? INFO_LEN_EXTENDED : INFO_LEN; }