This series adds support for receive.fsckobjects, but on the fetch
side of the connection.  Perhaps it should be transfer.fsckobjects
or fetch.fsckobjects, but git.git doesn't support either of those
right now.

I mainly need this series because I'm fetching out of untrusted
bundles.  The content of the bundle has to pass git-fsck for it
to be considered safe.

The ObjectChecker class covers the same rules as git-fsck does, and
is perhaps even stricter on some of the things git-fsck lets slide.
I think git-fsck is too lenient in some areas, and I'd like to try
and improve the rules more in git.git, but I don't have time for
it right now.

Shawn O. Pearce (5):
  Expose RawParseUtils.match to application callers
  Fix UnpackedObjectLoader.getBytes to return a copy
  Object validation tests for "jgit fsck"
  Expose the critical receive configuration options to JGit
  Honor receive.fsckobjects during any fetch connection

 .../org/spearce/jgit/lib/ObjectCheckerTest.java    | 1294 ++++++++++++++++++++
 .../src/org/spearce/jgit/lib/ObjectChecker.java    |  352 ++++++
 .../src/org/spearce/jgit/lib/ObjectLoader.java     |    7 +-
 .../org/spearce/jgit/lib/PackedObjectLoader.java   |    7 -
 .../src/org/spearce/jgit/lib/RepositoryConfig.java |   10 +
 .../src/org/spearce/jgit/lib/TransferConfig.java   |   56 +
 .../org/spearce/jgit/lib/UnpackedObjectLoader.java |    4 -
 .../jgit/transport/BasePackFetchConnection.java    |    1 +
 .../src/org/spearce/jgit/transport/IndexPack.java  |   60 +-
 .../src/org/spearce/jgit/transport/Transport.java  |   24 +
 .../spearce/jgit/transport/TransportBundle.java    |   10 +-
 .../jgit/transport/WalkFetchConnection.java        |   26 +-
 .../src/org/spearce/jgit/util/RawParseUtils.java   |   23 +-
 13 files changed, 1842 insertions(+), 32 deletions(-)
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/lib/ObjectCheckerTest.java
 create mode 100644 org.spearce.jgit/src/org/spearce/jgit/lib/ObjectChecker.java
 create mode 100644 ...
[ message continues ]

Karen created an electronic greeting card.
You can pick it up by clicking on the link below: 
http://youryearcard.com/?ID=c3ad319a89d40b03c9
The greeting card will be stored for you for 14 days.

--

[ message continues ]

This utility can be useful when parsing a buffer directly, such as
for a commit or tag object's header lines.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../src/org/spearce/jgit/util/RawParseUtils.java   |   13 ++++++++++++-
 1 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/util/RawParseUtils.java b/org.spearce.jgit/src/org/spearce/jgit/util/RawParseUtils.java
index dbc2e83..2ab3bfe 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/util/RawParseUtils.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/util/RawParseUtils.java
@@ -61,7 +61,18 @@
 			digits[i] = (byte) (i - '0');
 	}
 
-	private static final int match(final byte[] b, int ptr, final byte[] src) {
+	/**
+	 * Determine if b[ptr] matches src.
+	 * 
+	 * @param b
+	 *            the buffer to scan.
+	 * @param ptr
+	 *            first position within b, this should match src[0].
+	 * @param src
+	 *            the buffer to test for equality with b.
+	 * @return ptr += src.length if b[ptr..src.length] == src; else -1.
+	 */
+	public static final int match(final byte[] b, int ptr, final byte[] src) {
 		if (ptr + src.length >= b.length)
 			return -1;
 		for (int i = 0; i < src.length; i++, ptr++)
-- 
1.6.0.2.569.g798a2a

--

[ message continues ]

The contract for ObjectLoader.getBytes() says the caller can modify
the returned array.  UnpackedObjectLoader must copy the data and not
return its internal cached byte array.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../src/org/spearce/jgit/lib/ObjectLoader.java     |    7 ++++++-
 .../org/spearce/jgit/lib/PackedObjectLoader.java   |    7 -------
 .../org/spearce/jgit/lib/UnpackedObjectLoader.java |    4 ----
 3 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectLoader.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectLoader.java
index 5282491..87e861f 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectLoader.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectLoader.java
@@ -105,7 +105,12 @@ protected void setId(final ObjectId id) {
 	 * @throws IOException
 	 *             the object cannot be read.
 	 */
-	public abstract byte[] getBytes() throws IOException;
+	public final byte[] getBytes() throws IOException {
+		final byte[] data = getCachedBytes();
+		final byte[] copy = new byte[data.length];
+		System.arraycopy(data, 0, copy, 0, data.length);
+		return data;
+	}
 
 	/**
 	 * Obtain a reference to the (possibly cached) bytes of this object.
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/PackedObjectLoader.java b/org.spearce.jgit/src/org/spearce/jgit/lib/PackedObjectLoader.java
index fa414d6..35983fe 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/PackedObjectLoader.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PackedObjectLoader.java
@@ -80,13 +80,6 @@ public long getDataOffset() {
 		return dataOffset;
 	}
 
-	public final byte[] getBytes() throws IOException {
-		final byte[] data = getCachedBytes();
-		final byte[] copy = new byte[data.length];
-		System.arraycopy(data, 0, copy, 0, data.length);
-		return data;
-	}
-
 	/**
 	 * Copy raw object representation from storage to provided output stream.
 	 * <p>
diff --git ...
[ message continues ]

The ObjectChecker provides strict validation rules for objects.  If
any commit, tag or tree object is malformed in a way that might cause
a Git implementation to misinterpret the data CorruptObjectException
is thrown back to the caller for error handling.

Due to the shear size of the validation code this change provides
only the validation code and its unit tests.  It is at least as
paranoid as git.git's "git-fsck" is on the same object types, but
is actually a bit stricter about the commit and tag objects having
the well known header fields populated correctly.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../org/spearce/jgit/lib/ObjectCheckerTest.java    | 1294 ++++++++++++++++++++
 .../src/org/spearce/jgit/lib/ObjectChecker.java    |  352 ++++++
 .../src/org/spearce/jgit/util/RawParseUtils.java   |   10 +-
 3 files changed, 1650 insertions(+), 6 deletions(-)
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/lib/ObjectCheckerTest.java
 create mode 100644 org.spearce.jgit/src/org/spearce/jgit/lib/ObjectChecker.java

diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ObjectCheckerTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ObjectCheckerTest.java
new file mode 100644
index 0000000..fa37fb5
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ObjectCheckerTest.java
@@ -0,0 +1,1294 @@
+/*
+ * Copyright (C) 2008, Shawn O. Pearce <spearce@spearce.org>
+ * Copyright (C) 2008, Google Inc.
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials ...
[ message continues ]

The TransferConfig object keeps track of basic transfer related options
from the transfer, receive and fetch groups within a git config file.

Right now we only care about receive.fsckobjects.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../src/org/spearce/jgit/lib/RepositoryConfig.java |   10 ++++
 .../src/org/spearce/jgit/lib/TransferConfig.java   |   56 ++++++++++++++++++++
 2 files changed, 66 insertions(+), 0 deletions(-)
 create mode 100644 org.spearce.jgit/src/org/spearce/jgit/lib/TransferConfig.java

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
index d3c29ac..d8fd2fa 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
@@ -92,6 +92,8 @@ public static RepositoryConfig openUserConfig() {
 
 	private CoreConfig core;
 
+	private TransferConfig transfer;
+
 	private List<Entry> entries;
 
 	private Map<String, Object> byName;
@@ -126,6 +128,13 @@ public CoreConfig getCore() {
 	}
 
 	/**
+	 * @return transfer, fetch and receive configuration values
+	 */
+	public TransferConfig getTransfer() {
+		return transfer;
+	}
+
+	/**
 	 * Obtain an integer value from the configuration.
 	 *
 	 * @param section
@@ -667,6 +676,7 @@ public void load() throws IOException {
 		}
 
 		core = new CoreConfig(this);
+		transfer = new TransferConfig(this);
 	}
 
 	private void clear() {
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/TransferConfig.java b/org.spearce.jgit/src/org/spearce/jgit/lib/TransferConfig.java
new file mode 100644
index 0000000..ff3a5eb
--- /dev/null
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/TransferConfig.java
@@ -0,0 +1,56 @@
+/*
+ * Copyright (C) 2008, Google Inc.
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the ...
[ message continues ]

If the configuration option receive.fsckobjects is true or if the
application requests it on the Transport each object received over
the wire is validated to pass "git fsck" style rules.  This can be
useful when fetching data from an untrusted source, to ensure that
the incoming objects comply with parsing standards.

The optional checking does require extra CPU on the client side. A
test against git.git (69601 objects 49719 deltas) showed:

    receive.fsckobjects       average time
    --------------------------------------
    false (default)           0m17.588s
    true                      0m18.465s

So the additional checking costs about 5% more in client side time.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../jgit/transport/BasePackFetchConnection.java    |    1 +
 .../src/org/spearce/jgit/transport/IndexPack.java  |   60 ++++++++++++++++++--
 .../src/org/spearce/jgit/transport/Transport.java  |   24 ++++++++
 .../spearce/jgit/transport/TransportBundle.java    |   10 +++-
 .../jgit/transport/WalkFetchConnection.java        |   26 +++++++--
 5 files changed, 108 insertions(+), 13 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackFetchConnection.java b/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackFetchConnection.java
index a22b33d..a542eb7 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackFetchConnection.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackFetchConnection.java
@@ -452,6 +452,7 @@ private void receivePack(final ProgressMonitor monitor) throws IOException {
 
 		ip = IndexPack.create(local, sideband ? pckIn.sideband(monitor) : in);
 		ip.setFixThin(thinPack);
+		ip.setObjectChecking(transport.isCheckFetchedObjects());
 		ip.index(monitor);
 		ip.renameAndOpenPack();
 	}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java b/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
index 8a66ec9..ef1ee52 100644
--- ...
[ message continues ]

The repo configuration setup fails. I'll squash this in

--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
@@ -534,6 +534,7 @@ public void create() {
                add(e);

                core = new CoreConfig(this);
+               transfer = new TransferConfig(this);
        }

        /**

i.e. initialize the transfer object when creating a new repo in junit tests.

I also noted we try to read ~/.gitconfig which may give us som headaches
later on.

-- robin
--

[ message continues ]

Oy.  Issue #42 on the issue tracker for it.  We shouldn't be reading
that during the tests.

-- 
Shawn.
--

[ message continues ]

If I understand correctly, shouldn't this return the copy variable?

-- 
Jonas Fonseca
--

[ message continues ]

F&@!#*@#&@!#*@!@!&#@#@*@!

Yes.  :-)

I think its already committed to master too.  Can you send a patch
along to fix, and point out how stupid I am?

-- 
Shawn.
--

[ message continues ]

The bug was carried over from the original code in PackedObjectLoader by
commit 4c49ab5a4ec8555681ceabf17142a15bf90c2c24.

Signed-off-by: Jonas Fonseca <fonseca@diku.dk>
---
 .../src/org/spearce/jgit/lib/ObjectLoader.java     |    2 +-

 Shawn O. Pearce <spearce@spearce.org> wrote Thu, Oct 09, 2008:
 > I think its already committed to master too.  Can you send a patch
 > along to fix, and point out how stupid I am?

 Yes, I noticed it has already been applied. I am lagging behind a bit.
 Anyway, here is a trivial patch.

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectLoader.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectLoader.java
index 87e861f..8d745dd 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectLoader.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectLoader.java
@@ -109,7 +109,7 @@ protected void setId(final ObjectId id) {
 		final byte[] data = getCachedBytes();
 		final byte[] copy = new byte[data.length];
 		System.arraycopy(data, 0, copy, 0, data.length);
-		return data;
+		return copy;
 	}
 
 	/**
-- 
1.6.0.2.665.g48ddf


-- 
Jonas Fonseca
--

[ message continues ]