Bug 103865 – [Qt] Don't rely on QMimeDatabase for essential MIME types

• Component: New Bugs
• Status: RESOLVED FIXED

[Pierre Rossi]

[Qt] Don't rely on QMimeDatabase for well-known extension to MIME type mapping.

[Pierre Rossi]

Created attachment 177202 [details]
Patch

[Allan Sandfeld Jensen]

Comment on attachment 177202 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=177202&action=review

> Source/WebCore/platform/qt/MIMETypeRegistryQt.cpp:87
>  String MIMETypeRegistry::getMIMETypeForPath(const String& path)
>  {
> +    size_t pos = path.reverseFind('.');
> +    if (pos != notFound) {
> +        String extension = path.substring(pos + 1);
> +        String result = getWellKnownMIMETypeForExtension(extension);
> +        if (result.length())
> +            return result;
> +    }
> +

Why is the change in getMIMETypeForPath? it is getMIMETypeForExtension that is used to determine if we should/can load content.

[Allan Sandfeld Jensen]

Btw, I think we should add all the aliases of the supported mime-types so that we don't end up in that situation.

[Pierre Rossi]

(In reply to comment #2)
> (From update of attachment 177202 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=177202&action=review
> 
> > Source/WebCore/platform/qt/MIMETypeRegistryQt.cpp:87
> >  String MIMETypeRegistry::getMIMETypeForPath(const String& path)
> >  {
> > +    size_t pos = path.reverseFind('.');
> > +    if (pos != notFound) {
> > +        String extension = path.substring(pos + 1);
> > +        String result = getWellKnownMIMETypeForExtension(extension);
> > +        if (result.length())
> > +            return result;
> > +    }
> > +
> 
> Why is the change in getMIMETypeForPath? it is getMIMETypeForExtension that is used to determine if we should/can load content.

It's used in QNetworkReplyHandler::sendResponseIfNeeded when loading local files.

[Allan Sandfeld Jensen]

It seems some tools used for creating associating mimetypes to application will dynamical invent new mimetypes such as application/x-extension-html, which can confuse the mimedatabase because we only rely on the first match. 

We need to ensure users can not shoot themselves this hard with the basic mimetypes. For images and other types it can be potentially useful, but for html and other essential types it can only break stuff.

[Allan Sandfeld Jensen]

Created attachment 177443 [details]
Patch

A condensed version of Pierre's patch only detecting the essential mimetypes and doing it for both getMimetype functions

[Jocelyn Turcotte]

Comment on attachment 177443 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=177443&action=review

> Source/WebCore/ChangeLog:9
> +        Extend the short static list to also include MIME types essential WebKit,

to WebKit

> Source/WebCore/platform/qt/MIMETypeRegistryQt.cpp:59
> +    { "xls", ".xls", "text/xsl" },

I didn't know we supported Excel sheets with WebKit :P

> Source/WebCore/platform/qt/MIMETypeRegistryQt.cpp:76
> +    // FIXME: We should get all the matched mimetypes with mimeTypesForFileName, and prefer one we support.

mimeTypesForFileName -> mimeTypesForFile?

> Source/WebCore/platform/qt/MIMETypeRegistryQt.cpp:77
> +    // But initializeSupportedImageMIMETypes will first have to stop using getMIMETypeForExtension.

Could you write why? I don't have enough context to understand this part.

[Allan Sandfeld Jensen]

(In reply to comment #7)
> (From update of attachment 177443 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=177443&action=review
> > Source/WebCore/platform/qt/MIMETypeRegistryQt.cpp:76
> > +    // FIXME: We should get all the matched mimetypes with mimeTypesForFileName, and prefer one we support.
> 
> mimeTypesForFileName -> mimeTypesForFile?
> 
Well, that is actually the name of this particular method in QMimeDatabase ;)

> > Source/WebCore/platform/qt/MIMETypeRegistryQt.cpp:77
> > +    // But initializeSupportedImageMIMETypes will first have to stop using getMIMETypeForExtension.
> 
> Could you write why? I don't have enough context to understand this part.

Because to tell if a mime-type is supported we have to check against the supported mime-types, but we can not do that while we are still in the process of initializing the supported mime-types.

[Allan Sandfeld Jensen]

Created attachment 177707 [details]
Patch

[Jocelyn Turcotte]

Comment on attachment 177707 [details]
Patch

(In reply to comment #8)
> Well, that is actually the name of this particular method in QMimeDatabase ;)

Ah! I see what you mean, sorry about that.

[WebKit Review Bot]

Comment on attachment 177707 [details]
Patch

Clearing flags on attachment: 177707

Committed r136669: <http://trac.webkit.org/changeset/136669>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.