GNOME Bugzilla – Bug 670534
Reading empty files with gvfsd-archive
Last modified: 2013-11-07 11:48:33 UTC
Created attachment 208106 [details] [review] Patch to determine file size from data blocks. If a size of a file included in an archive is not set in an archive header, then the size is set to zero and consequently the read file is empty. The bug is similar with the bug in the bsdcpio at http://code.google.com/p/libarchive/issues/detail?id=218&thanks=218&ts=1325281908 (there is also attached the problem archive). I attached the possible patch, which determine the size from an archive entry, but this is ugly for a large files. An another solution is do not set a zero size, if the size is not known, but recently for example nautilus can not handle it.
Review of attachment 208106 [details] [review]: This shouldn't be a problem for big files with no size entry because the entire file is read through in create_file_tree anyway. It just means that archive_read_data_skip() will be a no-op. ::: daemon/gvfsbackendarchive.c @@ +413,3 @@ + int result; + const void *block; + int64_t offset; offset is declared as off_t over here: https://github.com/libarchive/libarchive/wiki/ManPageArchiveReadData3 @@ +419,3 @@ + { + result = archive_read_data_block (archive->archive, &block, &read, &offset); + if (result != ARCHIVE_OK) I don't really know libarchive well, but if an error occurs shouldn't the whole process be aborted (I see a similar approach is taken in create_file_tree)?
(In reply to comment #1) > Review of attachment 208106 [details] [review]: > > This shouldn't be a problem for big files with no size entry because the entire > file is read through in create_file_tree anyway. It just means that > archive_read_data_skip() will be a no-op. You are right... > ::: daemon/gvfsbackendarchive.c > @@ +413,3 @@ > + int result; > + const void *block; > + int64_t offset; > > offset is declared as off_t over here: > https://github.com/libarchive/libarchive/wiki/ManPageArchiveReadData3 Libarchive 3.0 and newer uses int64_t, see: https://github.com/libarchive/libarchive/wiki/ReleaseNotes#wiki-Libarchive_302 So, it should be fixed also in another places. > @@ +419,3 @@ > + { > + result = archive_read_data_block (archive->archive, &block, &read, > &offset); > + if (result != ARCHIVE_OK) > > I don't really know libarchive well, but if an error occurs shouldn't the whole > process be aborted (I see a similar approach is taken in create_file_tree)? It should be...
Created attachment 258729 [details] [review] fix wrong data types Libarchive 3.0 and newer uses int64_t as mentioned in the previous comment...
Created attachment 258730 [details] [review] fix segfault when libarchive fails Fix error handling slightly to avoid segfaults...
Created attachment 258731 [details] [review] fix reading files when size not set Abort when fatal error occurs when determining size...
Review of attachment 258729 [details] [review]: I guess their docs need to be updated. Looks good.
Review of attachment 258730 [details] [review]: Looks good.
Review of attachment 258731 [details] [review]: Looks good.
Comment on attachment 258729 [details] [review] fix wrong data types commit d3d6f93381b5ea20ee34dd6125c1638527d42ff9
Comment on attachment 258730 [details] [review] fix segfault when libarchive fails commit 41266f89894dabcc502d1bf1be65592d23eb24cc
Comment on attachment 258731 [details] [review] fix reading files when size not set commit 39ab8a0f6f673696941a681cf11d2fbbb17bdccf