Restore checkObjectCollisions flag

I am developing an offline pack verification feature based on
PackParser. The birthday collision check is a prohibitive obstacle
to performance at scale because it interacts with the repository
to perform collision checks. This CL restores the checkObjectCollisions
flag that was removed in 9638e0aa87614a6fb4f109bbeac0cde3462b9769,
while changing the flag getter and setter to protected from public as a
precaution against misuse.

Change-Id: I363cd0c9de57c5e8659cdfe2d51b17823f4fe793
Signed-off-by: David Pletcher <dpletcher@google.com>

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 04abe22323d2680246632ec01d0261f2962e6237..918df94de9d8f5c5e0468673ca34fd5cdfc37515 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/PackParser.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/PackParser.java
@@ -135,6 +135,8 @@ public abstract class PackParser {
 
        private boolean allowThin;
 
+       private boolean checkObjectCollisions;
+
        private boolean needBaseObjectIds;
 
        private boolean checkEofAfterPackFooter;
@@ -204,6 +206,7 @@ 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. */
@@ -224,6 +227,39 @@ public abstract class PackParser {
                allowThin = allow;
        }
 
+       /**
+        * @return if true received objects are verified to prevent collisions.
+        * @since 4.1
+        */
+       protected 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).
+        * @since 4.1
+        */
+       protected void setCheckObjectCollisions(boolean check) {
+               checkObjectCollisions = check;
+       }
+
        /**
         * Configure this index pack instance to keep track of new objects.
         * <p>
@@ -988,7 +1024,8 @@ public abstract class PackParser {
                        }
                        inf.close();
                        tempObjectId.fromRaw(objectDigest.digest(), 0);
-                       checkContentLater = readCurs.has(tempObjectId);
+                       checkContentLater = isCheckObjectCollisions()
+                                       && readCurs.has(tempObjectId);
                        data = null;
 
                } else {
@@ -1022,17 +1059,19 @@ public abstract class PackParser {
                        }
                }
 
-               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()));
+               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.
                        }
-               } 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.
                }
        }