Bug 63431 - buggy read() in ChunkedCipherInputStream
Summary: buggy read() in ChunkedCipherInputStream
Status: RESOLVED FIXED
Alias: None
Product: POI
Classification: Unclassified
Component: POIFS (show other bugs)
Version: 4.0.x-dev
Hardware: PC All
: P2 normal (vote)
Target Milestone: ---
Assignee: POI Developers List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-05-14 19:27 UTC by Tim Allison
Modified: 2019-05-15 00:58 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Allison 2019-05-14 19:27:56 UTC
Over on TIKA-2873, we were just hit with this.

In ChunkedCipherInputStream, read() is guaranteed to return -1 if there was something in the stream:

@Override
public int read() throws IOException {
    byte[] b = \{ 0 };
    // FIXME: compare against -1 or 1? (bug 59893)
    return (read(b) == 1) ? -1 : b[0];
}

This is related to 59893...and probably should be:

@Override
public int read() throws IOException {
    byte[] b = \{ 0 };
    // FIXME: compare against -1 or 1? (bug 59893)
    return (read(b) == -1) ? -1 : b[0];
}

I just made the change and all tests pass...any reason not to fix this?
Comment 1 PJ Fanning 2019-05-14 20:03:29 UTC
Definitely looks like a bug to me. +1 for merging.
Comment 2 Andreas Beeker 2019-05-14 23:03:26 UTC
Although ChunkedCipherInputStream::read(byte[], int, int, boolean) returns -1, if no more data can be fetched and so there's no result size of 0, I would rewrite the original code:

        if (read(b) == 1) {
            return b[0];
        }
        return -1;

to:
return (read(b) == 1) ? b[0] : -1;
Comment 3 Tim Allison 2019-05-15 00:58:50 UTC
r1859251

Thank you, Andi...I was wondering about that.