[Webkit-unassigned] [Bug 60875] [SOUP] Abnormal operation if server sends 5xx status code without HTTP body

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=60875


Martin Robinson <mrobinson at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #93736|review?                     |review-
               Flag|                            |




--- Comment #5 from Martin Robinson <mrobinson at webkit.org>  2011-05-18 13:46:16 PST ---
(From update of attachment 93736)
View in context: https://bugs.webkit.org/attachment.cgi?id=93736&action=review

Do you have an example of a site this breaks? Ideally this change would come with a test case. Is the issue that when there is a 5xx error and no body, soup doesn't fail the load and it should? What do other network stacks do?

> Source/WebCore/ChangeLog:23
> +        Handle status code 5xx after receiving HTTP body
> +           - Background
> +            1) Webkit soup port premises that server always sends error page in case of status code 5xx.
> +            2) But, sometimes there is no HTTP body for error page. 
> +            3) In that case, Webkit tries to download the HTTP response even if there is no HTTP body. (abnormal operation)
> +           - Modified algorithm;
> +            1) libsoup sends status code 5xx (server error)
> +            2) Webkit soup port orders to accumulate HTTP body chunks to libsoup.
> +            3) Webkit soup port ignores gotHeadersCallback, contentSniffedCallback and gotChunkCallback.
> +            4) Webkit soup port checks there is HTTP body or not on sendRequestCallback and;
> +                4-1) if there is HTTP body, calls didReceiveResponse()
> +                4-2) if there is no HTTP body, calls didFail() - prevent the abnormal operation
> +           - With this modification;
> +            1) If server sends error page with HTTP body, Browser will show the received error page.
> +            2) If server sends no error page, that is, no HTTP body, Browser will show its own error page from each port.
> +

This seems okay, but typically ChangeLogs are in paragraph form. Take a look at trac.webkit.org for inspiration.

> Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp:411
> +            || ((message && SOUP_STATUS_IS_SERVER_ERROR(message->status_code)) && (!message->response_body || !message->response_body->length))

Do you mind splitting this off into a helper function called something like statusIsServerErrorAndHaveNotReceivedBody?

> Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp:422
> +        || ((message && SOUP_STATUS_IS_SERVER_ERROR(message->status_code)) && (!message->response_body || !message->response_body->length))) {

You can also use it here.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.