Parser.readImageData() infinite loop may never break for a broken gif file

[vince-styling]

in case of a broken gif

fun ReplayInputStream.readImageData(): ImageData {
        val position = getPosition()
        var length = 1
        skip(1)
        while (true) {
            val blockSize = readUByte().toInt()
            length++
            if (blockSize == 0) {
                // never reach
                break
            }
            length += blockSize
            skip(blockSize.toLong())
        }

        return ImageData(position, length)
    }

[redwarp]

Oh that is interesting. Do you have a broken gif you can share with me to reproduce?

[redwarp]

(I mean, I can see how it could loop, but a broken gif will speed things up)

[redwarp]

The problem here is that we don't consider -1, so when the read file.

[redwarp]

Doing readUByte().toInt() is pretty dumb: readUByte does a read() that returns an int, convert it to a ubyte, then reconvert it to int. And that will erase the -1 possibility. So here instead I should just do val blockSize = read() and then check for -1.

[vince-styling]

the problem occur inside the skip(blockSize.toLong()) line, this line will enter a dead loop, because

code from InputStream#skip(n)
while (remaining > 0) {
    nr = read(skipBuffer, 0, (int)Math.min(size, remaining));
    if (nr < 0) {
        break;
    }
    remaining -= nr;
}

the read() will invoke inherited RandomAccessFileInputStream#read() which will always return 0 since the RandomAccessFile reached the end of file.

the broken file is too large, hard to share, you can make your own by strip off numbers of bytes in the tail of file to reproduce.

[vince-styling]

thanks for sharing this library, it help me so much, especially the huge file consideration, some problems or improvements want to discuss with you during using, I will open them one by one issues

[redwarp]

Also found a bug in my BufferedRandomAccessFile where read could also loop forever if we ask to read more than available in the file.

[vince-styling]

my solution is have BufferedRandomAccessFile implement skip method, assert it must skip enough bytes or I will throw an InvalidGifException

class BufferedRandomAccessFile {
    fun skip(byteCount: Int) {
        bufferPosition += byteCount
        if (bufferPosition < bufferEnd) return
        val leftSkipCount = bufferPosition - bufferEnd
        realPosition += leftSkipCount
        if (fillBuffer() <= 0) throw InvalidGifException(
            "skip to $realPosition failed as ${file.length()}"
        )
    }
}

[redwarp]

Oh I fixed that, if you check my latest commit, skip and read now work properly, I added a test to make sure