Browse Source

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 9638e0aa87,
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>
stable-4.1
David Pletcher 10 years ago
parent
commit
c06620882b
  1. 61
      org.eclipse.jgit/src/org/eclipse/jgit/transport/PackParser.java

61
org.eclipse.jgit/src/org/eclipse/jgit/transport/PackParser.java

@ -135,6 +135,8 @@ public abstract class PackParser {
private boolean allowThin; private boolean allowThin;
private boolean checkObjectCollisions;
private boolean needBaseObjectIds; private boolean needBaseObjectIds;
private boolean checkEofAfterPackFooter; private boolean checkEofAfterPackFooter;
@ -204,6 +206,7 @@ public abstract class PackParser {
objectDigest = Constants.newMessageDigest(); objectDigest = Constants.newMessageDigest();
tempObjectId = new MutableObjectId(); tempObjectId = new MutableObjectId();
packDigest = Constants.newMessageDigest(); packDigest = Constants.newMessageDigest();
checkObjectCollisions = true;
} }
/** @return true if a thin pack (missing base objects) is permitted. */ /** @return true if a thin pack (missing base objects) is permitted. */
@ -224,6 +227,39 @@ public abstract class PackParser {
allowThin = allow; 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. * Configure this index pack instance to keep track of new objects.
* <p> * <p>
@ -988,7 +1024,8 @@ public abstract class PackParser {
} }
inf.close(); inf.close();
tempObjectId.fromRaw(objectDigest.digest(), 0); tempObjectId.fromRaw(objectDigest.digest(), 0);
checkContentLater = readCurs.has(tempObjectId); checkContentLater = isCheckObjectCollisions()
&& readCurs.has(tempObjectId);
data = null; data = null;
} else { } else {
@ -1022,17 +1059,19 @@ public abstract class PackParser {
} }
} }
try { if (isCheckObjectCollisions()) {
final ObjectLoader ldr = readCurs.open(id, type); try {
final byte[] existingData = ldr.getCachedBytes(data.length); final ObjectLoader ldr = readCurs.open(id, type);
if (!Arrays.equals(data, existingData)) { final byte[] existingData = ldr.getCachedBytes(data.length);
throw new IOException(MessageFormat.format( if (!Arrays.equals(data, existingData)) {
JGitText.get().collisionOn, id.name())); 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.
} }
} }

Loading…
Cancel
Save