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

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);
 		}
 	}
 

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.
+	 * <p>
+	 * 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.
+	 * <p>
+	 * 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.
 	 *

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;
 		}
 

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;
  * </p>
  */
 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.
+	 * <p>
+	 * 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);
+	}
 }