[Webkit-unassigned] [Bug 86344] [BlackBerry] xhr request to non existent file response is 0 and not 404.

[bugzilla-daemon at webkit.org]

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





--- Comment #7 from Joe Mason <jmason at rim.com>  2012-05-14 08:35:17 PST ---
(In reply to comment #6)
> If I understand you right:
> 
> For a non-existent file, curl is returning 404 for GET  but 0 for HEAD.  I don't see anything in the spec that says it should be doing this, so it sounds like a curl bug.

Nevermind, I'm not understanding right.

The problem is in handleNotifyClose:

sendResponseIfNeeded();
if (isClientAvailable()) {
    RecursionGuard guard(m_callingClient);

    if (isError(m_extendedStatusCode) && !m_dataReceived && m_handle->firstRequest().httpMethod() != "HEAD") {
        String domain = m_extendedStatusCode < 0 ? ResourceError::platformErrorDomain : ResourceError::httpErrorDomain;
        ResourceError error(domain, m_extendedStatusCode, m_response.url().string(), m_response.httpStatusText());
        m_handle->client()->didFail(m_handle.get(), error);
    } else
        m_handle->client()->didFinishLoading(m_handle.get(), 0);
}

I'm not sure when that "HEAD" check got added, but it needs to match the check in sendResponseIfNeeded, since that gets called immediately beforehand.

The intent is that on error we either call "didFail" with the error message, if an error code was returned but no data, or "didReceiveResponse" followed by "didFinishLoading", if an error code was returned but there was data to display (such as a custom 404 page).  didReceiveResponse is sent by sendResponseIfNeeded.

So on a HEAD request, which never has data, if a 404 is received we return from sendResponseIfNeeded without calling didReceiveResponse, but then call didFinishLoading.

I guess the HEAD check was added so that didFail is never called on HEAD - it should call didReceiveResponse followed by didFinishLoading even without any data.

And we don't want to get rid of the didFail branch entirely, because for top-level requests that's where we format our custom error page.

So the code change looks good to me.  But I think "isError(m_extendedStatusCode) && !m_dataReceived && m_handle->firstRequest().httpMethod() != "HEAD"" should be put into a helper function so that if it's changed again it only needs to be changed in one place.

Also, are we sure that an XHR "GET" for to a server that returns just 404 with no data doesn't have the same problem?  Maybe we should also change the "HEAD" check to a check for TargetIsMainFrame || TargetIsSubframe (since we only ever need to call didFail and create an error page for errors that will actually be displayed in a frame), or a check for TargetIsXHR (if didFail has side effects that we should be triggering everywhere expect in an XHR).

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