commons-compress icon indicating copy to clipboard operation
commons-compress copied to clipboard

COMPRESS-623: make ZipFile's getRawInputStream usable when local headers are not read

Open dweiss opened this issue 2 years ago • 7 comments

See https://issues.apache.org/jira/browse/COMPRESS-623

dweiss avatar Aug 12 '22 08:08 dweiss

Codecov Report

Merging #306 (3c6b0b0) into master (7cae698) will decrease coverage by 0.03%. The diff coverage is 53.12%.

@@             Coverage Diff              @@
##             master     #306      +/-   ##
============================================
- Coverage     80.05%   80.01%   -0.04%     
+ Complexity     6621     6620       -1     
============================================
  Files           339      339              
  Lines         25416    25420       +4     
  Branches       4199     4200       +1     
============================================
- Hits          20346    20341       -5     
- Misses         3473     3480       +7     
- Partials       1597     1599       +2     
Impacted Files Coverage Δ
...apache/commons/compress/archivers/zip/ZipFile.java 77.00% <53.12%> (-1.77%) :arrow_down:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

codecov-commenter avatar Aug 12 '22 08:08 codecov-commenter

Eh. It wasn't me - it was intellij for some reason... That's why I like spotless/google formatter... makes life much easier for folks using different tools. Give me a minute to revert those changes.

dweiss avatar Aug 12 '22 10:08 dweiss

Removed those whitespace changes.

dweiss avatar Aug 12 '22 10:08 dweiss

I was a bit lazy and wanted to piggyback too many changes, sorry about this. I've removed linked list removal for now and just left the changes related to getRawInputStream - hope it'll make them clearer to read. I've ran mvn test -Prun-zipit and everything passed. I'm not sure why getRawInputStream wasn't used inside getInputStream itself but if you take a look at the patch, this seems like a better decision to me.

The only vital non-backward compatible change is the additional throws IOException added to getRawInputStream. I think this is more transparent in the end than hiding the exception under an unchecked exception (or swallowing it). People who use the API will most likely not even notice the change because they catch/handle IOException with other methods of ZipFile.

dweiss avatar Aug 13 '22 07:08 dweiss

Ping. There's not much for me to do here; I've reverted whitespace changes as requested. You probably looked at an outdated diff, when you requested changes, @garydgregory ?

dweiss avatar Aug 20 '22 16:08 dweiss

You can expect bursts of activity and inactivity, we're all volunteers here. I'll take a look over the weekend hopefully.

garydgregory avatar Aug 20 '22 17:08 garydgregory

That's fine. Not my first time.

dweiss avatar Aug 20 '22 17:08 dweiss