Delete checkObjectCollisions from PackParser

This flag was added to provide an unsafe operation on the local
repository because the storage.dht code was too damn slow to provide
proper safe Git behavior all of the time. Now that stoarge.dht has
been removed from the repository, also delete this unsafe flag to
prevent applications from misusing the JGit library and permitting
users to potentially damage their local repository with bad data
received from an untrusted peer.

Change-Id: Ib1861c48bb74836731e7b7d57b635dd654b0dc66
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

1 files changed, 11 insertions, 46 deletions

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/PackParser.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/PackParser.java
index 5a7d74de79..0c035e547d 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/PackParser.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/PackParser.java
@@ -135,8 +135,6 @@ public abstract class PackParser {
 
 	private boolean allowThin;
 
-	private boolean checkObjectCollisions;
-
 	private boolean needBaseObjectIds;
 
 	private boolean checkEofAfterPackFooter;
@@ -206,7 +204,6 @@ public abstract class PackParser {
 		objectDigest = Constants.newMessageDigest();
 		tempObjectId = new MutableObjectId();
 		packDigest = Constants.newMessageDigest();
-		checkObjectCollisions = true;
 	}
 
 	/** @return true if a thin pack (missing base objects) is permitted. */
@@ -227,35 +224,6 @@ public abstract class PackParser {
 		allowThin = allow;
 	}
 
-	/** @return if true received objects are verified to prevent collisions. */
-	public boolean isCheckObjectCollisions() {
-		return checkObjectCollisions;
-	}
-
-	/**
-	 * Enable checking for collisions with existing objects.
-	 * <p>
-	 * By default PackParser looks for each received object in the repository.
-	 * If the object already exists, the existing object is compared
-	 * byte-for-byte with the newly received copy to ensure they are identical.
-	 * The receive is aborted with an exception if any byte differs. This check
-	 * is necessary to prevent an evil attacker from supplying a replacement
-	 * object into this repository in the event that a discovery enabling SHA-1
-	 * collisions is made.
-	 * <p>
-	 * This check may be very costly to perform, and some repositories may have
-	 * other ways to segregate newly received object data. The check is enabled
-	 * by default, but can be explicitly disabled if the implementation can
-	 * provide the same guarantee, or is willing to accept the risks associated
-	 * with bypassing the check.
-	 *
-	 * @param check
-	 *            true to enable collision checking (strongly encouraged).
-	 */
-	public void setCheckObjectCollisions(boolean check) {
-		checkObjectCollisions = check;
-	}
-
 	/**
 	 * Configure this index pack instance to keep track of new objects.
 	 * <p>
@@ -1004,8 +972,7 @@ public abstract class PackParser {
 			}
 			inf.close();
 			tempObjectId.fromRaw(objectDigest.digest(), 0);
-			checkContentLater = isCheckObjectCollisions()
-					&& readCurs.has(tempObjectId);
+			checkContentLater = readCurs.has(tempObjectId);
 			data = null;
 
 		} else {
@@ -1037,19 +1004,17 @@ public abstract class PackParser {
 			}
 		}
 
-		if (isCheckObjectCollisions()) {
-			try {
-				final ObjectLoader ldr = readCurs.open(id, type);
-				final byte[] existingData = ldr.getCachedBytes(data.length);
-				if (!Arrays.equals(data, existingData)) {
-					throw new IOException(MessageFormat.format(
-							JGitText.get().collisionOn, id.name()));
-				}
-			} catch (MissingObjectException notLocal) {
-				// This is OK, we don't have a copy of the object locally
-				// but the API throws when we try to read it as usually its
-				// an error to read something that doesn't exist.
+		try {
+			final ObjectLoader ldr = readCurs.open(id, type);
+			final byte[] existingData = ldr.getCachedBytes(data.length);
+			if (!Arrays.equals(data, existingData)) {
+				throw new IOException(MessageFormat.format(
+						JGitText.get().collisionOn, id.name()));
 			}
+		} catch (MissingObjectException notLocal) {
+			// This is OK, we don't have a copy of the object locally
+			// but the API throws when we try to read it as usually its
+			// an error to read something that doesn't exist.
 		}
 	}