Bug 3547 – XMLHttpRequest.statusText returns always "OK"

• Status: RESOLVED
• Resolution: FIXED
• Component: XML

[Peter Speck]

When the webserver returns "500 Invalid dumpType arg = 'x'", XMLHttpRequest.statusText still returns 
"OK".

XMLHttpRequest.status returns the correct value.

tcpdump:
20:16:53.966539 IP localhost.commplex-main > localhost.55191: P 2:415(413) ack 464 win 65535 
<nop,nop,timestamp 1638750388 1638750388>
E...g.@.@...............M"..7U}v...........
a.X.a.X.TTP/1.0 500 Invalid dumpType arg = 'x'
Date: Wed, 15 Jun 2005 18:16:53 GMT
Server: DanCenterGraphicsServer/5.2.3
Connection: close
MIME-version: 1.0
X-ErrorMessage: Intern fejl i katalogserveren: Invalid dumpType arg = 'x'
Content-type: text/html

[Peter Speck]

For testing the bug, use
http://jjj.dancenter.com/pubweb/safaritest?t=1
and something like sudo tcpdump -s0 -A -i en0 host jjj.dancenter.com

[Anders Carlsson]

Looking at the NSHTTPURLResponse class, there's no way to get the status text in a response, it's only 
possible to get a general localized status text given a code, so this is a problem with CoreFoundation right 
now. 

Is it possible to add a getter for this to WebKitSystemInterface?

[Mark Rowe (bdash)]

<rdar://problem/5439321>

[Joran]

Firefox is already doing it right apparently. Shockingly still not fixed for WebKit! Come on guys!

[Adam Bergkvist]

Created attachment 28071 [details]
patch

Return status text if available.

[Alexey Proskuryakov]

What platforms does this fix the bug for? Unfortunately, on Mac OS X, httpStatusText() always returns "OK".

[Adam Bergkvist]

(In reply to comment #6)
GTK WebKit (soup)
https://bugs.webkit.org/show_bug.cgi?id=24226

[Peter Speck]

Shouldn't the patch remove the FIXME as this code no longer needs to be fixed  (but only CF on OSX)?

[Alexey Proskuryakov]

Comment on attachment 28071 [details]
patch

     // FIXME: <http://bugs.webkit.org/show_bug.cgi?id=3547> XMLHttpRequest.statusText returns always "OK".

Yes, this FIXME should be removed now, and new bugs should be filed for platforms that have this problem with ResourceResponse. Note that this bug is categorized as Macintosh/10.4, which means that the reporter needed it fixed on Mac - but I think that it's OK to make a new bug tracking the Mac fix.

     if (m_response.httpStatusCode())
-        return "OK";
+        if (!m_response.httpStatusText().isEmpty())
+            return m_response.httpStatusText();
+        else
+            return "OK";

At least Mac implementation always fills the response status text with OK. It would be better to ensure that all implementations do this, and to remove the isEmpty() check.

[Adam Bergkvist]

Created attachment 28171 [details]
patch

Updated patch with test.

[Adam Bergkvist]

Created attachment 28177 [details]
patch

Sorry, forgot to add the expected results file.

[Alexey Proskuryakov]

Comment on attachment 28177 [details]
patch

This is a good test, but it is not needed. There are at least two existing tests that should change from FAIL to PASS with this test (http/tests/xmlhttprequest/web-apps/012.html and 013.html). Instead of adding a new test, please add platform-specific results for these ones, and please do run all http tests. I believe that these tests should pass at least on gtk and windows (CF) now - it may make sense to change default results to expected ones, and to add platform-specific ones for platforms that don't pass yet.

It seems that QNetworkReplyHandler needs to set a fake httpStatusText, too.

[Adam Bergkvist]

Created attachment 28222 [details]
patch

You are right. This patch contains new expected results for web-apps/012 web-apps/013 for GTK.

[Alexey Proskuryakov]

> You are right. This patch contains new expected results for web-apps/012
> web-apps/013 for GTK.

Windows/CF also has this member initialized correctly, so it should have a passing result now, too.

What about my other comment (concerning QNetworkReplyHandler)?

[Adam Bergkvist]

Created attachment 28264 [details]
patch

I have prepared a patch to correctly set the status text on QT (See https://bugs.webkit.org/show_bug.cgi?id=24349).

This updated patch includes platform specific expected results for GTK, QT, and Win. Perhaps the default expected result should be PASS instead and platform specific expected results added for platforms that set a fake status text?

[Alexey Proskuryakov]

(In reply to comment #15)
> Perhaps the default expected result should be PASS instead and platform
> specific expected results added for platforms that set a fake status text?

Yes, that makes sense.

[Adam Bergkvist]

Created attachment 28306 [details]
patch

Patch changes default expected results for tests 012 and 013 (http/tests/xmlhttprequest/web-apps/) to PASS and adds platform specific expected results for mac.

Modified XMLHttpRequest::statusText to return the status text if available regardless of the state. (Same behavior as XMLHttpRequest::status.)

[Alexey Proskuryakov]

(In reply to comment #17)
> Modified XMLHttpRequest::statusText to return the status text if available
> regardless of the state. (Same behavior as XMLHttpRequest::status.)

This will change the behavior on Mac - "OK" will be always returned, even in cases that raised an exception before. Checking by status code should be more straightforward, in my opinion.

[Adam Bergkvist]

(In reply to comment #18)
> This will change the behavior on Mac - "OK" will be always returned, even in
> cases that raised an exception before. Checking by status code should be more
> straightforward, in my opinion.

In my opinion it is more correct to check the variable that is actually returned than a related value. If mac sets the fake status text where the real value should be set, I can not see how that would change the behavior.

[Alexey Proskuryakov]

Comment on attachment 28306 [details]
patch

Yes, seems fine indeed. r=me

[Alexey Proskuryakov]

Committed <http://trac.webkit.org/changeset/41665>.

Filed bug 24572 to track a Mac fix.

[Alexey Proskuryakov]

This actually did change other results a little, see <http://trac.webkit.org/changeset/41666>. But the change was a progression.