Last Comment Bug 309391 - XMLHttpRequest fails to track proxy server failover
: XMLHttpRequest fails to track proxy server failover
Status: RESOLVED FIXED
: fixed1.8
Product: Core
Classification: Components
Component: Networking: HTTP (show other bugs)
: Trunk
: All All
: -- critical (vote)
: mozilla1.8beta5
Assigned To: Darin Fisher
:
: Patrick McManus [:mcmanus]
Mentors:
: 309390 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2005-09-20 15:16 PDT by Darin Fisher
Modified: 2005-09-23 11:54 PDT (History)
1 user (show)
asa: blocking1.8b5+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v1 patch (1.28 KB, patch)
2005-09-20 15:32 PDT, Darin Fisher
cbiesinger: review+
asa: approval1.8b5+
Details | Diff | Splinter Review

Description Darin Fisher 2005-09-20 15:16:27 PDT
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 2005-09-20 15:32:24 PDT
Created attachment 196843 [details] [diff] [review]
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) 2005-09-20 16:01:56 PDT
*** Bug 309390 has been marked as a duplicate of this bug. ***
Comment 3 Christian :Biesinger (don't email me, ping me on IRC) 2005-09-20 16:07:04 PDT
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 2005-09-20 17:05:13 PDT
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 2005-09-20 17:19:25 PDT
fixed-on-trunk
Comment 6 Scott MacGregor 2005-09-21 10:34:34 PDT
Darin, should we let this bake on the trunk for a couple days before deciding to
take it? 
Comment 7 Asa Dotzler [:asa] 2005-09-21 11:25:57 PDT
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 2005-09-21 21:23:59 PDT
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 2005-09-22 11:45:21 PDT
fixed1.8

Note You need to log in before you can comment on or make changes to this bug.