theobisproject commented on a change in pull request #97:
URL: https://github.com/apache/commons-compress/pull/97#discussion_r426733165
##########
File path:
src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveEntry.java
##########
@@ -368,10 +397,74 @@ public TarArchiveEntry(final File file) {
* @param fileName the name to be used for the entry.
*/
public TarArchiveEntry(final File file, final String fileName) {
+ final String normalizedName = normalizeFileName(fileName, false);
+ this.file = file.toPath();
+
+ try {
+ readFileMode(this.file, normalizedName);
+ } catch (IOException e) {
+ // Ignore for backwards compatibility
Review comment:
I don't think this will ever work without ignoring the exception here.
Have a look at TarArchiveEntryTest#testTarFileWithFSRoot. On Windows the
entries are directly added from the C:\ drive. They don't have to exist that
the test passes with the old File API. But the NIO API tries to read the file
size from disk. When it is not present it will throw an NoSuchFileException.
Also throwing a runtime exception here is in my opinion a really bad idea.
This is totally unexpected for the user since he only notices it at runtime.
With a checked exception he would directly notice it after upgrading. But since
changing the API is no option I think we are only left with swallowing the
error.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
