[Webkit-unassigned] [Bug 24804] [GTK] 401 responses cause rogue content to be loaded

[bugzilla-daemon at webkit.org]

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





------- Comment #3 from zecke at selfish.org  2009-03-25 21:30 PDT -------
(From update of attachment 28937)
> a5cf3ffc4ba04bd9ca5be3cf646f112aee3fe2db
> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> index 03b7af7..a27bbf0 100644
> --- a/WebCore/ChangeLog
> +++ b/WebCore/ChangeLog
> @@ -1,3 +1,17 @@
> +2009-03-25  Gustavo Noronha Silva  <gustavo.noronha at collabora.co.uk>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +	https://bugs.webkit.org/show_bug.cgi?id=24804
> +	[GTK] 401 responses cause rogue content to be loaded
> +
> +        Our soup code handles 401 responses itself, so we should not feed
> +        the headers and data of those responses to the loader.


tabs vs. spaces here? anyway the pre-commit hook would catch these :)

> +    // For 401, we will accumulate the resource body, and only use it
> +    // in case authentication with the soup feature doesn't happen
> +    if (msg->status_code == SOUP_STATUS_UNAUTHORIZED) {
> +        soup_message_body_set_accumulate(msg->response_body, TRUE);
> +        return;
> +    }
> +
> +    // For all the other responses, we handle each chunk ourselves,
> +    // and we don't need msg->response_body to contain all of the data
> +    // we got, when we finish downloading.
> +    soup_message_body_set_accumulate(msg->response_body, FALSE);
> +
>      // The 304 status code (SOUP_STATUS_NOT_MODIFIED) needs to be fed
>      // into WebCore, as opposed to other kinds of redirections, which
>      // are handled by soup directly, so we special-case it here and in
>      // gotChunk.
>      if (SOUP_STATUS_IS_TRANSPORT_ERROR(msg->status_code)
> -        || (SOUP_STATUS_IS_REDIRECTION(msg->status_code) && (msg->status_code != SOUP_STATUS_NOT_MODIFIED)))
> +        || (SOUP_STATUS_IS_REDIRECTION(msg->status_code) && (msg->status_code != SOUP_STATUS_NOT_MODIFIED))
> +        || (msg->status_code == SOUP_STATUS_UNAUTHORIZED))
>          return;

okay, with status->code == SOUP_STATUS_UNAU... we will never reach here...
unless soup_message_body_set_accumulate changes the status_code. So can this be
removed?


>  
>      // We still don't know anything about Content-Type, so we will try
> @@ -268,7 +281,8 @@ static void gotHeadersCallback(SoupMessage* msg, gpointer data)
>  static void gotChunkCallback(SoupMessage* msg, SoupBuffer* chunk, gpointer data)
>  {
>      if (SOUP_STATUS_IS_TRANSPORT_ERROR(msg->status_code)
> -        || (SOUP_STATUS_IS_REDIRECTION(msg->status_code) && (msg->status_code != SOUP_STATUS_NOT_MODIFIED)))
> +        || (SOUP_STATUS_IS_REDIRECTION(msg->status_code) && (msg->status_code != SOUP_STATUS_NOT_MODIFIED))
> +        || (msg->status_code == SOUP_STATUS_UNAUTHORIZED))
>          return;

what about an ASSERT(msg->status_code != SOUP_STATUS_UNAUTHORIZED)? Or will
gotChunkCallback be called regardles of the body accumulate?


otherwise, it looks pretty good.


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