[ 
https://issues.apache.org/jira/browse/COMPRESS-280?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13987078#comment-13987078
 ] 

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: https://issues.apache.org/jira/browse/COMPRESS-280
>             Project: Commons Compress
>          Issue Type: Improvement
>          Components: Archivers
>    Affects Versions: 1.8
>            Reporter: BELUGA BEHR
>            Priority: Minor
>             Fix For: 1.9
>
>         Attachments: TarArchiveInputStream.java.patch
>
>
> 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
(v6.2#6252)

Reply via email to