Bug 24600 - [GTK] responses with status code >= 400 should not be given special treatment
Summary: [GTK] responses with status code >= 400 should not be given special treatment
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk, Soup
Depends on:
Blocks:
 
Reported: 2009-03-14 15:04 PDT by Gustavo Noronha (kov)
Modified: 2009-03-25 07:50 PDT (History)
0 users

See Also:


Attachments
proposed fix (2.94 KB, patch)
2009-03-14 15:07 PDT, Gustavo Noronha (kov)
no flags Details | Formatted Diff | Diff
proposed fix (3.78 KB, patch)
2009-03-17 12:33 PDT, Gustavo Noronha (kov)
zecke: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gustavo Noronha (kov) 2009-03-14 15:04:52 PDT
Current code avoids calling gotHeaders and gotChunk for messages that are not "successful". This makes them skip things such as content sniffing, and would hog the memory for responses with big bodies, were accumulate not disabled =). This last fact is the reason why error pages with content, such as plone.org's 404 pages are not displayed.
Comment 1 Gustavo Noronha (kov) 2009-03-14 15:07:26 PDT
Created attachment 28630 [details]
proposed fix
Comment 2 Holger Freyther 2009-03-17 10:26:17 PDT
Okay, any idea about the impact on the http tests?
Comment 3 Gustavo Noronha (kov) 2009-03-17 12:31:26 PDT
(In reply to comment #2)
> Okay, any idea about the impact on the http tests?
> 

I was getting some tests fixed, and one regression. I have reworked the patch to fix the xmlhttprequest regression, and we are now down to 2 failing tests, and no crash, in my tests. I'm going to upload the patch now.
Comment 4 Gustavo Noronha (kov) 2009-03-17 12:33:42 PDT
Created attachment 28695 [details]
proposed fix

This patch differs from the other in that it also does not give special treatment to 304 regarding gotHeaders and gotChunk, but does avoid trying a content sniff on it. On my local setup, I get the following impact on the http tests:

Without the patch:

Fails:

http/tests/xmlhttprequest/access-control-basic-whitelist-request-headers.html
http/tests/xmlhttprequest/xmlhttprequest-missing-file-exception.html
http/tests/xmlhttprequest/xmlhttprequest-no-content-length-onProgress.html
http/tests/xmlhttprequest/web-apps/013.html

Crashes:

http/tests/xmlhttprequest/web-apps/002-simple.html

With the patch:

Fails:

http/tests/xmlhttprequest/access-control-basic-whitelist-request-headers.html
http/tests/xmlhttprequest/xmlhttprequest-missing-file-exception.html
Comment 5 Gustavo Noronha (kov) 2009-03-17 14:17:46 PDT
Comment on attachment 28630 [details]
proposed fix

Forgot to obsolete when uploading the new one.
Comment 6 Holger Freyther 2009-03-25 00:17:24 PDT
Comment on attachment 28695 [details]
proposed fix


> +    if ((msg->status_code != SOUP_STATUS_NOT_MODIFIED)
> +        &&!soup_message_headers_get_content_type(msg->response_headers, NULL))

style. put a space in there...
Comment 7 Gustavo Noronha (kov) 2009-03-25 07:50:12 PDT
Landed as r41974 with the style fix.