Long delay between receiving a 421 response and parsing the body / returning the body to JS

RESOLVED FIXED in Firefox 40

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: mgunther, Assigned: mcmanus)

Tracking

(4 keywords)

38 Branch
mozilla42
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox38 wontfix, firefox38.0.5 wontfix, firefox39 wontfix, firefox40 fixed, firefox41 fixed, firefox42 fixed, firefox-esr31 unaffected, firefox-esr38- affected)

Details

Attachments

(1 attachment, 1 obsolete attachment)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/43.0.2357.130 Safari/537.36

Steps to reproduce:

Execute the following code in FF's console:

(function() {
  var url = 'https://api.apple-cloudkit.com/database/1/iCloud.com.example.CloudKitCatalog/development/public/users/current?ckAPIToken=1c6d0ac40cf685931a021359dd217680182692e958d5c95248e36ee44696cd5d';

  var xhr = new XMLHttpRequest(); 
  xhr.open('GET', url, true); 
  xhr.onload = function() { console.log(xhr.status, xhr.responseText); }; 
  xhr.send();
})()


Actual results:

Note that you immediately see a warning: "not well-formed"

In the network tab, observe the XHR request:
-- The response headers are received and displayed promptly. 
-- The 'Timings' tab seems to indicate that the response was fully received.
-- However the columns 'Type', 'Transferred', 'Size' stay empty.

About 2 minutes later, these columns get populated and the xhr's onload handler gets triggered (console prints the status code and response text).


Expected results:

xhr.onload should have been called as soon as the response body was available.
Component: Untriaged → Networking
Product: Firefox → Core
[Tracking Requested - why for this release]: Regressed since Firefox36. 


Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=c7c0cb53949d&tochange=2c56d741c3fd

Suspect: 
a2609830541a	Patrick McManus — Bug 1082261 - http 421 retry r=hurley
Blocks: 1082261
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(mcmanus)
Keywords: regression
mgunther - thanks for the test case. There is definitely a firefox problem there, which I'll fix.

But also the apple API is also using 421 in a way inconsistent with the IANA registration. That's now defined to be "not authoritative" which is not what is meant by the service. So apple probably wants to change that to a different 4xx code.

But the bigger bug is on the firefox side. Thanks again.
Flags: needinfo?(mcmanus)
Posted patch some 421 retries dont work (obsolete) — Splinter Review
Attachment #8629192 - Flags: review?(hurley)
Assignee: nobody → mcmanus
Status: NEW → ASSIGNED
Attachment #8629383 - Flags: review?(hurley)
Attachment #8629192 - Attachment is obsolete: true
Attachment #8629192 - Flags: review?(hurley)
the change in comment 5 makes the error code a soft stream error, which will mean a h2 session with a 421 will keep the session alive for other transactions.
Comment on attachment 8629383 [details] [diff] [review]
some 421 retries dont work

Review of attachment 8629383 [details] [diff] [review]:
-----------------------------------------------------------------

::: netwerk/protocol/http/nsHttpTransaction.cpp
@@ +773,5 @@
>      mWriter = nullptr;
>  
> +    if (mForceRestart) {
> +        // The forceRestart condition was dealt with on the stack, but it did not
> +        // clear the flag because nsPipe in the readsegment stack clears out

nit: this is the writesegment stack here, not readsegment :)
Attachment #8629383 - Flags: review?(hurley) → review+
https://hg.mozilla.org/mozilla-central/rev/d9ea769d96b8
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
Comment on attachment 8629383 [details] [diff] [review]
some 421 retries dont work

Approval Request Comment
[Feature/regressing bug #]:1082261 (FF 38)
[User impact if declined]: sites using 421 HTTP response code might suffer long delays depending on exact response. This apparently includes api.apple-cloudkit.com. (prior to recent times 421 was undefined, but of course that does not mean it wasn't in use anywhere).
[Describe test coverage new/current, TreeHerder]: old general regression coverage and new case for specific issue here.
[Risks and why]: low risk as it only impacts uses of this (previously undefined code), but as h2 defined the code we can expect to see more of it and should handle it correctly
[String/UUID change made/needed]:none
Attachment #8629383 - Flags: approval-mozilla-beta?
Attachment #8629383 - Flags: approval-mozilla-aurora?
Comment on attachment 8629383 [details] [diff] [review]
some 421 retries dont work

Has tests, in m-c for 4 days, early in the beta cycle, taking it.
Attachment #8629383 - Flags: approval-mozilla-beta?
Attachment #8629383 - Flags: approval-mozilla-beta+
Attachment #8629383 - Flags: approval-mozilla-aurora?
Attachment #8629383 - Flags: approval-mozilla-aurora+
QA Whiteboard: [good first verify]
Untracking for ESR38 as this does not meet the esr triage bar which primarily involves security related bug fixes and issues that have a significant end-user impact.
You need to log in before you can comment on or make changes to this bug.