Hi Stefan, thanks for your nice reply! We are using tar as part of our software to prepare data sent to devices that use tar or tgz format for receiving e.g.: CSV, XML,... files
Some more info 1) I agree that this NPE might not occur under normal circumstances. Therefore, I am quite sure that the finding is due to our static code analysis tool remarks and just some beautification. But danger to introduce problems is quite low. 2) we did this fix some 10-15 years ago since we had some problem with one of our embedded devices that is not very fault-tolerant. But to be true, that's all I remember. Big sorry! We patched the code at this time and - shame on me - did not report this back to the community ☹ Since that time we used the adapted function without any problem. Your suggestion sounds very good as best combination of two ideas. But since our fix was introduced such a long time ago my hope goes for some detailed analysis of the code by the maintainer - maybe due to some other code changes, my proposal is not needed anymore or would introduce new problems. Br Michael -----Original Message----- From: Stefan Bodewig <bode...@apache.org> Sent: Sonntag, 26. Februar 2023 12:35 To: dev@ant.apache.org Subject: Re: Improvement ideas for Ant 1.10.13 *EXTERNAL source* Hi Michael many thanks for your suggestions. You could have just opened enahncement requests in Bugzilla but personally I appreciate you trying to have a discussion about the changes here first. I am wondering whether you are using the tar package directly or you are actually facing issues with the code as it is while running Ant. In the former case I'd suggest you move to Apache Commons Compress which contains an improved offspring of Ant's tar package - it may even already contain fixes that are nor present in Ant's original code as they have not been backported. On 2023-02-23, KUNES Michael wrote: > 1) org.apache.tools.tar.TarEntry > The method public TarEntry[] getDirectoryEntries() could be improved to make > a null check for > final String[] list = this.file.list(); because the result of this > statement could be null and in this case the next statement would fail > with NPE you are correct. Commons Compress solves this by not invoking #list anymore https://github.com/apache/commons-compress/blob/master/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveEntry.java#L856 The case of #list returning null is a curious case since it probably represents and I/O error according to the javadoc - the other reason file not being a directory has already been checked for. Still returning an empty array probably is a better fallback than throwing an NPE. > 2) org.apache.tools.tar.TarBuffer > The method writeBlock() has this statement > this.outStream.write(this.blockBuffer, 0, this.blockSize); we > think, that this statement > this.outStream.write(this.blockBuffer, 0, this.currRecIdx * > this.recordSize); would be a better solution > Reason: with original code, after the EOF-blocks, the remaining data > (garbage) from the last block was also put to the outstream Doesn't the Arrays.fill at the end of writeBlock ensure there is no remaining garbage in the block buffer? Actually Common Compress writes records directly instead of batching them to full blocks and only ensures EOF is padded with 0-blocks to fill the last block, so it effectively avoid the garbage problem you talk about. The proper fix here probably would be to apply your suggestion and in addition write enough data to fill the rest with 0s - or make sure blckbuffer is padded with zeros before writing it completely. Sounds a lot like https://issues.apache.org/jira/browse/COMPRESS-81 which lead me to find https://bz.apache.org/bugzilla/show_bug.cgi?id=47421 which suggest the same change you did. And back then I changed the code to zero out the block buffer. You said you were using th ecode of an older version of Ant, maybe you haven't applied https://svn.apache.org/viewvc/ant/core/trunk/src/main/org/apache/tools/tar/TarBuffer.java?r1=789556&r2=789555&pathrev=789556 ? If you are already using the current master code then I wonder how the garbage gets into the blockBuffer so we can better understand how to fix this. Cheers Stefan --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional commands, e-mail: dev-h...@ant.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional commands, e-mail: dev-h...@ant.apache.org
