From b6b843e51996141d3329353bc9799f1e761199e1 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Mon, 24 Nov 2014 21:43:59 -0800 Subject: [PATCH] Move checkPath from DirCacheCheckout to ObjectChecker The bulk of the "is this sane" logic is inside of ObjectChecker. The only caller for the version in DirCacheCheckout is an obtuse usage for the static isValidRefName() method in Repository. Deprecate the weird single use method in DirCacheCheckout and move all code for checking a sequence of path components into ObjectChecker, where it makes sense alongside the existing code that checks a single component at a time. Reuse a single ObjectChecker for the local platform, to avoid looking up the system properties on each path string considered. Change-Id: Iae6e769f2bfcad05c166e70ff255f9cf9fcdc87e --- .../jgit/dircache/DirCacheCheckout.java | 19 ++------- .../org/eclipse/jgit/lib/ObjectChecker.java | 39 ++++++++++++++++++ .../src/org/eclipse/jgit/lib/Repository.java | 11 +++-- .../org/eclipse/jgit/util/SystemReader.java | 41 +++++++++++++++++-- 4 files changed, 86 insertions(+), 24 deletions(-) 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 1693cec51c..c7dd03dd46 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java @@ -58,7 +58,6 @@ import org.eclipse.jgit.errors.IncorrectObjectTypeException; import org.eclipse.jgit.errors.IndexWriteException; import org.eclipse.jgit.errors.MissingObjectException; import org.eclipse.jgit.internal.JGitText; -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; @@ -1285,24 +1284,14 @@ public class DirCacheCheckout { * @throws InvalidPathException * if the path is invalid * @since 3.3 + * @deprecated Use {@link SystemReader#checkPath(String)}. */ + @Deprecated public static void checkValidPath(String path) throws InvalidPathException { - ObjectChecker chk = new ObjectChecker() - .setSafeForWindows(SystemReader.getInstance().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] == '/') { - chk.checkPathSegment(bytes, segmentStart, i); - segmentStart = i + 1; - } - } - chk.checkPathSegment(bytes, segmentStart, bytes.length); + SystemReader.getInstance().checkPath(path); } catch (CorruptObjectException e) { - throw new InvalidPathException(e.getMessage()); + throw new InvalidPathException(path); } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectChecker.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectChecker.java index d8a70c104f..9fe54b0c5c 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectChecker.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectChecker.java @@ -423,6 +423,45 @@ public class ObjectChecker { return ptr; } + /** + * Check tree path entry for validity. + *

+ * Unlike {@link #checkPathSegment(byte[], int, int)}, this version + * scans a multi-directory path string such as {@code "src/main.c"}. + * + * @param path path string to scan. + * @throws CorruptObjectException path is invalid. + * @since 3.6 + */ + public void checkPath(String path) throws CorruptObjectException { + byte[] buf = Constants.encode(path); + checkPath(buf, 0, buf.length); + } + + /** + * Check tree path entry for validity. + *

+ * Unlike {@link #checkPathSegment(byte[], int, int)}, this version + * scans a multi-directory path string such as {@code "src/main.c"}. + * + * @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 path is invalid. + * @since 3.6 + */ + public void checkPath(byte[] raw, int ptr, int end) + throws CorruptObjectException { + int start = ptr; + for (; ptr < end; ptr++) { + if (raw[ptr] == '/') { + checkPathSegment(raw, start, ptr); + start = ptr + 1; + } + } + checkPathSegment(raw, start, end); + } + /** * Check tree path entry for validity. * 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 a9a45a2406..6353a5b5f3 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java @@ -65,8 +65,6 @@ 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; @@ -90,6 +88,7 @@ import org.eclipse.jgit.util.FS; import org.eclipse.jgit.util.FileUtils; import org.eclipse.jgit.util.IO; import org.eclipse.jgit.util.RawParseUtils; +import org.eclipse.jgit.util.SystemReader; import org.eclipse.jgit.util.io.SafeBufferedOutputStream; /** @@ -1154,11 +1153,11 @@ public abstract class Repository { if (refName.endsWith(".lock")) //$NON-NLS-1$ return false; - // Borrow logic for filtering out invalid paths. These - // are also invalid ref + // Refs may be stored as loose files so invalid paths + // on the local system must also be invalid refs. try { - DirCacheCheckout.checkValidPath(refName); - } catch (InvalidPathException e) { + SystemReader.getInstance().checkPath(refName); + } catch (CorruptObjectException e) { return false; } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/SystemReader.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/SystemReader.java index e73f100f96..3447f639d4 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/SystemReader.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/SystemReader.java @@ -57,7 +57,9 @@ import java.util.Locale; import java.util.TimeZone; import org.eclipse.jgit.storage.file.FileBasedConfig; +import org.eclipse.jgit.errors.CorruptObjectException; import org.eclipse.jgit.lib.Config; +import org.eclipse.jgit.lib.ObjectChecker; /** * Interface to read values from the system. @@ -68,7 +70,14 @@ import org.eclipse.jgit.lib.Config; *

*/ public abstract class SystemReader { - private static SystemReader DEFAULT = new SystemReader() { + private static final SystemReader DEFAULT; + static { + SystemReader r = new Default(); + r.init(); + DEFAULT = r; + } + + private static class Default extends SystemReader { private volatile String hostname; public String getenv(String variable) { @@ -126,7 +135,7 @@ public abstract class SystemReader { public int getTimezone(long when) { return getTimeZone().getOffset(when) / (60 * 1000); } - }; + } private static SystemReader INSTANCE = DEFAULT; @@ -143,8 +152,22 @@ public abstract class SystemReader { public static void setInstance(SystemReader newReader) { if (newReader == null) INSTANCE = DEFAULT; - else + else { + newReader.init(); INSTANCE = newReader; + } + } + + private ObjectChecker platformChecker; + + private void init() { + // Creating ObjectChecker must be deferred. Unit tests change + // behavior of is{Windows,MacOS} in constructor of subclass. + if (platformChecker == null) { + platformChecker = new ObjectChecker() + .setSafeForWindows(isWindows()) + .setSafeForMacOS(isMacOS()); + } } /** @@ -286,4 +309,16 @@ public abstract class SystemReader { return "Mac OS X".equals(osDotName) || "Darwin".equals(osDotName); //$NON-NLS-1$ //$NON-NLS-2$ } + /** + * Check tree path entry for validity. + *

+ * Scans a multi-directory path string such as {@code "src/main.c"}. + * + * @param path path string to scan. + * @throws CorruptObjectException path is invalid. + * @since 3.6 + */ + public void checkPath(String path) throws CorruptObjectException { + platformChecker.checkPath(path); + } } -- 2.39.5