Closed Bug 309391 Opened 17 years ago Closed 17 years ago

XMLHttpRequest fails to track proxy server failover


(Core :: Networking: HTTP, defect)

Not set





(Reporter: darin.moz, Assigned: darin.moz)



(Keywords: fixed1.8)


(1 file)

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.
Attached patch v1 patchSplinter Review
Simple patch.  I decided not to send this event to nsIHttpEventSink since that
interface only exists for backwards compatibility.
Attachment #196843 - Flags: review?(cbiesinger)
Flags: blocking1.8b5?
Target Milestone: --- → mozilla1.8beta5
*** Bug 309390 has been marked as a duplicate of this bug. ***
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?
Closed: 17 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+
Keywords: fixed1.8
Flags: blocking1.8b5? → blocking1.8b5+
You need to log in before you can comment on or make changes to this bug.