Bug 331065 - rar: Separate the code handling different RAR releases
Summary: rar: Separate the code handling different RAR releases
Status: RESOLVED FIXED
Alias: None
Product: ark
Classification: Applications
Component: general (show other bugs)
Version: unspecified
Platform: unspecified FreeBSD
: NOR task
Target Milestone: ---
Assignee: Raphael Kubo da Costa
URL:
Keywords: junior-jobs
Depends on:
Blocks:
 
Reported: 2014-02-12 22:15 UTC by Raphael Kubo da Costa
Modified: 2015-09-13 16:45 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In: 15.12.0
Sentry Crash Report:


Attachments
May be solved bug 331065. (15.54 KB, patch)
2014-03-11 14:48 UTC, ck Lux
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Raphael Kubo da Costa 2014-02-12 22:15:06 UTC
The current parser code in clirarplugin's cliplugin.cpp is a big mess: we need to handle RAR v4, RAR v5 and unrar-free and they all have different output formats.

Having just a big switch to handle the parser's state made sense when we only supported RAR v3 and v4 (which had the same output format), but now a method that's ~230-lines long and is hard to follow.

We could first identify the RAR version we're dealing with and instantiate a different parser based on that.
Comment 1 Christoph Feck 2014-02-12 22:32:55 UTC
Agreed, and sorry for the mess :)
Comment 2 Raphael Kubo da Costa 2014-02-12 22:49:58 UTC
Git commit e13da507c72490a7c5a594d646f040e812fa9d15 by Raphael Kubo da Costa.
Committed on 12/02/2014 at 22:46.
Pushed by rkcosta into branch 'KDE/4.12'.

rar: Do not crash when unrar lists a symlink.

RAR v3 and v4 output an extra line when an entry is a symlink. This line
contains the symlink target. Since we were not accounting for this extra
line before, our parser crashed.

Work around the issue by checking if we're listing a symlink and skipping an
extra line if so.
FIXED-IN:	4.12.3

M  +17   -1    plugins/clirarplugin/cliplugin.cpp
M  +1    -0    plugins/clirarplugin/cliplugin.h
M  +25   -1    plugins/clirarplugin/tests/clirartest.cpp
M  +1    -0    plugins/clirarplugin/tests/clirartest.h
A  +22   -0    plugins/clirarplugin/tests/data/testReadArchiveWithSymlink.txt

http://commits.kde.org/ark/e13da507c72490a7c5a594d646f040e812fa9d15
Comment 3 Raphael Kubo da Costa 2014-02-12 22:51:28 UTC
Crap, I shouldn't keep multiple Bugzilla tabs open at the same time :-)

This commit was supposed to refer to bug 314297.
Comment 4 ck Lux 2014-03-11 14:48:51 UTC
Created attachment 85536 [details]
May be solved bug 331065.
Comment 5 Raphael Kubo da Costa 2014-03-18 23:04:47 UTC
ck Lux's patch is being tracked at https://git.reviewboard.kde.org/r/116721/
Comment 6 Ragnar Thomsen 2015-09-13 16:45:36 UTC
Git commit b738a0041a44ad9c36644aa903c6621ac9e265e9 by Ragnar Thomsen.
Committed on 13/09/2015 at 16:45.
Pushed by rthomsen into branch 'master'.

clirar: Rewrite unrar parsing code

readListLine() is now very short. It simply checks the version of unrar
and calls line-handling functions specific for unrar 3/4 and 5 termed
handleUnrar4Line() and handleUnrar5Line(), respectively.

When the line-handling functions have completed parsing all the details
of an archive entry, they call an entry-handling function, either
handleUnrar4Entry() or handleUnrar5Entry().
FIXED-IN: 15.12.0
REVIEW: 124503

M  +329  -191  plugins/clirarplugin/cliplugin.cpp
M  +19   -9    plugins/clirarplugin/cliplugin.h

http://commits.kde.org/ark/b738a0041a44ad9c36644aa903c6621ac9e265e9