RESOLVED FIXED 102778
should sniff it if mimetype don't contain slash
https://bugs.webkit.org/show_bug.cgi?id=102778
Summary should sniff it if mimetype don't contain slash
Mary Wu
Reported 2012-11-20 00:06:02 PST
the test site khanapp.com Content-Type: web; charset=utf-8 Both Chrome and Firefox handle this. http://mimesniff.spec.whatwg.org/ says "If the Content-Type header field is absent or if its value cannot be interpreted as a media type (e.g. because its value doesn't contain a U+002F SOLIDUS ('/') character), then there is no official-type." We should use sniffing here since the Content-Type is invalid.
Attachments
Patch (2.14 KB, patch)
2012-11-20 00:21 PST, Mary Wu
no flags
Patch (1.86 KB, patch)
2012-11-21 23:15 PST, Mary Wu
no flags
Patch (1.97 KB, patch)
2012-12-02 18:40 PST, Mary Wu
no flags
Mary Wu
Comment 1 2012-11-20 00:21:16 PST
Yong Li
Comment 2 2012-11-20 07:33:52 PST
Comment on attachment 175166 [details] Patch What's the problem here? 1) Just checking '/' isn't enough to identify all malformed MIME type strigns. 2) What is different between malformed MIME strings and unknown MIME strings? And why should we treat them differently?
Yong Li
Comment 3 2012-11-20 07:34:18 PST
Comment on attachment 175166 [details] Patch Also, if it does appear to be problem, we need a http test
Brady Eidson
Comment 4 2012-11-20 16:49:32 PST
(In reply to comment #0) > the test site khanapp.com > Content-Type: web; charset=utf-8 > > Both Chrome and Firefox handle this. Great... so if Chrome handles it at least one WebKit port does. Have you tried Safari? > http://mimesniff.spec.whatwg.org/ says "If > the Content-Type header field is absent or if its value cannot be interpreted > as a media type (e.g. because its value doesn't contain a U+002F SOLIDUS ('/') > character), then there is no official-type." I agree, that is the language of the spec. At least the snippet you include doesn't specify what browsers should do in this case. Do you have a bug to report here? In other words, is this causing problems at a real world site? > We should use sniffing here since the Content-Type is invalid. Some platforms sniff all network traffic (except for very specific scenarios) since the network is an untrusted place. Are you trying to fix only one specific platform here?
Mary Wu
Comment 5 2012-11-21 23:00:19 PST
(In reply to comment #4) > (In reply to comment #0) > > the test site khanapp.com > > Content-Type: web; charset=utf-8 > > > > Both Chrome and Firefox handle this. > > Great... so if Chrome handles it at least one WebKit port does. Have you tried Safari? yes, safari also works. > > > http://mimesniff.spec.whatwg.org/ says "If > > the Content-Type header field is absent or if its value cannot be interpreted > > as a media type (e.g. because its value doesn't contain a U+002F SOLIDUS ('/') > > character), then there is no official-type." > > I agree, that is the language of the spec. At least the snippet you include doesn't specify what browsers should do in this case. > > Do you have a bug to report here? In other words, is this causing problems at a real world site? khanapp.com was the site reported, don't have other sites at hand. > > > We should use sniffing here since the Content-Type is invalid. > > Some platforms sniff all network traffic (except for very specific scenarios) since the network is an untrusted place. > > Are you trying to fix only one specific platform here? It looks only Qt/BlackBerry uses MIMESniffing.cpp, so with this patch, qt/blackberry will work the same way as firefox/chrome/safari. Besides, Chrome porting also uses similar solution in its mime_sniffer.cc to check if mimetype contains slash '/' and commented it's following firefox.
Mary Wu
Comment 6 2012-11-21 23:04:00 PST
(In reply to comment #2) > (From update of attachment 175166 [details]) > What's the problem here? > 1) Just checking '/' isn't enough to identify all malformed MIME type strigns. > 2) What is different between malformed MIME strings and unknown MIME strings? And why should we treat them differently? "malform" would be too general, I will narrow down the subject. Come up with new patch...
Mary Wu
Comment 7 2012-11-21 23:15:35 PST
Brady Eidson
Comment 8 2012-11-22 10:10:03 PST
(In reply to comment #5) > (In reply to comment #4) > It looks only Qt/BlackBerry uses MIMESniffing.cpp, so with this patch, qt/blackberry will work the same way as firefox/chrome/safari. Besides, Chrome porting also uses similar solution in its mime_sniffer.cc to check if mimetype contains slash '/' and commented it's following firefox. Apparently Chrome designed to match Firefox here. Saying it'll also act like Safari seems like a leap. Achieving the same end result as Safari does not mean the more thorough content-sniffing that's built in to Safari's networking library does it the same way. I'm still not convinced this patch is the right thing to do, but I won't r- it.
WebKit Review Bot
Comment 9 2012-11-28 11:02:03 PST
Comment on attachment 175596 [details] Patch Rejecting attachment 175596 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: ueue/Source/WebKit/chromium/third_party/leveldatabase/src --revision 68 --non-interactive --force --accept theirs-conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' 38>At revision 68. ________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' ________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' Updating webkit projects from gyp files... Full output: http://queues.webkit.org/results/15015552
Mary Wu
Comment 10 2012-12-02 18:40:40 PST
Mary Wu
Comment 11 2012-12-02 18:55:15 PST
Comment on attachment 177170 [details] Patch fix nit in change log
WebKit Review Bot
Comment 12 2012-12-02 19:31:19 PST
Comment on attachment 177170 [details] Patch Clearing flags on attachment: 177170 Committed r136361: <http://trac.webkit.org/changeset/136361>
WebKit Review Bot
Comment 13 2012-12-02 19:31:24 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.