commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
Subject [jira] [Commented] (COMPRESS-280) [COMPRESS] Change TarInputStream Skip Behavior
Date Thu, 01 May 2014 22:23:17 GMT


BELUGA BEHR commented on COMPRESS-280:

The way that 1.8 is implemented, the TarInputStream performs as advertised. It includes unnecessary
overhead of going through IOUtils.skip(), but causes no real harm.

With the changes in this next release to IOUtils.skip(), both skip() and read() of the underlying
InputStream are invoked.  This kind of behavior should be delegated to the caller.  If someone
is writing a test on top of a custom stream that skips less than the requested amount (such
as in the case of CipherInputStream), then the author of such a class would be surprised to
find that if they call skip on TarInputStream that their stream's skip will be call repeatedly,
and then their read method will be invoked.  This is far from intuitive and while I can't
immediately think of a reason this would be bad, it seems like a potential "gotchya" for someone
implementing a stream that TarInputStream sits on top of.

> [COMPRESS] Change TarInputStream Skip Behavior
> ----------------------------------------------
>                 Key: COMPRESS-280
>                 URL:
>             Project: Commons Compress
>          Issue Type: Improvement
>          Components: Archivers
>    Affects Versions: 1.8
>            Reporter: BELUGA BEHR
>            Priority: Minor
>             Fix For: 1.9
>         Attachments:
> InputStream#skip declares:
> {quote}
> Skips over and discards n bytes of data from this input stream. The skip method may,
for a variety of reasons, end up skipping over some smaller number of bytes, possibly 0. 
This may result from any of a number of conditions; reaching end of file before n bytes have
been skipped is only one possibility. The actual number of bytes skipped is returned.  If
n is negative, no bytes are skipped.
> {quote}
> I would recommend doing away with the call to the local IOUtils in the Stream's skip
method and just call skip directly on the underlying stream.  I'd also amend the JavaDoc to
say "end up skipping .. some smaller number of bytes... reaching the end of the current entry."
 The stream is not required to make any best-effort.  For your example, there should be a
BufferedInputStream between the TarInputStream and the CipherInputStream.  This would put
it more in line with all other InputStreams.
> If a client wants to skip an entry manually, they would have to call Commons-IO IOUtils#skipFully,
IOUtils#skip, etc.
> You would then have to modify getNextTarEntry() to call the IOUtils#skip method.

This message was sent by Atlassian JIRA

View raw message