[webkit-reviews] review granted: [Bug 51366] REGRESSION (r66452): Form data no longer contains 'Content-Type' header for files with unrecognized extensions : [Attachment 77061] Proposed Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Dec 21 13:19:28 PST 2010


Darin Adler <darin at apple.com> has granted Andy Estes <aestes at apple.com>'s
request for review:
Bug 51366: REGRESSION (r66452): Form data no longer contains 'Content-Type'
header for files with unrecognized extensions
https://bugs.webkit.org/show_bug.cgi?id=51366

Attachment 77061: Proposed Patch
https://bugs.webkit.org/attachment.cgi?id=77061&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=77061&action=review

Things I’d like to see after landing this:

    1) Functions in MIME type registry made consistent about how the express no
known MIME type. Null string seems a good standard; maybe we can move to that
everywhere.

    2) The "application/octet-stream" MIME type string in one place rather than
repeated multiple places. The model should be how we use the blankURL()
function for the "about:blank" URL.

    3) Test coverage for the Blob case we would have broken with Andy’s patch.

>> WebCore/platform/network/FormData.cpp:231
>> +			contentType = "application/octet-stream";
> 
> I think the more appropriate way to fix this is to change
createBlobDataForFile() to call MIMETypeRegistry::getMIMETypeForPath() rather
than MIMETypeRegistry::getMIMETypeForExtension(). getMIMETypeForPath() already
has the logic to default to 'application/octet-stream'. If we claim that
'application/octet-stream' is the appropriate default MIME type, we should
consolidate that logic in one place.

As we discussed, it’s fine to have the fix here, but things are a mess, with
multiple layers that do not agree on how to say “no known MIME type”. Lets get
this fixed, and then do a bit of cleanup.


More information about the webkit-reviews mailing list