Closed Bug 309391 Opened 15 years ago Closed 15 years ago
Request fails to track proxy server failover
XMLHttpRequest fails to track proxy server failover. As a result, when the XMLHttpRequest is asked for its status, statusText, etc., it will query those properties on the wrong Necko channel. It observes OnChannelRedirect to update the value of its mChannel member, but because that event is not generated when we failover to a different proxy server, XMLHttpRequest does not learn about the new channel. One way to fix this would be to make XMLHttpRequest update mChannel in its OnStartRequest implementation, but it would be cleaner if it could get a OnChannelRedirect event. We have the flag REDIRECT_INTERNAL for a reason, and so we should make use of it. Patch coming up. This is a critical bug because it makes XMLHttpRequest unreliable on networks where PAC is used. The problem is made worse by the fact that we defer loading of HTTP channels until the PAC script is loaded, and we issue internal redirects on those deferred channels once the PAC script is loaded.
Simple patch. I decided not to send this event to nsIHttpEventSink since that interface only exists for backwards compatibility.
Attachment #196843 - Flags: review?(cbiesinger)
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.8beta5
*** Bug 309390 has been marked as a duplicate of this bug. ***
15 years ago
Attachment #196843 - Flags: review?(cbiesinger) → review+
I suppose writing a unit test for this case is not so trivial... but it would be nice to have one.
Comment on attachment 196843 [details] [diff] [review] v1 patch This patch is a low-risk fix for a bug that can basically render XMLHttpRequest-based apps useless for PAC users. I think we should fix this for Firefox 1.5b2.
Attachment #196843 - Flags: approval1.8b5?
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Darin, should we let this bake on the trunk for a couple days before deciding to take it?
Darin, what's the potential fallout? is this only going to impact PAC users? What kind of testing coverage do we need to feel confident about this?
I think this is a low risk patch. This bug only impacts PAC users. I don't think we gain much by baking this on the trunk.
Attachment #196843 - Flags: approval1.8b5? → approval1.8b5+
You need to log in before you can comment on or make changes to this bug.