{"thread":{"id":"15271","subject":"[JGIT PATCH] index-pack: Avoid disk corruption in objects appended to thin packs","startedAt":"2008-08-29T05:54:10Z","lastAt":"2008-08-29T05:54:10Z","messageCount":1,"participants":["Shawn O. Pearce"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"89087","messageId":"1219989250-39943-1-git-send-email-spearce@spearce.org","threadId":"15271","inReplyTo":null,"subject":"[JGIT PATCH] index-pack: Avoid disk corruption in objects appended to thin packs","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-08-29T05:54:10Z","receivedAt":"2008-08-29T05:54:10Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"When we are appending onto a thin pack we need to make sure the whole\nobject we wrote into the pack is what is there when we re-read the\npack to compute the final footer.\n\nNicolas Pitre pointed out the need to perform this sort of compare\nin C Git.  We missed it in my first round of index-pack improvements.\n\nWe also now take advantage of \"roughly\" block aligned reads during\nthe fixup operation.  4k is a popular filesystem block size and by\narranging the code a little differently we can always perform reads\non 4k alignments within the file.  This should make it easier for\nthe OS to move data to us.\n\nSigned-off-by: Shawn O. Pearce <spearce@spearce.org>\n---\n .../src/org/spearce/jgit/transport/IndexPack.java  |   47 ++++++++++++++-----\n 1 files changed, 34 insertions(+), 13 deletions(-)\n\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java b/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java\nindex 7b1f7ee..d8e6548 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java\n@@ -76,7 +76,7 @@\n \t/** Progress message when computing names of delta compressed objects. */\n \tpublic static final String PROGRESS_RESOLVE_DELTA = \"Resolving deltas\";\n \n-\tprivate static final int BUFFER_SIZE = 2048;\n+\tprivate static final int BUFFER_SIZE = 4096;\n \n \t/**\n \t * Create an index pack instance to load a new pack into a repository.\n@@ -403,6 +403,7 @@ while (bi < b.size()) {\n \tprivate void fixThinPack(final ProgressMonitor progress) throws IOException {\n \t\tgrowEntries();\n \n+\t\tpackDigest.reset();\n \t\toriginalEOF = packOut.length() - 20;\n \t\tfinal Deflater def = new Deflater(Deflater.DEFAULT_COMPRESSION, false);\n \t\tlong end = originalEOF;\n@@ -432,7 +433,7 @@ if (!baseById.isEmpty()) {\n \t\t\tthrow new MissingObjectException(need, \"delta base\");\n \t\t}\n \n-\t\tfixHeaderFooter();\n+\t\tfixHeaderFooter(packcsum, packDigest.digest());\n \t}\n \n \tprivate void writeWhole(final Deflater def, final int typeCode,\n@@ -446,6 +447,7 @@ while (sz > 0) {\n \t\t\tbuf[hdrlen++] = (byte) (sz & 0x7f);\n \t\t\tsz >>>= 7;\n \t\t}\n+\t\tpackDigest.update(buf, 0, hdrlen);\n \t\tcrc.update(buf, 0, hdrlen);\n \t\tpackOut.write(buf, 0, hdrlen);\n \t\tdef.reset();\n@@ -453,37 +455,56 @@ while (sz > 0) {\n \t\tdef.finish();\n \t\twhile (!def.finished()) {\n \t\t\tfinal int datlen = def.deflate(buf);\n+\t\t\tpackDigest.update(buf, 0, datlen);\n \t\t\tcrc.update(buf, 0, datlen);\n \t\t\tpackOut.write(buf, 0, datlen);\n \t\t}\n \t}\n \n-\tprivate void fixHeaderFooter() throws IOException {\n+\tprivate void fixHeaderFooter(final byte[] origcsum, final byte[] tailcsum)\n+\t\t\tthrows IOException {\n \t\tfinal MessageDigest origDigest = Constants.newMessageDigest();\n-\t\tlong origRemaining = originalEOF - 12;\n+\t\tfinal MessageDigest tailDigest = Constants.newMessageDigest();\n+\t\tlong origRemaining = originalEOF;\n \n \t\tpackOut.seek(0);\n-\t\tif (packOut.read(buf, 0, 12) != 12)\n-\t\t\tthrow new IOException(\"Cannot re-read pack header to fix count\");\n-\t\torigDigest.update(buf, 0, 12);\n+\t\tbAvail = 0;\n+\t\tbOffset = 0;\n+\t\tfillFromFile(12);\n+\n+\t\t{\n+\t\t\tfinal int origCnt = (int) Math.min(bAvail, origRemaining);\n+\t\t\torigDigest.update(buf, 0, origCnt);\n+\t\t\torigRemaining -= origCnt;\n+\t\t\tif (origRemaining == 0)\n+\t\t\t\ttailDigest.update(buf, origCnt, bAvail - origCnt);\n+\t\t}\n+\n \t\tNB.encodeInt32(buf, 8, entryCount);\n \t\tpackOut.seek(0);\n \t\tpackOut.write(buf, 0, 12);\n+\t\tpackOut.seek(bAvail);\n \n \t\tpackDigest.reset();\n-\t\tpackDigest.update(buf, 0, 12);\n+\t\tpackDigest.update(buf, 0, bAvail);\n \t\tfor (;;) {\n \t\t\tfinal int n = packOut.read(buf);\n \t\t\tif (n < 0)\n \t\t\t\tbreak;\n-\t\t\tif (origRemaining > 0) {\n-\t\t\t\torigDigest.update(buf, 0, (int) Math.min(n, origRemaining));\n-\t\t\t\torigRemaining -= n;\n-\t\t\t}\n+\t\t\tif (origRemaining != 0) {\n+\t\t\t\tfinal int origCnt = (int) Math.min(n, origRemaining);\n+\t\t\t\torigDigest.update(buf, 0, origCnt);\n+\t\t\t\torigRemaining -= origCnt;\n+\t\t\t\tif (origRemaining == 0)\n+\t\t\t\t\ttailDigest.update(buf, origCnt, n - origCnt);\n+\t\t\t} else\n+\t\t\t\ttailDigest.update(buf, 0, n);\n+\n \t\t\tpackDigest.update(buf, 0, n);\n \t\t}\n \n-\t\tif (!Arrays.equals(origDigest.digest(), packcsum))\n+\t\tif (!Arrays.equals(origDigest.digest(), origcsum)\n+\t\t\t\t|| !Arrays.equals(tailDigest.digest(), tailcsum))\n \t\t\tthrow new IOException(\"Pack corrupted while writing to filesystem\");\n \n \t\tpackcsum = packDigest.digest();\n-- \n1.6.0.272.g9ab4\n"}]}