Closed Bug 100022 Opened 23 years ago Closed 19 years ago

PAC: first page/homepage load fails (b/c automatic proxy configuration is slower than first HTTP request)

Categories

(Core :: Networking, defect, P4)

Product:
Core
Component:
Networking
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla1.8beta2

People

(Reporter: luolong, Assigned: darin.moz)

References

Details

Attachments

(5 files, 1 obsolete file)

Simple PAC file
123 bytes, application/x-ns-proxy-autoconfig
Details
v1.1 patch
22.11 KB, patch
Biesinger
: review+
asa
: approval-aviary1.1a2+
asa
: approval1.8b3+
Details | Diff | Splinter Review
http log of stopped communication
17.84 KB, text/plain
Details
v1 followup patch
12.33 KB, patch
Biesinger
: review+
benjamin
: approval1.8b3+
Details | Diff | Splinter Review
alternative patch?
14.85 KB, patch
Details | Diff | Splinter Review 
From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; WinNT4.0; en-US; rv:0.9.4+) Gecko/20010912
BuildID:    2001091203



Reproducible: Always
Steps to Reproduce:
1. Configure the browser to use Autometic proxy configuration script, open
Homepage on startup and set homepage to be somewhere outside Your firewall/proxy.
2. Exit Mozilla completely.
3. Run Mozilla.

Actual Results:  I get message, saying the URL was not found. When I reload or
try again (without closing Mozilla) - it works.

Expected Results:  The homepage should have been loaded the first time.

I am sitting behind a quite restrictive firewall and thus rely heavily on the
proxy server configuratin. This kind of behaviour has forced me previously to go
back to fixed proxy configurations, assuming automatic configuration does not
work. And several of my collegues have dumped Mozilla for the very same reason...
My guess as to what's going on is that you're hitting a race condition -- the
PAC file is loaded asynchronously on startup.  If this takes a little while,
it's possible that the http request to fetch your homepage goes into PAC
resolution before the PAC has been fully retrieved/digested/etc.  In this case,
the nsProxyAutoConfig will default to a direct connection -- so the home page
doesn't load.

I'll try to confirm that this is the case when I get a chance.

See also bug 97605, which is not entirely related.
Yes - that was my guess allso even before I started to report this bug. Looking
at the (somewhat) related PAC: bugs confirmed my suspicions.

This bug has existed for quite some time - I wander that I was the first to
report it though.
another vaguely related bug is bug 83894 -- where a similar problem prevents an
easy fix.

I'm going to go ahead and mark this NEW, even though I can't reproduce it -- the
race in the code is pretty clear.

I think the solution darin proposed to bug 97605 might take care of this as well
(judging by the phrase "PAC download would then become an app modal
operation"), but I'm not sure.  darin?
Status: UNCONFIRMED → NEW
Ever confirmed: true
QA Contact: benc → pacqa
it's possible that that could be the problem... is the problem just a slow-to-
respond PAC server?
I've done a quick packet capture on my system (I'm getting this exact same
problem), and mozilla is doing the DNS lookup for the homepage in the middle of
receiving the .PAC file from the server.  e.g.

(Timestamps in seconds)

3.271317 Mozilla initiates TCP connection to server holding PAC file
3.272612 TCP handshake completes
3.274658 Mozilla issues DNS lookup request to internal DNS server for external
home page
3.276537 Mozilla receives PAC file from server
3.276878 Mozilla re-issues DNS lookup request to internal server

PAC
Assignee: neeti → serge
correction to Additional Comments From Chase Tingley 2001-09-17 09:39
>another vaguely related bug is bug 83894 
bug 83984 has some discussion about properly pac loading implementation,
unfortunately, there is found no easy solution fro this problem yet.
to default owner
Assignee: serge → new-network-bugs
Temporarily "futuring" all PAC&SOCKS bugs to clear new-networking queue.

I will review later. (I promise)

If you object, and can make a case for a mozilla 1.0 fix, please reset milestone
to  "--" or email me.
Target Milestone: --- → Future
*** Bug 146545 has been marked as a duplicate of this bug. ***
comment #5 seems to be the key piece of data to these scattered, but credible
complaints about PAC loading problems that are fixed w/ reload.
Keywords: helpwanted
Target Milestone: Future → ---
Target Milestone: --- → Future
This appears to have got *much* worse in the beta of 1.2 (build 20020826) It now
takes several attempts of clicking "home" before Mozilla finally manages to get
to my home page. This is on Win2K
if you are seeing this bug, you might actually need to reinstall mozilla from
scratch (after completely deleting the installation directory), or at least try
deleting components/compreg.dat.  please comment if this doesn't help.
Happens on XP with Build 2002111211 too.
I have the same problem with build 20030125 on W2K.

This, bug #180811, and bug #188006 all appear to be dupes of bug #83984.  Since
this one has the most votes and the best description, I'll add my vote here in
the hopes that it will do some good.  This bug will force my corporation to go
to IE if it's not fixed soon!
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.3b) Gecko/20030125

Did some network sniffing today and discovered the following behaviour:

- ISA-Server has some rules to allow proxying only after NTLM or Basic
  authentication
- Mozilla tries to get the default automatic configuration URL
  (http://<myisaserver>:8080/array.dll?Get.Routing.Script)
- Mozilla gets a 401 (authorization required) but never asks the user
  for basic credentials

Then I copied the automatic configuration script to an anonymously reachable 
webserver into PAC.TXT and entered the URL into Mozilla.

With this setup it works just fine!
I guess that since the request for the PAC file is being made outside of a
browser window, there's no handler to deal with 401 or any other other errors
that may prevent the PAC from being loaded.

I'm just trying to think if there are any other instances where moz makes an
http request that doesn't result in the response being displayed in a browser
window.. (any suggestions?)

When an error occurs in loading the html page for any other window, the process
is modal, and blocks until it gets a 200, or presents a authorisation box prompt
etc etc..

In your situation (needing authorisation to get the PAC file), what would you
suggest would be a reasonable thing for mozilla to do ? I don't know what IE
does - and you probably do, but my guess is that you would end up with either a
blank initial browser window that gets blocked by the PAC loading, which in turn
is blocked by an authorisation dialog, or .... you would see just the
authorisation diaglog.

The crux is that the PAC loader needs to block the page loader.

It shouldn't be THAT hard to get that code to work, as page loading already
blocks in several situations - just modify that code to check for a boolean that
says whether or not the PAC config is stable or not.

On initial startup, and whenever someone hits the "reload" in the proxy config
dialog in preferences, that value would be false.

It should be set true whenever the PAC has successfully loaded, or when loading
has been aborted, and should stay false during PAC load.

Also - comment 13 implies that this bug has already been addresses - can someone
confirm this either way ?
I just had a thought... what about view document source ?

At the moment, there's a bug open that complains that view source often reloads
the URI, rather than showing the source of the document in the appropriate
window...   while that bug still exists, it should be possible to use it to find
out if the authorisation required diaglog can work in places other than a normal
browser window.

I'll see if I can do it, although I don't think the 'view source reloads' is
always reproducable.. but I'll give it a shot anyway.


damn.. can't seem to reproduce that other bug.. even if I post the http query to
the same URL as the current page.. which is what I think the problem was

There must be some other place in mozilla other than the browser and PAC that
makes an http request.. that can be used to check if the authorisation handler
works anywhere else.. but my mind has gone blank
Attached file Simple PAC file — 
Simple PAC file used for testing on our network
Re: comment #13 - this doesn't help

Re: comment #16 - neither our (Netscape/iPlanet) web server that holds the
proxy.pac nor our (Netscape) proxy servers require end user authentication.  

Just for the fun of it, I took our PAC file, simplified it ridiculously (see
attachment), and copied it to another (IIS) web server.  The problem persisted.

However, if the file sits on my local hard drive, my home page does load when
Mozilla first starts.
I don't know if this information is of any use but I have been using Mozilla 
since version 1.0 with the auto proxy script enabled with absolutely no 
problem.  In fact Mozzie often sorts out the proxies faster than IE.
-> suresh
Assignee: new-network-bugs → suresh
Still using 20030128 build on W2K.

I start on a system with three Netscape 4.79 profiles.  After installing
Mozilla, I import one of those profiles (the third in the list).  The home page
is imported from the profile, while the PAC file location is read from all.js. 
When I start Mozilla the second time (so the Mozilla/Start page isn't loaded),
the problem occurs. It occurs on all restarts thereafter.

However, if I create another profile (not imported from NS4 - dunno if it
matters), then all of a sudden, I can start Mozilla using either profile, and
the home page will load.

This seems repeatable - I've killed all my Mozilla profiles and repeated the
sequence.
addendum to #24:

If I delete the 'other' profile, the problem returns on the first profile.  If I
create another new profile, it goes away.

Seems like I've found a workaround, but damned if I know why it works.
Sorry for the spam-effect.  Hopefully, this'll be the last note for now.

If I start the Profile Manager, and start my profile from there, the problem
doesn't occur, whether or not I've got two Mozilla profiles.  

I assume that starting via the Profile Manager gives the PAC file time to load,
clearing the race condition.
Summary: PAC: connection to page refused on browser startup when Automatic Proxy configuration enabled → PAC: connection to page refused startup
*** Bug 152324 has been marked as a duplicate of this bug. ***
Summary: PAC: connection to page refused startup → PAC: connection to page refused at startup
*** Bug 200468 has been marked as a duplicate of this bug. ***
*** Bug 202232 has been marked as a duplicate of this bug. ***
(I think this is the original bug, but I thought there were more dupes)

Thinking about this... if PAC's default value is "DIRECT", and so a slow PAC
load is allowing the first necko request to access the default PAC, then why not
have PAC manually set the PAC file to some blocking value/mode, until the PAC
request is returned?
Keywords: 4xp
Summary: PAC: connection to page refused at startup → PAC: first page load fails (b/c PAC is slower than first HTTP request)
*** Bug 140038 has been marked as a duplicate of this bug. ***
*** Bug 208828 has been marked as a duplicate of this bug. ***
*** Bug 208901 has been marked as a duplicate of this bug. ***
I'm using Mozilla 1.4RC3 and I am seeing this bug also.  Nothing huge, but
definately an annoyance.
I think we can solve this bug using nsIChannel::Open to load the PAC file.  I
will try to make this happen for 1.5 final.
Assignee: suresh → darin
Depends on: 192284
Priority: -- → P1
Target Milestone: Future → mozilla1.5beta
*** Bug 215502 has been marked as a duplicate of this bug. ***
Status: NEW → ASSIGNED
Target Milestone: mozilla1.5beta → mozilla1.6alpha
*** Bug 216329 has been marked as a duplicate of this bug. ***
*** Bug 218390 has been marked as a duplicate of this bug. ***
this affects all platforms.
OS: Windows NT → All
Hardware: PC → All
Target Milestone: mozilla1.6alpha → mozilla1.6beta
-> moz 1.7b
Target Milestone: mozilla1.6beta → mozilla1.7beta
Priority: P1 → P4
(In reply to comment #15)

> This, bug #180811, and bug #188006 all appear to be dupes of bug #83984.

Yeah, IMO were should be a dependency between bug 83984 and this bug.



hmm.. i think they are different bugs.  bug 83984 is about lazily initializing
PAC.  this bug is about completely initializing PAC in time to satisfy the first
request for something that should be inspected by the PAC module.  i don't see a
dependency there.  we could fix that one without affecting this bug, or we could
fix this one without affecting that bug.
*** Bug 234637 has been marked as a duplicate of this bug. ***
Target Milestone: mozilla1.7beta → mozilla1.8alpha
*** Bug 163615 has been marked as a duplicate of this bug. ***
Target Milestone: mozilla1.8alpha1 → Future
*** Bug 255006 has been marked as a duplicate of this bug. ***
Blocks: 239812
All comments for bug 239812 apply to this bug. We looked at it a bit 
differently but it's the same bug.  Should 239812 be marked a duplicate instead 
of a dependency?
Bug 260797 is the same - only with firefox
*** Bug 260797 has been marked as a duplicate of this bug. ***
*** Bug 269385 has been marked as a duplicate of this bug. ***
Blocks: 208828
(sigh). So there have been 6 duplicates (bug 208828, bug 208901, bug 216329, bug
255006, Bug 260797, Bug 269385, Bug 239812) from firefox.

In the old day's I'd reopen them to dupe to bug 208828, but that just creates
too much bugmail, especially for darin.

FF contributors, please look at that bug description, and try to make it as
searchable as possible, to prevent another string of dupes into this bug.
No longer blocks: 239812
QA Contact: pacqa → benc
*** Bug 273945 has been marked as a duplicate of this bug. ***
Summary: PAC: first page load fails (b/c PAC is slower than first HTTP request) → PAC: first page/homepage load fails (b/c PAC is slower than first HTTP request)
*** Bug 272609 has been marked as a duplicate of this bug. ***
*** Bug 280216 has been marked as a duplicate of this bug. ***
(In reply to comment #53)
> *** Bug 280216 has been marked as a duplicate of this bug. ***

I just wanted to mention that I'm the person who submitted the latest duplicate
report of this bug. I couldn't find this entry because I was searching for
"proxy" and not "PAC". Someone should put the word "proxy" in the summary if
more duplicate submissions are to be avoided.

I'm also shocked to learn that this issue has been around for almost 3 and a
half years.
Summary: PAC: first page/homepage load fails (b/c PAC is slower than first HTTP request) → PAC: first page/homepage load fails (b/c automatic proxy configuration is slower than first HTTP request)
Ben: the problem is not easy to solve.

Darin: I am wondering what is the acceptable delay, and whether or not it could
be tuned.
Attachment #112868 - Attachment mime type: application/octet-stream → application/x-ns-proxy-autoconfig
*** Bug 221098 has been marked as a duplicate of this bug. ***
*** Bug 251622 has been marked as a duplicate of this bug. ***
Rather than calculating the acceptable delay, perhaps the browser should simply
wait to process the first HTTP request (except of course whatever requests are
in the script to set up the proxy settings) until the script has finished
executing.  In most cases you wouldn't want the script to be running in parallel
with the first user HTTP request anyway.

I haven't reviewed the code, so I'm not sure how easy that is.
My plan is to delay the load of the first HTTP request until the PAC file has
been downloaded.  This will maybe happen in time for Gecko 1.8 (Firefox 1.1).
Target Milestone: Future → mozilla1.8beta2
We're getting into symantics here, but don't you mean until the PAC has been
*executed*?  If the PAC has been downloaded and then the first HTTP request is
made, the environment still hasn't been set up properly for that request to go
through.
> We're getting into symantics here, but don't you mean until the PAC has been
> *executed*?  If the PAC has been downloaded and then the first HTTP request is
> made, the environment still hasn't been set up properly for that request to go
> through.

Yes, that's what I meant.  But, in fact the PAC file is executed synchronously
from the OnStopRequest event corresponding to the completion of loading the PAC
file into memory, so it makes no difference how you look at it.
(In reply to comment #59)
> My plan is to delay the load of the first HTTP request until the PAC file has
> been downloaded.  This will maybe happen in time for Gecko 1.8 (Firefox 1.1).
This problem happens for me 100% of the time if the PAC URL is a file:/// URI.

For some reason, I see this happening more often on recent builds on Win2K and
has always happened on OSX builds for me. Maybe we're just getting faster?
(In reply to comment #63)
> For some reason, I see this happening more often on recent builds on Win2K and
> has always happened on OSX builds for me. Maybe we're just getting faster?

This problam was severly aggravated by the checkin for bug 282442.
> This problam was severly aggravated by the checkin for bug 282442.

Indeed, but the patch in that bug is a pre-req for fixing this bug.  I do intend
to fix this for Firefox 1.1, but it help would definitely be appreciated.
*** Bug 295316 has been marked as a duplicate of this bug. ***
*** Bug 295433 has been marked as a duplicate of this bug. ***
*** Bug 247057 has been marked as a duplicate of this bug. ***
Flags: blocking1.8b3?
Flags: blocking-aviary1.1?
*** Bug 296163 has been marked as a duplicate of this bug. ***
Attached patch v1 patch (obsolete) — — Splinter Review 
ta-da !
Attachment #185913 - Flags: review?(cbiesinger)
NOTE: This patch introduces a new proxy type called "unknown" that if loaded
ordinarily will be treated as "direct" by the socket transport layer.  The HTTP
protocol recognizes this special proxy type as a signal to call AsyncResolve on
the nsIProtocolProxyService.  A couple tweaks to nsPACMan were required to pull
this off, but the result seems to work well.  Because of the socket transport
fu, I'm hopeful that this will not regress thunderbird.  Hopefully, thunderbird
will similarly leverage the same approach for its network connections (though I
suspect that PAC is less common for mailnews).
*** Bug 297225 has been marked as a duplicate of this bug. ***
Strangly enough, I dont see this bug with Thunderbird, even though with
thunderbird I have the proxy enabled as well.
Must be something to do with enough delay for the proxy.pac to be loaded
Comment on attachment 185913 [details] [diff] [review]
v1 patch

+     * NOTE: If PAC is configured, and the PAC file has not yet been loaded,
+     * then this method will return a nsIProxyInfo instance with a type of
+     * "unknown"

could the RESOLVE_NON_BLOCKING flag be used instead? i.e., if that flag is set
and the PAC file isn't loaded, return failure. I guess that doesn't answer what
to do if that flag is not set...
or maybe both should be done: if the flag is set, return WOULD_BLOCK; otherwise,
return a proxy with type unknown. that avoids special casing that proxy type
when the caller can handle WOULD_BLOCK.
I built firefox including this patch and did some testing.  I am currently on
vacation and connected via an iffy wireless modem connection so it is kind of a
worst case.  The patch appears to fix the probem as described.  Unfortuately it
has regressed the ability of the browser to fall back to not using a proxy in
the PAC case, if the proxy is not available.

At work I need to use a proxy with a complicated PAC configuration to access the
Internet.  I generally just leave Firefox with that configuration always and
count on the browser falling back to accessing everything DIRECT if the PAC
server is unavailable.

Additionally the errors which occur when attemtping to access the PAC server are
incorrectly reported as errors contacting the webserver.  I have my start page
configured to be a page on www.mozilla.org.  IF I am not at work the name of our
proxy PAC server cannot be resolved in DNS.  This results in a pup-up saying
"www.mozilla.org could not be found.  Please check the name and try again." 
Changing to using an IP address for the PAC server instead of the hostname
results in the message "The operation timed out when attempting to contact
www.mozilla.org."

The good news is that the proposed code seems to work in both the direct case
and the PAC case as long as you have the browser configured appropriately fro
your connection.  That is more than can be said for the current production code.

Also good, is that the new issue can be easily recreated by anyone with a direct
Internet connection and does not appear to be timing sensitive or require any
complex configuration to reproduce.

Steps to reproduce:

1.  In Firefox, go to Tools -> Options -> General, set the Home page to
https://www.mozilla.org.support/firefox/

2.  Click on COnnection Settings

3.  Select Automatic proxy configuration URL: and enter the URL
http://proxy.ray.com/proxy.pac
    (I selected this name on purpose because this is a domain I administer DNS
for and so I know this name does not and will never exist and returns a no such
host type error without the DNS server timing out.  This all makes the test case
simpler)

4.  Exit Firfox

5. Launch Firefox.

You will get an error saying the hostname www.mozilla.org does not exist.

  
(In reply to comment #76)
> worst case.  The patch appears to fix the probem as described.  Unfortuately it
> has regressed the ability of the browser to fall back to not using a proxy in
> the PAC case, if the proxy is not available.

I meant to say if the PAC server is not available.  Sorry for extra BUGSPAM.
(In reply to comment #76)
> Steps to reproduce:
> 
> 1.  In Firefox, go to Tools -> Options -> General, set the Home page to
> https://www.mozilla.org.support/firefox/

Should be http://www.mozilla.org/support/firefox/

This was meant to be a valid URL that returns actual content.  I should have
proof-read this better :-(

> 
> 2.  Click on COnnection Settings
> 
> 3.  Select Automatic proxy configuration URL: and enter the URL
> http://proxy.ray.com/proxy.pac
>     (I selected this name on purpose because this is a domain I administer DNS
> for and so I know this name does not and will never exist and returns a no such
> host type error without the DNS server timing out.  This all makes the test case
> simpler)
> 
> 4.  Exit Firfox
> 
> 5. Launch Firefox.
> 
> You will get an error saying the hostname www.mozilla.org does not exist.
> 
>   


biesi:

The point is that I need a way to convey the fact that proxy info is still
unknown to the HTTP protocol handler so that it can call AsyncResolve.  If I
used the NON_BLOCKING flag, then I would need a solution for protocols other
than http:// and https://, which probably involves creating a nsIChannel
implementation that wraps the real nsIChannel that has yet to be determined. 
That isn't something I want to try to solve just yet as it is a much more
involved patch with greater implications (e.g., how do I implement
QueryInterface on the wrapping nsIChannel implementation?).

An alternative to this approach that I considered was to return a nsIProxyInfo
of type "direct" with a new value OR'd into the flags attribute indicating that
the proxy info was yet to be resolved.  That may be a better approach, but there
is some code that converts a "direct" nsIProxyInfo into a NULL proxy info, so
I'd have to be careful to fixup such code.
William, thanks for the feedback.  I need to make the HTTP code failover to
direct when the PAC file fails to be loaded.
Attached patch v1.1 patch — — Splinter Review 
This version should handle failure to load the PAC URL properly.
Attachment #185913 - Attachment is obsolete: true
Attachment #186124 - Flags: review?(cbiesinger)
Attachment #185913 - Flags: review?(cbiesinger)
The version(In reply to comment #81)
> Created an attachment (id=186124) [edit]
> v1.1 patch
> 
> This version should handle failure to load the PAC URL properly.

As far as I can tell from my testing so far, the version 1.1 patch appears to
fix the original issue, as well as the problem I mentioned in comment #76.
When will this appear in the nightly builds?
As with any Mozilla patch, it will land when there are reviews and approvals. 
I'd expect this to take a couple days to a week depending on how busy people
are.  It should make it for Firefox 1.1 alpha 2.
(In reply to comment #71)
> this off, but the result seems to work well.  Because of the socket transport
> fu, I'm hopeful that this will not regress thunderbird.  Hopefully, thunderbird
> will similarly leverage the same approach for its network connections (though I
> suspect that PAC is less common for mailnews).

I have built Thunderbird including the v1.1 patch and it appears to still be
able to load images via the proxy specified in the PAC file.  I have noticed no
regressions so far.
I'm using Deer Park at work.  My homepage is set to something local.  If I
immediately click an external link, it fails.  Hopefully this will fix it.
(In reply to comment #86)
> I'm using Deer Park at work.  My homepage is set to something local.  If I
> immediately click an external link, it fails.  Hopefully this will fix it.

Yes, it should fix that problem.
Flags: blocking1.8b4+
Flags: blocking1.8b3?
Flags: blocking1.8b3-
*** Bug 298449 has been marked as a duplicate of this bug. ***
I'll get to this review tomorrow. Sorry for the delay, I didn't have much time
in the last week.
I'm using TB 1.1a 20050622  and I get a problem with the autoload of proxy info.

The pac file may have non-ascii chars in it? The header looks like this. I 
can't post any more because it has server names

/* DO NOT EDIT! DO NOT EDIT! DO NOT EDIT! DO NOT EDIT!
1,1,-1,-1
1,.aaa.com
 
 
 
 * Proxy autoconfig file for Netscape Navigator
 * Generated by Netscape-Proxy/2.5
 * Generated on Mon Dec 30 18:28:20 1996
 
 *
 * DO NOT EDIT! DO NOT EDIT! DO NOT EDIT! DO NOT EDIT!
 *Any changes you make could be overwritten by the admin interface
 */
function FindProxyForURL(url, host)
{
        // Direct connections to non-aaa hosts
        if (isPlainHostName(host)) {
            return "DIRECT";
        }
 
        // Direct connections to local domain
  


This caused the endless throbber in TB. The JavaScript console reports:

Error: redeclaration of const kMailToLength 
Source File: chrome://communicator/content/nsContextMenu.js 
Line: 13 

Error: redeclaration of const imgICache 
Source File: chrome://communicator/content/contentAreaUtils.js 
Line: 232 

Error: [Exception... "Component returned failure code: 0x80040111 
(NS_ERROR_NOT_AVAILABLE) [nsIXMLHttpRequest.status]" nsresult: "0x80040111 
(NS_ERROR_NOT_AVAILABLE)" location: "JS frame :: chrome://messenger-
newsblog/content/Feed.js :: anonymous :: line 174" data: no] 
Source File: chrome://messenger-newsblog/content/Feed.js 
Line: 174 (Twice) 

thanks

bugzilla@willmooney.com: This bug is ONLY for the first loading of the PAC file
and that the first request (homepage) could be slower and fails. This bug is NOT
the bug you are searching if your PAC file doesn't load at all. (that should be
very clear if you read comment #0)
There is another bug for non-ascii chars in the pac-file
Comment on attachment 186124 [details] [diff] [review]
v1.1 patch

base/src/nsPACMan.cpp
+  if (mStartingToLoad)
+    return NS_OK;

shouldn't this method also truncate result?

protocol/http/src/nsHttpChannel.cpp	
+	 else if (PL_strcmp(mConnectionInfo->ProxyType(), "unknown") == 0)

why not libc's strcmp?

+	     return ResolveProxy();  // Lazily resolve proxy info

I wonder if we can avoid that if we already have a valid cache entry...

+nsHttpChannel::ReplaceWithProxy(nsIProxyInfo *pi)
+{
+    nsresult rv;

I'd move the decl to the line where it is first used
Attachment #186124 - Flags: review?(cbiesinger) → review+
> base/src/nsPACMan.cpp
> +  if (mStartingToLoad)
> +    return NS_OK;
> 
> shouldn't this method also truncate result?

good catch


> protocol/http/src/nsHttpChannel.cpp	
> +	 else if (PL_strcmp(mConnectionInfo->ProxyType(), "unknown") == 0)
> 
> why not libc's strcmp?

because mConnectionInfo->ProxyType() can be null.


> +	     return ResolveProxy();  // Lazily resolve proxy info
> 
> I wonder if we can avoid that if we already have a valid cache entry...

Yeah, probably... but we only know if we have a cache entry by actually trying
to open a cache entry.  In other words, we'd have to add yet another case to the
Connect logic to resolve proxy info if we have to setup a transaction.  We could
do that, but I don't think it's worth the effort.  The number of requests that
process "unknown" nsIProxyInfo should be relatively small.


> +nsHttpChannel::ReplaceWithProxy(nsIProxyInfo *pi)
> +{
> +    nsresult rv;
> 
> I'd move the decl to the line where it is first used

It practically is ;-)


Thanks for the code review.
Comment on attachment 186124 [details] [diff] [review]
v1.1 patch

This patch fixes a top usability problem in Deer Park Alpha 1 for people using
PAC to access their homepage.  I think it would be great to get this patch in
for DAP2.
Attachment #186124 - Flags: approval1.8b3?
Attachment #186124 - Flags: approval-aviary1.1a2?
Attachment #186124 - Flags: approval1.8b3?
Attachment #186124 - Flags: approval1.8b3+
Attachment #186124 - Flags: approval-aviary1.1a2?
Attachment #186124 - Flags: approval-aviary1.1a2+
fixed-on-trunk
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Was the fix reverted?  This 20050627 build still has the bug.
Oops, never mind, it really is fixed!  Apparently installing a Deer Park nightly
without uninstalling a Firefox release confuses GMail Notifier, and it still
launches the old Firefox.

Sweet!  Thanks for the fix, this one was really bugging me.
I just tested the 20050627 linux build, and it appears to work great for me.
I have tested this on a Windows XP workstation and it works as it should.
(Tested build: 20050626)
(In reply to comment #88)
> *** Bug 298449 has been marked as a duplicate of this bug. ***
I guess my bug is also fixed. Teste with Mozilla/5.0 (Windows; U; Windows NT
5.0; en-US; rv:1.8b2) Gecko/20050627.

This bug is not fixed! With the checkin of patch v1.1 Firefox can't connect to
any website anymore. The first time when it appears is between 06-23 and 06-24
which is exactly the timeframe of this checkin. I don't know if this happens
while using the rewrite function of apache. Do we only check for the pac file
once and don't follow any redirect? I'll attach a HTTP log so you can see the
behavior.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Yup, redirects are not handled properly.  Good catch.  Basically, the problem is
that we delay loading the redirect while waiting for the PAC file to load! :-(

Fixing this probably requires teaching nsPACMan.cpp how to observe redirects,
and then have it remember the URI of the current channel being used to load the
PAC file so that it can return DIRECT for that URI when queried.
Status: REOPENED → ASSIGNED
Maybe what we need is a special load flag that always bypasses proxies?
A load flag wouldn't do it because the check happens in the nsIIOService::
newChannelFromURI call.  We only have the nsIURI at that point.  That is why I
proposed the solution in comment #103.
Well, actually... we could tag the channel (via load flag or some property) so
that it knows to ignore the nsIProxyInfo given to it.  That is another option. 
I'm not sure if I prefer that though.
hm... maybe nsIIOService should somehow expose newProxiedChannel, and the HTTP
channel could, if said load flag is present and it sets up its replacement
channel, call that, passing it a null proxyinfo?
This implements the simplest solution I could come up with.  When loading a PAC
URL that results in a redirect, we'll end up doing the following:

* LoadPACFromURI sets mPACURI to remember the PAC URL
* GetProxyForURI returns DIRECT for initial URL load b/c given URL == mPACURI
* redirect is processed by HTTP resulting in NewChannel being called
* GetProxyForURI returns NS_ERROR_IN_PROGRESS for PAC redirect URL
* nsHttpChannel calls OnChannelRedirect
* nsPACMan updates mPACURI
* nsHttpChannel calls nsIProtocolProxyService::AsyncResolve
* nsPACMan processes the AsyncGetProxyForURI call immediately instead of
  queuing as it normally would do in this case
* nsPACMan makes an asynchronous callback reporting DIRECT for the new PAC URI
  based on a call to GetProxyForURI (since mPACURI was updated, this works)

I would like to support a bypass proxies load flag at some point, but that
seems like a more involved change.
Attachment #187702 - Flags: review?(cbiesinger)
(In reply to comment #108)

If I understand what you are doing here correctly, it seems as if this approach
should even cover the degenerate case of the seocnd request resulting in a
second redirect!  I like it!
yes, that is true.
I liked this patch until I realized I had to do the HTTP channel change... now,
I'm not so sure that this is so good, especially as it makes HTTP violate the
docs I added to LOAD_BYPASS_PROXY... But maybe it would be enough to change
that documentation.
(and this patch does not include the change to the other protocols)

I'll review the other patch, but darin, if you prefer my patch, feel free to
review :)
Comment on attachment 187702 [details] [diff] [review]
v1 followup patch

ah... this patch is actually simpler than I imagined. r=biesi
Attachment #187702 - Flags: review?(cbiesinger) → review+
Comment on attachment 187702 [details] [diff] [review]
v1 followup patch

I like the idea of NewChannelWithFlags, but let's go with this one to fix the
regression.  nsIIOService2 might be 1.9 fodder.
Attachment #187702 - Flags: approval1.8b3?
Attachment #187702 - Flags: approval-aviary1.1a2?
Comment on attachment 187702 [details] [diff] [review]
v1 followup patch

a=bsmedberg for checkin on 6/30 only
Attachment #187702 - Flags: approval1.8b3?
Attachment #187702 - Flags: approval1.8b3+
Attachment #187702 - Flags: approval-aviary1.1a2?
"v1 followup patch" is now checked in on the trunk.  marking FIXED
Status: ASSIGNED → RESOLVED
Closed: 19 years ago → 19 years ago
Resolution: --- → FIXED
With the additional patch Firefox is working fine now. Thanks Darin.

Verified with Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b2)
Gecko/20050703 Firefox/1.0+
Status: RESOLVED → VERIFIED
Flags: blocking-aviary1.1?
Keywords: helpwanted
Proxy autoconfiguration still not works in Firefox 1.0.5 (Mozilla/5.0 (Windows;
U; Windows NT
5.0; en-US; rv:1.7.9) Gecko/20050711 Firefox/1.0.5)
(In reply to comment #118)
> Proxy autoconfiguration still not works in Firefox 1.0.5 (Mozilla/5.0 (Windows;
> U; Windows NT
> 5.0; en-US; rv:1.7.9) Gecko/20050711 Firefox/1.0.5)

This fix was never checked-in into the aviary branch. It's not a security fix so
the first release where it is available is DeerPark Alpha 2.
Blocks: 185644
*** Bug 185644 has been marked as a duplicate of this bug. ***
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: