[CURL] Crash if a local file has an unknow extension

[Marco Barisione]

MIMETypeRegistry::getMIMETypeForPath returns empty strings if the extension is not known. This also causes a crash because of the CURL backend setting the mime type to an empty string if the local file has an unknown extension.

From the code using the function and in particular from the comment in FrameLoaderClient::objectContentType() I think that the right behaviour is that getMIMETypeForPath should return "application/octet-stream" upon failure, while getMIMETypeForExtension should return an empty string.

[Marco Barisione]

Sorry, I realise only now that this is not GTK-specific code and that other ports have the same behaviour without breaking anything.

I still wonder why MIMETypeRegistry::getMIMETypeForPath() returns an empty string if the extension is not known but application/octet-stream if there is no extension, however the right fix is in the CURL code.

[Marco Barisione]

Created attachment 22023 [details]
Set the MIME type for files with an unknown extension to  "application/octet-stream"

[Wouter Bolsterlee]

> MIMETypeRegistry::getMIMETypeForPath returns
> empty strings if the extension is not known.

Why is that? Why not a null value or something like that?

[Eric Seidel (no email)]

Comment on attachment 22023 [details]
Set the MIME type for files with an unknown extension to  "application/octet-stream"

Why does curl crash when passed an empty string?  The right fix is right next to the line of curl code which is going to crash...  At least an ASSERT should go next to said line.

[Marco Barisione]

I think that we have 3 different problems:

1. RegularExpression::match() should not pass a null string to jsRegExpExecute. I opened a separate bug report for this, see bug #20295.

2. getMIMETypeForPath() returns "application/octet-stream" if there is not extension but getMIMETypeForExtension() returns a null string. If don't like this difference because it's confusing, but we have code that relies on that and also the win port has the same behaviour but it doesn't crash as it doesn't use the MIMETypeRegistry in this case. I don't know what the mac port does because it calls a function without public source code. The qt port always defaults to "application/octet-stream" so it's not affected by this problem. The soup back-end uses a function in gio for this instead of the MIMETypeRegistry, so it's not affected.
Unless someone (Alp?) thinks that changing getMIMETypeForExtension() to never return a null string is the right behaviour I think that my previous patch is useful, so I'm setting the review flag again

3. For now we don't have downloads in the GTK ports, so WebKit tries to show also unknown mime types. Of course this is wrong but crashing in this case is wronger :)

[Holger Freyther]

Comment on attachment 22023 [details]
Set the MIME type for files with an unknown extension to  "application/octet-stream"

Please patch MIMETypeRegistry::getMIMETypeForExtension (WebCore/platform/gtk/MIMETypeRegistryGtk.cpp) to have it return application/octet-stream by default, this would match with the default return of ::getMIMETypeForPath.

[Marco Barisione]

(In reply to comment #6)
> (From update of attachment 22023 [details] [edit])
> Please patch MIMETypeRegistry::getMIMETypeForExtension
> (WebCore/platform/gtk/MIMETypeRegistryGtk.cpp) to have it return
> application/octet-stream by default, this would match with the default return
> of ::getMIMETypeForPath.

Are you sure that this is the right behaviour? It's what I wanted to do at the beginning but then I found code relying on the null return value and also other ports do the same as the GTK one.

[Marco Barisione]

Any new opinion on this?

[Jan Alonzo]

(In reply to comment #8)
> Any new opinion on this?
> 

String MIMETypeRegistry::getMIMETypeForPath(const String& path) (platform/MIMETypeRegistry.cpp) already returns "application/octet-stream" if it can't find a mime type. We need to figure out why it's returning something else (empty string in this case) and not application/octet-stream. I would've been easier if you can provide at least a test url / case for this.

Here's the code i'm talking about:

String MIMETypeRegistry::getMIMETypeForPath(const String& path)
{
    int pos = path.reverseFind('.');
    if (pos >= 0) {
        String extension = path.substring(pos + 1);
        String result = getMIMETypeForExtension(extension);
        if (result.length())
            return result;
    }
    return "application/octet-stream";
}

[Gustavo Noronha (kov)]

Removing the Gtk keyword since Curl is no longer used by the GTK+ port.

[Brent Fulgham]

I wonder if this is the same as Bug 28312?

[Brent Fulgham]

I believe this is resolved by Bug 28312 for the following reasons:

1.  Full header processing prior to Bug 28312 was not being performed for local files.
2.  If the header processing is not performed, no URL is every set in the request object.
3.  If the URL is not set, the attempt to retrieve MIME type crashes due to null access in the HashMap.

Since there is no specific test case documented in this bug, and my attempts to access local files with meaningless extensions does not generate any failures with current WebKit builds, I am closing this bug as resolved.

If you do continue to see this bug, please provide a simple test case I can use to see what's going on.