Bug 24930

Summary: [Gtk] ISO files content is displayed inside the webview instead of being downloaded
Product: WebKit Reporter: Cosimo Cecchi <cosimoc>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: gns, jmalonzo
Priority: P2 Keywords: Gtk, Soup
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
URL: http://debian.fastweb.it/debian-cd/3.1_r2/i386/iso-cd/
Attachments:
Description Flags
Fix crash when load is cancelled after content sniffing.
ap: review+
Also sniff content of types declared as text/plain. ap: review+

Description Cosimo Cecchi 2009-03-30 05:39:19 PDT
- Go to http://debian.fastweb.it/debian-cd/3.1_r2/i386/iso-cd/
- Try to open one of the ISO files
- The content is displayed inside the web view, it should popup a window offering a download

Tested with Epiphany trunk + WebKit r42097, works fine with FF 3.0.8 instead.
Comment 1 Gustavo Noronha (kov) 2009-03-30 05:52:20 PDT
Here's the reason:

wget -S -O /dev/null http://debian.fastweb.it/debian-cd/3.1_r2/i386/iso-cd/debi…
…an-31r2-i386-binary-1.iso
--2009-03-30 09:30:52--  http://debian.fastweb.it/debian-cd/3.1_r2/i386/iso-cd/debian-31r2-i386-binary-1.iso
Resolving debian.fastweb.it... 213.156.32.111
Connecting to debian.fastweb.it|213.156.32.111|:80... connected.
HTTP request sent, awaiting response... 
  HTTP/1.1 200 OK
  Date: Mon, 30 Mar 2009 12:31:06 GMT
  Server: Apache/1.3.33 (Debian GNU/Linux)
  Last-Modified: Thu, 20 Apr 2006 04:22:25 GMT
  ETag: "325417a1-27c37000-44470c81"
  Accept-Ranges: bytes
  Content-Length: 667119616
  Keep-Alive: timeout=15, max=100
  Connection: Keep-Alive
  Content-Type: text/plain; charset=iso-8859-1
Length: 667119616 (636M) [text/plain]
Saving to: `/dev/null'

Which means we will have to start thinking about doing content sniffing when we have text/plain, or other types, as well. Notice that you can setup apache to send text/plain as a default content type =/.
Comment 2 Benjamin Otte 2009-03-30 06:30:35 PDT
Just had a discussion about how much data is required for content sniffing with g_content_type_guess(). xdgmime lists this in http://standards.freedesktop.org/shared-mime-info-spec/shared-mime-info-spec-latest.html#id2554163 in the MAX_EXTENT property. On my current Ubuntu this value is 2141. So with 4k downloaded you will get a 100% perfect content sniffing.
Comment 3 Gustavo Noronha (kov) 2009-04-01 11:30:21 PDT
Created attachment 29167 [details]
Fix crash when load is cancelled after content sniffing.

received also in gotChunkCallback, or we crash in didReceiveData when
the load is cancelled in didReceiveResponse.
---
 WebCore/ChangeLog                                  |   11 +++++++++++
 .../platform/network/soup/ResourceHandleSoup.cpp   |   10 +++++++---
 2 files changed, 18 insertions(+), 3 deletions(-)
Comment 4 Gustavo Noronha (kov) 2009-04-01 11:32:03 PDT
Comment on attachment 29167 [details]
Fix crash when load is cancelled after content sniffing.

This fixes a crash I found while investigating a fix for this problem.
Comment 5 Gustavo Noronha (kov) 2009-04-01 11:34:47 PDT
Created attachment 29169 [details]
Also sniff content of types declared as text/plain.

 WebCore/ChangeLog                                  |   14 ++++++++++++++
 .../platform/network/soup/ResourceHandleSoup.cpp   |    3 ++-
 2 files changed, 16 insertions(+), 1 deletions(-)
Comment 6 Gustavo Noronha (kov) 2009-04-01 11:36:41 PDT
Comment on attachment 29169 [details]
Also sniff content of types declared as text/plain.

The only fix for this case is not trusting what the server sends as Content Type for the file, and sniffing. I think also sniffing when we get text/plain is correct because many servers are configured to send text/plain as a default content type.
Comment 7 Alexey Proskuryakov 2009-04-02 08:24:56 PDT
Comment on attachment 29167 [details]
Fix crash when load is cancelled after content sniffing.

> +        Protect the handle when notifying the client that the response was
> +	received also in gotChunkCallback, or we crash in didReceiveData
> +	when the load is cancelled in didReceiveResponse.

Tabs instead of spaces here.

r=me
Comment 8 Alexey Proskuryakov 2009-04-02 08:29:43 PDT
Comment on attachment 29169 [details]
Also sniff content of types declared as text/plain.

r=me. Please note that we should eventually move to a cross-platform implementation of content sniffing, to ensure that all platforms work the same way.
Comment 9 Gustavo Noronha (kov) 2009-04-02 09:01:51 PDT
Landed as r42170 and r42171.