Bug 59893

Summary: Forbid calls to InputStream.available
Product: POI Reporter: Andreas Beeker <kiwiwings>
Component: POI OverallAssignee: POI Developers List <dev>
Status: RESOLVED FIXED    
Severity: normal    
Priority: P2    
Version: 3.15-dev   
Target Milestone: ---   
Hardware: All   
OS: All   
Bug Depends on: 59841    
Bug Blocks:    
Attachments: replace available() calls and add forbidden-apis-check
replace available() calls and add forbidden-apis-check
patch with some of the unrelated code removed
patch with all of the unrelated code removed

Description Andreas Beeker 2016-07-23 12:11:41 UTC
Created attachment 34067 [details]
replace available() calls and add forbidden-apis-check

In #59841 I've realized a lot of wrong calls to InputStream.available().
The wrong assumption is, that it returns the bytes until the EOF, whereas it really return the bytes until the next blocking read [1]

So when working with streams, you really need to read until EOF and not rely on available().

This patch removes most of the calls to available() or ignores them for delegates.


[1] http://stackoverflow.com/questions/3695372/what-does-inputstream-available-do-in-java
Comment 1 Andreas Beeker 2016-07-23 18:01:57 UTC
Created attachment 34068 [details]
replace available() calls and add forbidden-apis-check

fix issue with PropertySet
Comment 2 Dominik Stadler 2016-07-26 05:33:28 UTC
The patch has a number of unrelated changes, whitespaces, @Test, ... can you apply those and re-base the patch, that would make reviewing much easier.
Comment 3 Javen O'Neal 2017-02-17 09:10:21 UTC
Created attachment 34760 [details]
patch with some of the unrelated code removed

I committed some of Andi's unrelated changes from attachment 34068 [details] in r1783347 and r1783353.

Some of the line numbers may be off since I deleted unrelated changes from the patch by directly editing the text file.
Comment 4 Javen O'Neal 2017-02-17 09:28:02 UTC
ChunkedCipherInputStream.read needs attention. Changed from checking the return value of reading from the underlying stream from 1 to -1.

I have temporarily reverted this to restore the original behavior in 1783356. This should probably have been put back into the patch that excludes the unrelated stuff, since this behavior is related to the available() changes.
Comment 5 Javen O'Neal 2017-02-17 09:28:20 UTC
r1783356
Comment 6 Javen O'Neal 2017-02-17 09:55:21 UTC
Created attachment 34761 [details]
patch with all of the unrelated code removed

Less than half the size of the original. Hopefully this makes it easier to review, though it still needs to be rebased to trunk.
Comment 7 Javen O'Neal 2017-02-17 09:56:01 UTC
(unrelated changes) close leaked resources in r1783362
Comment 8 Andreas Beeker 2018-04-27 21:39:40 UTC
Patch rebased, reviewed and committed via r1830400