309391 - XMLHttpRequest fails to track proxy server failover

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect)

Description

[Darin Fisher]

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.

Comment 1

[Darin Fisher]

Attachment: v1 patch

Simple patch.  I decided not to send this event to nsIHttpEventSink since that
interface only exists for backwards compatibility.

Comment 2

[Christian :Biesinger (don't email me, ping me on IRC)]

*** Bug 309390 has been marked as a duplicate of this bug. ***

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

I suppose writing a unit test for this case is not so trivial... but it would be
nice to have one.

Comment 4

[Darin Fisher]

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.

Comment 5

[Darin Fisher]

fixed-on-trunk

Comment 6

[Scott MacGregor]

Darin, should we let this bake on the trunk for a couple days before deciding to
take it? 

Comment 7

[Asa Dotzler [:asa]]

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?

Comment 8

[Darin Fisher]

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.

Comment 9

[Darin Fisher]

fixed1.8