19794 2008-06-27 11:48:02 -0700 [CURL] Crash if a local file has an unknow extension 2009-08-17 15:51:35 -0700 1 1 1 Unclassified WebKit Page Loading 528+ (Nightly build) All All RESOLVED FIXED Curl P2 Major --- 1 marco.barisione marco.barisione ahya365 bfulgham jmalonzo marco.barisione uws+webkit oldest_to_newest 84597 0 marco.barisione 2008-06-27 11:48:02 -0700 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. 84599 1 marco.barisione 2008-06-27 12:05:15 -0700 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. 84845 2 22023 marco.barisione 2008-07-01 03:46:59 -0700 Created attachment 22023 Set the MIME type for files with an unknown extension to "application/octet-stream" 86090 3 uws+webkit 2008-07-16 12:29:03 -0700 > MIMETypeRegistry::getMIMETypeForPath returns > empty strings if the extension is not known. Why is that? Why not a null value or something like that? 87652 4 22023 eric 2008-08-01 16:57:52 -0700 Comment on attachment 22023 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. 87937 5 marco.barisione 2008-08-06 03:59:12 -0700 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 :) 88261 6 22023 zecke 2008-08-11 18:24:06 -0700 Comment on attachment 22023 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. 88294 7 marco.barisione 2008-08-12 02:12:10 -0700 (In reply to comment #6) > (From update of attachment 22023 [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. 92539 8 marco.barisione 2008-09-24 09:21:14 -0700 Any new opinion on this? 96633 9 jmalonzo 2008-10-24 20:08:47 -0700 (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"; } 111331 10 gustavo 2009-02-26 15:33:31 -0800 Removing the Gtk keyword since Curl is no longer used by the GTK+ port. 140376 11 bfulgham 2009-08-14 16:09:43 -0700 I wonder if this is the same as Bug 28312? 140854 12 bfulgham 2009-08-17 15:51:35 -0700 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. 22023 2008-07-01 03:46:59 -0700 2008-08-11 18:24:06 -0700 Set the MIME type for files with an unknown extension to "application/octet-stream" curl-unknown-extension.patch text/plain 1838 marco.barisione SW5kZXg6IFdlYkNvcmUvcGxhdGZvcm0vbmV0d29yay9jdXJsL1Jlc291cmNlSGFuZGxlTWFuYWdl ci5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PQotLS0gV2ViQ29yZS9wbGF0Zm9ybS9uZXR3b3JrL2N1cmwvUmVzb3Vy Y2VIYW5kbGVNYW5hZ2VyLmNwcAkocmV2aXNpb25lIDM0OTExKQorKysgV2ViQ29yZS9wbGF0Zm9y bS9uZXR3b3JrL2N1cmwvUmVzb3VyY2VIYW5kbGVNYW5hZ2VyLmNwcAkoY29waWEgbG9jYWxlKQpA QCAtMyw2ICszLDcgQEAKICAqIENvcHlyaWdodCAoQykgMjAwNiBNaWNoYWVsIEVtbWVsIG1pa2Uu ZW1tZWxAZ21haWwuY29tCiAgKiBDb3B5cmlnaHQgKEMpIDIwMDcgQWxwIFRva2VyIDxhbHAudG9r ZXJAY29sbGFib3JhLmNvLnVrPgogICogQ29weXJpZ2h0IChDKSAyMDA3IEhvbGdlciBIYW5zIFBl dGVyIEZyZXl0aGVyCisgKiBDb3B5cmlnaHQgKEMpIDIwMDggQ29sbGFib3JhIEx0ZC4KICAqIEFs bCByaWdodHMgcmVzZXJ2ZWQuCiAgKgogICogUmVkaXN0cmlidXRpb24gYW5kIHVzZSBpbiBzb3Vy Y2UgYW5kIGJpbmFyeSBmb3Jtcywgd2l0aCBvciB3aXRob3V0CkBAIC01NjYsNyArNTY3LDExIEBA CiAgICAgICAgIGlmICghcXVlcnkuaXNFbXB0eSgpKQogICAgICAgICAgICAgdXJsID0gdXJsLmxl ZnQodXJsLmZpbmQocXVlcnkpKTsKICAgICAgICAgLy8gRGV0ZXJtaW5lIHRoZSBNSU1FIHR5cGUg YmFzZWQgb24gdGhlIHBhdGguCi0gICAgICAgIGQtPm1fcmVzcG9uc2Uuc2V0TWltZVR5cGUoTUlN RVR5cGVSZWdpc3RyeTo6Z2V0TUlNRVR5cGVGb3JQYXRoKHVybCkpOworICAgICAgICBTdHJpbmcg bWltZVR5cGUgPSBNSU1FVHlwZVJlZ2lzdHJ5OjpnZXRNSU1FVHlwZUZvclBhdGgodXJsKTsKKyAg ICAgICAgaWYgKCFtaW1lVHlwZS5pc0VtcHR5KCkpCisgICAgICAgICAgICBkLT5tX3Jlc3BvbnNl LnNldE1pbWVUeXBlKG1pbWVUeXBlKTsKKyAgICAgICAgZWxzZQorICAgICAgICAgICAgZC0+bV9y ZXNwb25zZS5zZXRNaW1lVHlwZSgiYXBwbGljYXRpb24vb2N0ZXQtc3RyZWFtIik7CiAgICAgfQog CiAgICAgZC0+bV9oYW5kbGUgPSBjdXJsX2Vhc3lfaW5pdCgpOwpJbmRleDogV2ViQ29yZS9DaGFu Z2VMb2cKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PQotLS0gV2ViQ29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uZSAzNDkxMSkK KysrIFdlYkNvcmUvQ2hhbmdlTG9nCShjb3BpYSBsb2NhbGUpCkBAIC0xLDMgKzEsMTYgQEAKKzIw MDgtMDctMDEgIE1hcmNvIEJhcmlzaW9uZSAgPG1hcmNvLmJhcmlzaW9uZUBjb2xsYWJvcmEuY28u dWs+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgW0NV UkxdIENyYXNoIGlmIGEgbG9jYWwgZmlsZSBoYXMgYW4gdW5rbm93biBleHRlbnNpb24KKyAgICAg ICAgaHR0cDovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTk3OTQKKworICAgICAg ICBTZXQgdGhlIE1JTUUgdHlwZSBmb3IgZmlsZXMgd2l0aCBhbiB1bmtub3duIGV4dGVuc2lvbiB0 bworICAgICAgICAiYXBwbGljYXRpb24vb2N0ZXQtc3RyZWFtIi4KKworICAgICAgICAqIHBsYXRm b3JtL25ldHdvcmsvY3VybC9SZXNvdXJjZUhhbmRsZU1hbmFnZXIuY3BwOgorICAgICAgICAoV2Vi Q29yZTo6UmVzb3VyY2VIYW5kbGVNYW5hZ2VyOjppbml0aWFsaXplSGFuZGxlKToKKwogMjAwOC0w Ny0wMSAgQWxwIFRva2VyICA8YWxwQG51YW50aS5jb20+CiAKICAgICAgICAgUnViYmVyLXN0YW1w ZWQgYnkgSG9sZ2VyLgo=