Review Request: Don't hang when determining MIME type of corrupted files

[David Faure]

On Monday 22 August 2011 20:12:13 Miroslav Ľos wrote:
> > On Aug. 21, 2011, 10:07 a.m., David Faure wrote:
> > > Thanks Peter and Miroslav. The analysis looks correct, the pre-read part
> > > of the patch looks good. I'm just wondering about using Unbuffered. If
> > > someone installs a mimetype definition with multiple rules trying to
> > > match some bytes after the 2K limit, then all this seeking-and-reading
> > > back and forth will be very slow, in unbuffered mode (since neither
> > > cache will be used).
> I find Unbuffered causing slowness improbable. From what I've seen in uses
> of KMimeType::findBy\w*Content in kdelibs, they all (e.g. ftp* and http
> kioslaves) rather provide their own buffer in a QByteArray rather than a
> QIODevice; most just provide the path (which is only opened using QFile if
> it is_local_file).

The most common case for mimetype determination is from the file manager 
listing local files, in which case findByUrl will use a QFile for content-
determination.

Ah, and BTW I have just found a magic rule that needs 4K of data:
vmware-player.xml says:
  <match type="string" value='config.version = "' offset="0:4096"/>

> All QFile's buffering is implemented in the QIODevice superclass, it adds
> Unbuffered in open() to its openMode for its fileEngine() backend. Thus, no
> buffering is propagated down.

I'm not sure what you mean there.

> The unnecessary 16K read did hit several more
> EIO's on the broken CD I have, but it is just cosmetic I guess.

Yes I'm not sure it's worth optimizing for this special case.

> Nonetheless, I find most uses of these functions ever will be through QFile
> or QBuffer

Sure. It's QFile that I have in mind here, when I say that seeking back and 
forth will be slow, in unbuffered mode.

> Finally, I wonder if buffering makes a difference as we are only using a few
> small block reads, not many getChar()'s.

It's about seeking and reading, vs just having the data in memory.

> * ftp passes a 1K-capped buffer (kioslave/ftp/ftp.cpp:2471). That may be
> insufficient for the single rule on my system that needed 1029 bytes.

Right. Small bug, but a corner case (only matters if extension unknown).

-- 
David Faure, faure at kde.org, http://www.davidfaure.fr
Sponsored by Nokia to work on KDE, incl. Konqueror (http://www.konqueror.org).