tomasilluminati commented on PR #777:
URL: https://github.com/apache/commons-compress/pull/777#issuecomment-4827612979

   @garydgregory 
   Thanks, this is really helpful, and honestly it points at a simpler shape 
than what I pushed.
   
   If all we want is a decompressed-size limit, IO's BoundedInputStream already 
does it (setMaxCount plus an onMaxCount that throws). So we don't really need a 
compress-specific class or a new exception. I'm happy to drop 
BombGuardCompressorInputStream and DecompressionBombException and just have 
CompressorStreamFactory wrap the stream in a BoundedInputStream when a max size 
is set. That's the single place you're describing, it reuses IO, and the bomb 
wording goes away. The builder point sorts itself out too, since there's no 
class left to build.
   
   The only bit that doesn't fit there is the compression-ratio check, since 
BoundedInputStream has no idea about ratios and it needs getCompressedCount() 
from the compressor side. So really the only open question is scope. Just the 
size limit, or the ratio guard too?
   
   - Size only, I drop the class and the exception and wire BoundedInputStream 
into the factory. Smallest change.
   - Ratio too, I keep it but move it onto CompressorInputStream where both 
counts already live, still exposed through the factory.
   
   My hunch from your note is size-only, but just say the word and I'll redo it 
that way. Either way the InputStreamStatistics change stays, the compressed 
count is handy on its own.
   
   Thanks again for the steer.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to