The fix revision 1179452 was aimed to remove an NPE bug on the "zipArchive" in the method "close" of the file "/poi/trunk/src/ooxml/java/org/apache/poi/openxml4j/util/ZipFileZipEntrySource.java" , but it is incomplete. Since the "zipArchive" could be null during the run-time execution, its value should also be null-checked before being dereferenced in other methods. The buggy code locations the same fix needs to be applied at are as bellows: Line 44 of the method "getEntries()" : public Enumeration<? extends ZipEntry> getEntries() { [Line 44] return zipArchive.entries(); } Line 48 of the method "getInputStream()" : public InputStream getInputStream(ZipEntry entry) throws IOException { [Line 48] return zipArchive.getInputStream(entry); }
I'm not sure that zipArchive should ever be null - how are you triggering this?
If the method "close" is called, zipArchive will be null. After that, if the methods "getEntries()" and "getInputStream()" are called, NPE will happen at the code line 44 and 48. Please check the bug issue 51949 at https://issues.apache.org/bugzilla/show_bug.cgi?id=51949 for the method "close": public void close() throws IOException { zipArchive.close(); zipArchive = null; } Thanks.
What do you expect from "getEntries()" and "getInputStream()": return null or throw a more sensible exception? I would say that instead of the NPE they should throw IllegalStateException("zip file closed"), the way java.util.ZipFile does. Does it seem the right fix for you? Yegor (In reply to comment #2) > If the method "close" is called, zipArchive will be null. After that, if the > methods "getEntries()" and "getInputStream()" are called, NPE will happen at > the code line 44 and 48. > > Please check the bug issue 51949 at > https://issues.apache.org/bugzilla/show_bug.cgi?id=51949 for the method > "close": > > > public void close() throws IOException { > zipArchive.close(); > zipArchive = null; > } > > > > Thanks.
OK. Throwing an IllegalStateException("zip file closed") will be much better. Thanks.
As of r1244450, we now through an IllegalArgumentException for this case