Bug 24750 – [GTK] requests download instead of displaying page

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Gustavo Noronha (kov)]

This was initially reported in Epiphany's bug tracker: http://bugzilla.gnome.org/show_bug.cgi?id=576267.

When the server sends to or more different Content-Type headers, webkit will request that the page be downloaded instead of displaying it:

wget -S -O /dev/null 'http://lhv.delfi.ee/news/index.cfm?id=1146314'
--2009-03-22 11:16:44--  http://lhv.delfi.ee/news/index.cfm?id=1146314
Resolving lhv.delfi.ee... 90.190.151.34
Connecting to lhv.delfi.ee|90.190.151.34|:80... connected.
HTTP request sent, awaiting response... 
  HTTP/1.1 200 OK
  Connection: close
  Date: Sun, 22 Mar 2009 14:16:51 GMT
  Server: Microsoft-IIS/6.0
  Content-type: text/html
  Page-Completion-Status: Normal
  Expires: -1
  Content-Type: text/html; charset=windows-1257
  Page-Completion-Status: Normal
  Set-Cookie: CFID=45627684; path=/; 
  Set-Cookie: CFTOKEN=52268372; path=/; 
  Set-Cookie: LOG_CLIENT_ID=35258851; expires=Sun, 27-Sep-2037 00:00:00 GMT;
path=/;

This is caused by the fact that libsoup will concatenate all instances of the header, and WebCore doesn't like that.

[Gustavo Noronha (kov)]

Created attachment 28840 [details]
proposed fix

[Holger Freyther]

Comment on attachment 28840 [details]
proposed fix

> -    String contentType = soup_message_headers_get(msg->response_headers, "Content-Type");
> +    GHashTable *contentTypeParameters = 0;

style, '*' pleaced wrongly.



> +    if (!contentType.isEmpty() && contentType.find(",", 0)) {
> +        Vector<String> contentTypes;
> +        contentType.split(',', contentTypes);
> +        contentType = contentTypes[0];
> +    }

Interesting question. What is faster... searching the string twice or creating the Vector?


> +
> +    if (contentTypeParameters) {
> +        GHashTableIter hashTableIter;
> +        gpointer hashKey;
> +        gpointer hashValue;
> +
> +        g_hash_table_iter_init(&hashTableIter, contentTypeParameters);
> +        while (g_hash_table_iter_next(&hashTableIter, &hashKey, &hashValue)) {
> +            contentType += "; ";
> +            contentType += static_cast<char*>(hashKey);
> +            contentType += "=";
> +            contentType += static_cast<char*>(hashValue);
> +        }

You could consider using StringBuilder here. And another thing. With HTTP headers are like ASCII which makes your use of String(char*) okay, but maybe write String() around it as well to highlight that we really want this.

[Gustavo Noronha (kov)]

Landed as r41975.

(In reply to comment #2)
> > +    if (!contentType.isEmpty() && contentType.find(",", 0)) {
> > +        Vector<String> contentTypes;
> > +        contentType.split(',', contentTypes);
> > +        contentType = contentTypes[0];
> > +    }
> 
> Interesting question. What is faster... searching the string twice or creating
> the Vector?

I've gone with creating the Vector. Since split already works correctly, and doesn't do much more than checking if a , exists in the string, I believe this is the way to go.
 
> > +        while (g_hash_table_iter_next(&hashTableIter, &hashKey, &hashValue)) {
> > +            contentType += "; ";
> > +            contentType += static_cast<char*>(hashKey);
> > +            contentType += "=";
> > +            contentType += static_cast<char*>(hashValue);
> > +        }
> 
> You could consider using StringBuilder here. And another thing. With HTTP
> headers are like ASCII which makes your use of String(char*) okay, but maybe
> write String() around it as well to highlight that we really want this.
 
I have added the String()'s, but since I am not sure of the advantages of using StringBuilder here, when compared to simply using the + operator, I have left that out for the time being.

Thanks!

[Gustavo Noronha (kov)]

Created attachment 28944 [details]
use soup facilities to append parameters to the content type

 WebCore/ChangeLog                                  |   10 ++++++++++
 .../platform/network/soup/ResourceHandleSoup.cpp   |   10 ++++++----
 2 files changed, 16 insertions(+), 4 deletions(-)

[Gustavo Noronha (kov)]

Comment on attachment 28944 [details]
use soup facilities to append parameters to the content type

danw noticed that my work-around is not robust enough for parameters with spaces in their values; this patch uses the libsoup facilities, which handle these cases correctly, to append the parameters

[Gustavo Noronha (kov)]

Reopening bug because I posted a new, related patch.

[Holger Freyther]

Comment on attachment 28944 [details]
use soup facilities to append parameters to the content type

Okay, didn't know this method, nice cleanup.

[Gustavo Noronha (kov)]

Landed as r42106.