]> source.dussan.org Git - jgit.git/commitdiff
Fix infinite loop in IndexPack 49/1049/4
authorShawn O. Pearce <spearce@spearce.org>
Sat, 3 Jul 2010 01:21:55 +0000 (18:21 -0700)
committerShawn O. Pearce <spearce@spearce.org>
Fri, 16 Jul 2010 17:12:04 +0000 (10:12 -0700)
A programming error using the Inflater API led to an infinite
loop within IndexPack, caused by the Inflater returning 0 from
the inflate() method, but it didn't want more input.  This happens
when it has reached the end of the stream, or has reached a spot
asking for an external dictionary.  Such a case is a failure for us,
and we should abort out.

Thanks to Alex for pointing out that we had 3 implementations of
the inflate rountine, which should be consolidated into one and
use a switch to determine where to load data from.

Bug: 317416
Change-Id: I34120482375b687ea36ed9154002d77047e94b1f
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
org.eclipse.jgit/resources/org/eclipse/jgit/JGitText.properties
org.eclipse.jgit/src/org/eclipse/jgit/JGitText.java
org.eclipse.jgit/src/org/eclipse/jgit/transport/IndexPack.java

index 4a54089e067ad92d30b740fdc80956c335628a8a..e4a603cbbad322e4c29122746ae69acdf4b97753 100644 (file)
@@ -357,6 +357,7 @@ unknownIndexVersionOrCorruptIndex=Unknown index version (or corrupt index): {0}
 unknownObjectType=Unknown object type {0}.
 unknownRepositoryFormat2=Unknown repository format "{0}"; expected "0".
 unknownRepositoryFormat=Unknown repository format
+unknownZlibError=Unknown zlib error.
 unmergedPath=Unmerged path: {0}
 unpackError=unpack error {0}
 unreadablePackIndex=Unreadable pack index: {0}
index f0bedfa2f461ffad041965c747011f13bd45527d..94cf1e48bda3992a192d5c2d493d08a1c3570e19 100644 (file)
@@ -416,6 +416,7 @@ public class JGitText extends TranslationBundle {
        /***/ public String unknownObjectType;
        /***/ public String unknownRepositoryFormat2;
        /***/ public String unknownRepositoryFormat;
+       /***/ public String unknownZlibError;
        /***/ public String unmergedPath;
        /***/ public String unpackError;
        /***/ public String unreadablePackIndex;
index 2a5b4344f590e47192f1c7c218f9ee7f8c370357..789d8cdd74d0f97ff07ab30c2682fd2539af8799 100644 (file)
@@ -131,6 +131,19 @@ public class IndexPack {
                return ip;
        }
 
+       private static enum Source {
+               /** Data is read from the incoming stream. */
+               INPUT,
+
+               /**
+                * Data is read from the spooled pack file.
+                * <p>
+                * During streaming, some (or all) data might be saved into the spooled
+                * pack file so it can be randomly accessed later.
+                */
+               FILE;
+       }
+
        private final Repository repo;
 
        /**
@@ -200,7 +213,7 @@ public class IndexPack {
 
        private LongMap<UnresolvedDelta> baseByPos;
 
-       private byte[] objectData;
+       private byte[] skipBuffer;
 
        private MessageDigest packDigest;
 
@@ -232,7 +245,7 @@ public class IndexPack {
                inflater = InflaterCache.get();
                readCurs = new WindowCursor();
                buf = new byte[BUFFER_SIZE];
-               objectData = new byte[BUFFER_SIZE];
+               skipBuffer = new byte[512];
                objectDigest = Constants.newMessageDigest();
                tempObjectId = new MutableObjectId();
                packDigest = Constants.newMessageDigest();
@@ -474,12 +487,12 @@ public class IndexPack {
                        byte[] data, PackedObjectInfo oe) throws IOException {
                crc.reset();
                position(pos);
-               int c = readFromFile();
+               int c = readFrom(Source.FILE);
                final int typeCode = (c >> 4) & 7;
                long sz = c & 15;
                int shift = 4;
                while ((c & 0x80) != 0) {
-                       c = readFromFile();
+                       c = readFrom(Source.FILE);
                        sz += (c & 0x7f) << shift;
                        shift += 7;
                }
@@ -490,19 +503,19 @@ public class IndexPack {
                case Constants.OBJ_BLOB:
                case Constants.OBJ_TAG:
                        type = typeCode;
-                       data = inflateFromFile((int) sz);
+                       data = inflateAndReturn(Source.FILE, sz);
                        break;
                case Constants.OBJ_OFS_DELTA: {
-                       c = readFromFile() & 0xff;
+                       c = readFrom(Source.FILE) & 0xff;
                        while ((c & 128) != 0)
-                               c = readFromFile() & 0xff;
-                       data = BinaryDelta.apply(data, inflateFromFile((int) sz));
+                               c = readFrom(Source.FILE) & 0xff;
+                       data = BinaryDelta.apply(data, inflateAndReturn(Source.FILE, sz));
                        break;
                }
                case Constants.OBJ_REF_DELTA: {
-                       crc.update(buf, fillFromFile(20), 20);
+                       crc.update(buf, fill(Source.FILE, 20), 20);
                        use(20);
-                       data = BinaryDelta.apply(data, inflateFromFile((int) sz));
+                       data = BinaryDelta.apply(data, inflateAndReturn(Source.FILE, sz));
                        break;
                }
                default:
@@ -657,7 +670,7 @@ public class IndexPack {
                packOut.seek(0);
                bAvail = 0;
                bOffset = 0;
-               fillFromFile(12);
+               fill(Source.FILE, 12);
 
                {
                        final int origCnt = (int) Math.min(bAvail, origRemaining);
@@ -728,7 +741,7 @@ public class IndexPack {
 
        private void readPackHeader() throws IOException {
                final int hdrln = Constants.PACK_SIGNATURE.length + 4 + 4;
-               final int p = fillFromInput(hdrln);
+               final int p = fill(Source.INPUT, hdrln);
                for (int k = 0; k < Constants.PACK_SIGNATURE.length; k++)
                        if (buf[p + k] != Constants.PACK_SIGNATURE[k])
                                throw new IOException(JGitText.get().notAPACKFile);
@@ -743,7 +756,7 @@ public class IndexPack {
        private void readPackFooter() throws IOException {
                sync();
                final byte[] cmpcsum = packDigest.digest();
-               final int c = fillFromInput(20);
+               final int c = fill(Source.INPUT, 20);
                packcsum = new byte[20];
                System.arraycopy(buf, c, packcsum, 0, 20);
                use(20);
@@ -757,7 +770,7 @@ public class IndexPack {
        // Cleanup all resources associated with our input parsing.
        private void endInput() {
                in = null;
-               objectData = null;
+               skipBuffer = null;
        }
 
        // Read one entire object or delta from the input.
@@ -765,12 +778,12 @@ public class IndexPack {
                final long pos = position();
 
                crc.reset();
-               int c = readFromInput();
+               int c = readFrom(Source.INPUT);
                final int typeCode = (c >> 4) & 7;
                long sz = c & 15;
                int shift = 4;
                while ((c & 0x80) != 0) {
-                       c = readFromInput();
+                       c = readFrom(Source.INPUT);
                        sz += (c & 0x7f) << shift;
                        shift += 7;
                }
@@ -783,24 +796,24 @@ public class IndexPack {
                        whole(typeCode, pos, sz);
                        break;
                case Constants.OBJ_OFS_DELTA: {
-                       c = readFromInput();
+                       c = readFrom(Source.INPUT);
                        long ofs = c & 127;
                        while ((c & 128) != 0) {
                                ofs += 1;
-                               c = readFromInput();
+                               c = readFrom(Source.INPUT);
                                ofs <<= 7;
                                ofs += (c & 127);
                        }
                        final long base = pos - ofs;
                        final UnresolvedDelta n;
-                       skipInflateFromInput(sz);
+                       inflateAndSkip(Source.INPUT, sz);
                        n = new UnresolvedDelta(pos, (int) crc.getValue());
                        n.next = baseByPos.put(base, n);
                        deltaCount++;
                        break;
                }
                case Constants.OBJ_REF_DELTA: {
-                       c = fillFromInput(20);
+                       c = fill(Source.INPUT, 20);
                        crc.update(buf, c, 20);
                        final ObjectId base = ObjectId.fromRaw(buf, c);
                        use(20);
@@ -809,7 +822,7 @@ public class IndexPack {
                                r = new DeltaChain(base);
                                baseById.add(r);
                        }
-                       skipInflateFromInput(sz);
+                       inflateAndSkip(Source.INPUT, sz);
                        r.add(new UnresolvedDelta(pos, (int) crc.getValue()));
                        deltaCount++;
                        break;
@@ -821,7 +834,7 @@ public class IndexPack {
 
        private void whole(final int type, final long pos, final long sz)
                        throws IOException {
-               final byte[] data = inflateFromInput(sz);
+               final byte[] data = inflateAndReturn(Source.INPUT, sz);
                objectDigest.update(Constants.encodedTypeString(type));
                objectDigest.update((byte) ' ');
                objectDigest.update(Constants.encodeASCII(sz));
@@ -867,19 +880,9 @@ public class IndexPack {
        }
 
        // Consume exactly one byte from the buffer and return it.
-       private int readFromInput() throws IOException {
-               if (bAvail == 0)
-                       fillFromInput(1);
-               bAvail--;
-               final int b = buf[bOffset++] & 0xff;
-               crc.update(b);
-               return b;
-       }
-
-       // Consume exactly one byte from the buffer and return it.
-       private int readFromFile() throws IOException {
+       private int readFrom(final Source src) throws IOException {
                if (bAvail == 0)
-                       fillFromFile(1);
+                       fill(src, 1);
                bAvail--;
                final int b = buf[bOffset++] & 0xff;
                crc.update(b);
@@ -893,36 +896,32 @@ public class IndexPack {
        }
 
        // Ensure at least need bytes are available in in {@link #buf}.
-       private int fillFromInput(final int need) throws IOException {
+       private int fill(final Source src, final int need) throws IOException {
                while (bAvail < need) {
                        int next = bOffset + bAvail;
                        int free = buf.length - next;
                        if (free + bAvail < need) {
-                               sync();
+                               switch(src){
+                               case INPUT:
+                                       sync();
+                                       break;
+                               case FILE:
+                                       if (bAvail > 0)
+                                               System.arraycopy(buf, bOffset, buf, 0, bAvail);
+                                       bOffset = 0;
+                                       break;
+                               }
                                next = bAvail;
                                free = buf.length - next;
                        }
-                       next = in.read(buf, next, free);
-                       if (next <= 0)
-                               throw new EOFException(JGitText.get().packfileIsTruncated);
-                       bAvail += next;
-               }
-               return bOffset;
-       }
-
-       // Ensure at least need bytes are available in in {@link #buf}.
-       private int fillFromFile(final int need) throws IOException {
-               if (bAvail < need) {
-                       int next = bOffset + bAvail;
-                       int free = buf.length - next;
-                       if (free + bAvail < need) {
-                               if (bAvail > 0)
-                                       System.arraycopy(buf, bOffset, buf, 0, bAvail);
-                               bOffset = 0;
-                               next = bAvail;
-                               free = buf.length - next;
+                       switch(src){
+                       case INPUT:
+                               next = in.read(buf, next, free);
+                               break;
+                       case FILE:
+                               next = packOut.read(buf, next, free);
+                               break;
                        }
-                       next = packOut.read(buf, next, free);
                        if (next <= 0)
                                throw new EOFException(JGitText.get().packfileIsTruncated);
                        bAvail += next;
@@ -941,112 +940,69 @@ public class IndexPack {
                bOffset = 0;
        }
 
-       private void skipInflateFromInput(long sz) throws IOException {
-               final Inflater inf = inflater;
-               try {
-                       final byte[] dst = objectData;
-                       int n = 0;
-                       int p = -1;
-                       while (!inf.finished()) {
-                               if (inf.needsInput()) {
-                                       if (p >= 0) {
-                                               crc.update(buf, p, bAvail);
-                                               use(bAvail);
-                                       }
-                                       p = fillFromInput(1);
-                                       inf.setInput(buf, p, bAvail);
-                               }
-
-                               int free = dst.length - n;
-                               if (free < 8) {
-                                       sz -= n;
-                                       n = 0;
-                                       free = dst.length;
-                               }
-                               n += inf.inflate(dst, n, free);
-                       }
-                       if (n != sz)
-                               throw new DataFormatException(JGitText.get().wrongDecompressedLength);
-                       n = bAvail - inf.getRemaining();
-                       if (n > 0) {
-                               crc.update(buf, p, n);
-                               use(n);
-                       }
-               } catch (DataFormatException dfe) {
-                       throw corrupt(dfe);
-               } finally {
-                       inf.reset();
-               }
+       private void inflateAndSkip(final Source src, final long inflatedSize)
+                       throws IOException {
+               inflate(src, inflatedSize, skipBuffer, false /* do not keep result */);
        }
 
-       private byte[] inflateFromInput(final long sz) throws IOException {
-               final byte[] dst = new byte[(int) sz];
-               final Inflater inf = inflater;
-               try {
-                       int n = 0;
-                       int p = -1;
-                       while (!inf.finished()) {
-                               if (inf.needsInput()) {
-                                       if (p >= 0) {
-                                               crc.update(buf, p, bAvail);
-                                               use(bAvail);
-                                       }
-                                       p = fillFromInput(1);
-                                       inf.setInput(buf, p, bAvail);
-                               }
-
-                               n += inf.inflate(dst, n, dst.length - n);
-                       }
-                       if (n != sz)
-                               throw new DataFormatException(JGitText.get().wrongDecompressedLength);
-                       n = bAvail - inf.getRemaining();
-                       if (n > 0) {
-                               crc.update(buf, p, n);
-                               use(n);
-                       }
-                       return dst;
-               } catch (DataFormatException dfe) {
-                       throw corrupt(dfe);
-               } finally {
-                       inf.reset();
-               }
+       private byte[] inflateAndReturn(final Source src, final long inflatedSize)
+                       throws IOException {
+               final byte[] dst = new byte[(int) inflatedSize];
+               inflate(src, inflatedSize, dst, true /* keep result in dst */);
+               return dst;
        }
 
-       private byte[] inflateFromFile(final int sz) throws IOException {
+       private void inflate(final Source src, final long inflatedSize,
+                       final byte[] dst, final boolean keep) throws IOException {
                final Inflater inf = inflater;
                try {
-                       final byte[] dst = new byte[sz];
-                       int n = 0;
-                       int p = -1;
-                       while (!inf.finished()) {
-                               if (inf.needsInput()) {
-                                       if (p >= 0) {
-                                               crc.update(buf, p, bAvail);
-                                               use(bAvail);
+                       int off = 0;
+                       long cnt = 0;
+                       int p = fill(src, 24);
+                       inf.setInput(buf, p, bAvail);
+
+                       do {
+                               int r = inf.inflate(dst, off, dst.length - off);
+                               if (r == 0) {
+                                       if (inf.finished())
+                                               break;
+                                       if (inf.needsInput()) {
+                                               if (p >= 0) {
+                                                       crc.update(buf, p, bAvail);
+                                                       use(bAvail);
+                                               }
+                                               p = fill(src, 24);
+                                               inf.setInput(buf, p, bAvail);
+                                       } else {
+                                               throw new CorruptObjectException(MessageFormat.format(
+                                                               JGitText.get().packfileCorruptionDetected,
+                                                               JGitText.get().unknownZlibError));
                                        }
-                                       p = fillFromFile(1);
-                                       inf.setInput(buf, p, bAvail);
                                }
-                               n += inf.inflate(dst, n, sz - n);
+                               cnt += r;
+                               if (keep)
+                                       off += r;
+                       } while (cnt < inflatedSize);
+
+                       if (!inf.finished() || cnt != inflatedSize) {
+                               throw new CorruptObjectException(MessageFormat.format(JGitText
+                                               .get().packfileCorruptionDetected,
+                                               JGitText.get().wrongDecompressedLength));
                        }
-                       n = bAvail - inf.getRemaining();
-                       if (n > 0) {
-                               crc.update(buf, p, n);
-                               use(n);
+
+                       int left = bAvail - inf.getRemaining();
+                       if (left > 0) {
+                               crc.update(buf, p, left);
+                               use(left);
                        }
-                       return dst;
                } catch (DataFormatException dfe) {
-                       throw corrupt(dfe);
+                       throw new CorruptObjectException(MessageFormat.format(JGitText
+                                       .get().packfileCorruptionDetected, dfe.getMessage()));
                } finally {
                        inf.reset();
                }
        }
 
-       private static CorruptObjectException corrupt(final DataFormatException dfe) {
-               return new CorruptObjectException(MessageFormat.format(
-                               JGitText.get().packfileCorruptionDetected, dfe.getMessage()));
-       }
-
        private static class DeltaChain extends ObjectId {
                UnresolvedDelta head;