Failure while executing KTar::open while using KCompressionDevice as the device

[Aleix Pol]

On Mon, Nov 23, 2015 at 4:54 PM, Luiz Romário Santana Rios
<luizromario at gmail.com> wrote:
> 2015-11-02 14:53 GMT-03:00 Luiz Romário Santana Rios <luizromario at gmail.com>:
>> Hello,
>>
>> I'm trying to decompress a XZ archive downloaded using
>> QNetworkAccessManager, so, according to the documents, I have to pass
>> the QNetworkReply pointer to a KCompressionDevice and, then, use it as
>> Ktar's device like this:
>>
>> https://gist.github.com/anonymous/b8fb686367f518a7dbb5
>>
>> The problem is that KTar::open() fails and returns false. The file I'm
>> trying to extract has the following structure more or less:
>> /root
>> /root/dir
>> /root/dir/file1
>> /root/dir/file2
>> ...
>>
>> So, as far as I've seen, the code runs normally when entering /root
>> and /root/dir, but, pretty high in the stack, at
>> KXzFilter::uncompress(), the call to lzma_code returns
>> LZMA_FORMAT_ERROR while trying to uncompress file1 (or file2, I'm not
>> sure). Here's the call stack:
>>
>> https://gist.github.com/anonymous/9ea380cfe48daadb5971
>>
>> Is this a bug? If it's a bug, how can I proceed to fix it?
>>
>> Thanks for the attention.
>>
>> --
>> Luiz Romário Santana Rios
>
> After some discussion in a review request
> (https://git.reviewboard.kde.org/r/125974/), I found out that the
> problem is that QNetworkReply is a sequential device. This, as I
> understand it, is only a problem because KCompressionDevice is able to
> open any file it wants in any sequence; after all, tars are
> serializable, which means there's no need to seek back to extract a
> stream of data. Knowing that, I can think of two solutions:
>
> - Make KCompressionDevice check if the device it is receiving is
> sequential. If it is, remove the ability to open any file and just
> ensure KTar::copyTo() works properly.
> - Check if the device passed to KCompressionDevice is sequential and
> make it invalid if it is; create a new KSequentialCompressionDevice
> class which only extracts the data from a sequential QIODevice and
> does not have the ability to extract individual files in any order
> like KCompressionDevice.
>
> In short: either limit the functionalities of KCompressionDevice if
> the device is sequential or forbid sequential devs in
> KCompressionDevice and create a new class to handle them. If
> KCompressionDevice relies too much on the device being sequential, the
> second option is the way to go.
>
> What do you think?

I don't think you want to do something like:
if (dev->isSequiential()) {
   new KCopyCompressionDevice;
   //do stuff
} else {
   new KCompressionDevice;
   //do essentially the same stuff
}

We aren't adding any semantics by offering a new class. I'd say that
KCompressionDevice should either take it all or just assert on
isSequential and expect the users to actually implement it on the app
side.

Aleix