Bug 38935 – [Qt][Symbian] data URIs cause crash at QFile layer

• Component: WebKit Qt
• Status: RESOLVED INVALID

[Siddharth Mathur]

Created attachment 55759 [details]
repro case

[reporting on behalf of Jyri-Petteri Huttunen and Tom Hechang as reported on s60qt mailing list. The fix might be cross-platform, if QtWebkit can be changed to avoid involving QFile at all in case of data URIs]


Use case: 
---------
We currently have an issue related to showing image in base64 format in Qwebkit. We create a simple qt application which has a qwebview and load a pic.html. The application run normally on windows, and will show a red cross on screen.

While we build the application on symbian^3/4, it will crash when start the app. We are wondering if this is a bug for qt webkit. The pic.html is attached. 


Prelim analysis by Shane Kearns: 
-------------------------------

The findBackend() function calls each backend factory in an iterator.
The first one to successfully process the request is used.
 
The file backend calls QFileInfo("data:.......").exists() which crashes inside open C.
open C needs to check the length of filenames passed to stat(), fopen() etc to prevent a buffer overrun panic when it is asked for a filename that is longer than the OS can support.
 
Once open C is fixed, then the exists() function would return false; and the data backend would be tried (and presumably succeed).

[Laszlo Gombos]

If this can not be addressed in OpenC - or lower in the stack - (which seems to be the best place), we should consider trying to address it in QtCore/QFileInfo before looking into changing QtWebKit.

[Simon Hausmann]

(In reply to comment #1)
> If this can not be addressed in OpenC - or lower in the stack - (which seems to be the best place), we should consider trying to address it in QtCore/QFileInfo before looking into changing QtWebKit.

I agree, this is not a QtWebKit issue at all. I think we should close this bug report.

Markus, what do you think about avoiding the stat on Qt?

Even though I agree that OpenC shouldn't crash (d'oh!), we shouldn't call QFileInfo::exists on data urls either.

[Markus Goetz]

I can try work around it in Qt, but OpenC needs to fix this too.

[Markus Goetz]

Actually this uncovered a "bug" in Qt. Fix is pending in Qt 4.6 staging repo as c1423c03d69f51d76b3629f2fedce555696759fa

This bug can be closed as soon it has been re-tested with a Qt with this commit. The patch is at http://qt.pastebin.com/XkRBg7dL ..

Still OpenC must not crash for too long filenames.

[Siddharth Mathur]

(In reply to comment #4)
> Actually this uncovered a "bug" in Qt. Fix is pending in Qt 4.6 staging repo as c1423c03d69f51d76b3629f2fedce555696759fa
> 
> This bug can be closed as soon it has been re-tested with a Qt with this commit. The patch is at http://qt.pastebin.com/XkRBg7dL ..
> 
> Still OpenC must not crash for too long filenames.

Does the above change fix the problem that file-system shouldn't be pinged at all when the raw image data is already supplied? 

If the file-system is still being touched, it is a performance problem. For 100 such icons, 0.001 seconds will be for decoding images and blitting them, 2 seconds for stat()ing files. :)

[Markus Goetz]

Don't worry :)
The order is now the "correct" one:

static void ensureInitialized()
{
#ifndef QT_NO_HTTP
    (void) httpBackend();
#endif // QT_NO_HTTP
    (void) dataBackend();
#ifndef QT_NO_FTP
    (void) ftpBackend();
#endif

#ifdef QT_BUILD_INTERNAL
    (void) debugpipeBackend();
#endif

    // leave this one last since it will query the special QAbstractFileEngines
    (void) fileBackend();
}

[Kenneth Rohde Christiansen]

(In reply to comment #6)
> Don't worry :)
> The order is now the "correct" one:
> 
> static void ensureInitialized()
> {
> #ifndef QT_NO_HTTP
>     (void) httpBackend();
> #endif // QT_NO_HTTP
>     (void) dataBackend();
> #ifndef QT_NO_FTP
>     (void) ftpBackend();
> #endif
> 
> #ifdef QT_BUILD_INTERNAL
>     (void) debugpipeBackend();
> #endif
> 
>     // leave this one last since it will query the special QAbstractFileEngines
>     (void) fileBackend();
> }

So it is fixed now? Can we close this bug?

[Siddharth Mathur]

OpenC to fix their crash in TRLM-84XDAF 
QtNetwork fix in c1423c03d69f51d76b3629f2fedce555696759fa 
(thanks Markus! :) )