After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 310015 - crash when listing 7-zip archive with 7za 4.x
crash when listing 7-zip archive with 7za 4.x
Status: RESOLVED FIXED
Product: file-roller
Classification: Applications
Component: general
unspecified
Other Linux
: High major
: ---
Assigned To: Paolo Bacchilega
file-roller-maint
Depends on:
Blocks:
 
 
Reported: 2005-07-11 12:07 UTC by Claudio Bley
Modified: 2006-04-04 13:58 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
fix for bug #310015 (1.41 KB, patch)
2005-07-11 12:11 UTC, Claudio Bley
committed Details | Review
really fixes #310015 this time (1.69 KB, patch)
2005-07-12 16:56 UTC, Claudio Bley
none Details | Review

Description Claudio Bley 2005-07-11 12:07:00 UTC
file-roller expects 7za to print 6 fields when listing an archive, but 7za v4.x
sometimes leaves out the 5th field (probably a space optimization for solid
archives?!) which makes file-roller segfault on the input, because the name
field - usually the 6th item - is not there.

Additionally, file-roller removes trailing spaces of the name field which I
think is not right, cause when there are trailing spaces, they are part of the
file name!

I also found a memory leak in fr-command-rar.c where the string vector "fields"
might not get freed (when the entry is a directory for example).

The attached patch should fix these problems.
Comment 1 Claudio Bley 2005-07-11 12:11:20 UTC
Created attachment 48947 [details] [review]
fix for bug #310015
Comment 2 Paolo Bacchilega 2005-07-11 15:23:15 UTC
patch applied, thank you.
Comment 3 Claudio Bley 2005-07-12 14:30:22 UTC
NO, wait. The patch doesn't fix the problem with 7-zip archive listings. I just
came across an archive which didn't work yesterday right after I submitted this
bug. (it didn't segfault, so we might call this an improvement).

The problem is that one can't tell without looking at the format (or numbers of
whitespace) of the line whether the 5th field is missing or whether the file
itself has some whitespace in it:

$ 7za l -bd -y foo.7z
   Date      Time    Attr         Size   Compressed  Name
------------------- ----- ------------ ------------  ------------
[...]
2005-07-04 01:00:00 ....A      4256252     24016007  trailer siegfried.mov
2005-07-05 01:00:00 ....A      4970789               trailer antikoerper.swf
2005-07-06 15:39:13 ....A      7815544               antikoerper.wmv


It works for the first and third line, but the second line has 6 fields although
the "Compressed" field is missing, but the word "trailer" is regarded as a field
on its own.

Unfortunately, I had no time to prepare a new patch. 

I think it would be best to scan the line with the dashes for the last occurence
of a space (which would be the boundary before the name field begins) and
remember its index. Now, when processing the file lines you only have to skip
(index+1) chars and the rest of the line is the file name (including leading and
trailing spaces).

I'll implement that if I got the time, nobody has a better idea and nobody is
faster (on the draw).
Comment 4 Claudio Bley 2005-07-12 16:56:56 UTC
Created attachment 49032 [details] [review]
really fixes #310015 this time

This patch implements the method I already proposed in my last comment to this
bug.

+ src/fr-command-7z.h: added a new member "gint name_index;" to struct
_FRCommand7z 

+ src/fr-command-7z.c: added new function "static gint str_rfind(char*, int)"
which finds the last occurence of a character in string and returns the index
of it or -1 if not found.

  (process_line): scan each "-----" line for the last space character, remember
its index in p7z_comm->name_index; name_field = line+p7z_comm->name_index.
 just read the first 4 fields with split_line because the 5th field is not used
at all and sometimes it's not even there.

Please re-re-read the patch carefully before applying it anyways. :-)

Btw, the patch is against current CVS. I tried to reverse the old patch and
making a new one obsoleting the old, but I didn't succeed.

(and sorry for all the hassle)
Comment 5 Olav Vitters 2005-07-21 17:24:11 UTC
Attachment 48947 [details] was apparently commit, so even if it needed work, marking as
committed. 

Still one patch outstanding.
Comment 6 Paolo Bacchilega 2005-07-25 18:11:14 UTC
second patch applied.