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

VERIFIED FIXED in mozilla1.8beta2

Status

()

Core
Networking
P4
major
VERIFIED FIXED
17 years ago
12 years ago

People

(Reporter: Roland, Assigned: Darin Fisher)

Tracking

Trunk
mozilla1.8beta2
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.8b3 -
blocking1.8b5 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments, 1 obsolete attachment)

(Reporter)

Description

17 years ago
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...

Comment 1

17 years ago
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.
(Reporter)

Comment 2

17 years ago
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.

Comment 3

17 years ago
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
(Assignee)

Comment 4

17 years ago
it's possible that that could be the problem... is the problem just a slow-to-
respond PAC server?

Comment 5

16 years ago
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

Comment 6

16 years ago
PAC
Assignee: neeti → serge

Comment 7

16 years ago
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.

Comment 8

16 years ago
to default owner
Assignee: serge → new-network-bugs

Comment 9

16 years ago
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

Comment 10

16 years ago
*** Bug 146545 has been marked as a duplicate of this bug. ***

Comment 11

15 years ago
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 → ---

Updated

15 years ago
Target Milestone: --- → Future

Comment 12

15 years ago
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
(Assignee)

Comment 13

15 years ago
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.

Comment 14

15 years ago
Happens on XP with Build 2002111211 too.

Comment 15

15 years ago
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!

Comment 16

15 years ago
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!

Comment 17

15 years ago
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 ?

Comment 18

15 years ago
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.

Comment 19

15 years ago
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

Comment 20

15 years ago
Created attachment 112868 [details]
Simple PAC file

Simple PAC file used for testing on our network

Comment 21

15 years ago
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.

Comment 22

15 years ago
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.
(Assignee)

Comment 23

15 years ago
-> suresh
Assignee: new-network-bugs → suresh

Comment 24

15 years ago
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.

Comment 25

15 years ago
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.

Comment 26

15 years ago
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.

Updated

15 years ago
Summary: PAC: connection to page refused on browser startup when Automatic Proxy configuration enabled → PAC: connection to page refused startup

Comment 27

15 years ago
*** Bug 152324 has been marked as a duplicate of this bug. ***

Updated

15 years ago
Summary: PAC: connection to page refused startup → PAC: connection to page refused at startup
(Assignee)

Comment 28

15 years ago
*** Bug 200468 has been marked as a duplicate of this bug. ***
*** Bug 202232 has been marked as a duplicate of this bug. ***

Comment 30

15 years ago
(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)

Comment 31

15 years ago
*** Bug 140038 has been marked as a duplicate of this bug. ***

Comment 32

15 years ago
*** Bug 208828 has been marked as a duplicate of this bug. ***

Comment 33

15 years ago
*** Bug 208901 has been marked as a duplicate of this bug. ***

Comment 34

15 years ago
I'm using Mozilla 1.4RC3 and I am seeing this bug also.  Nothing huge, but
definately an annoyance.
(Assignee)

Comment 35

15 years ago
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. ***
(Assignee)

Updated

15 years ago
Status: NEW → ASSIGNED
Target Milestone: mozilla1.5beta → mozilla1.6alpha
(Assignee)

Comment 37

15 years ago
*** Bug 216329 has been marked as a duplicate of this bug. ***
*** Bug 218390 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 39

14 years ago
this affects all platforms.
OS: Windows NT → All
Hardware: PC → All
(Assignee)

Updated

14 years ago
Target Milestone: mozilla1.6alpha → mozilla1.6beta
(Assignee)

Comment 40

14 years ago
-> moz 1.7b
Target Milestone: mozilla1.6beta → mozilla1.7beta
(Assignee)

Updated

14 years ago
Priority: P1 → P4

Comment 41

14 years ago
(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.


(Assignee)

Comment 42

14 years ago
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. ***
(Assignee)

Updated

14 years ago
Target Milestone: mozilla1.7beta → mozilla1.8alpha

Comment 44

14 years ago
*** Bug 163615 has been marked as a duplicate of this bug. ***
(Assignee)

Updated

14 years ago
Target Milestone: mozilla1.8alpha1 → Future
*** Bug 255006 has been marked as a duplicate of this bug. ***

Updated

14 years ago
Blocks: 239812

Comment 46

13 years ago
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
(Assignee)

Comment 48

13 years ago
*** Bug 260797 has been marked as a duplicate of this bug. ***

Comment 49

13 years ago
*** Bug 269385 has been marked as a duplicate of this bug. ***

Updated

13 years ago
Blocks: 208828

Comment 50

13 years ago
(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. ***

Comment 54

13 years ago
(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)

Comment 55

13 years ago
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.

Updated

13 years ago
Attachment #112868 - Attachment mime type: application/octet-stream → application/x-ns-proxy-autoconfig

Comment 56

13 years ago
*** Bug 221098 has been marked as a duplicate of this bug. ***
*** Bug 251622 has been marked as a duplicate of this bug. ***

Comment 58

13 years ago
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.
(Assignee)

Comment 59

13 years ago
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

Comment 60

13 years ago
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.
(Assignee)

Comment 61

13 years ago
> 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.

Comment 62

13 years ago
(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.

Comment 63

13 years ago
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.
(Assignee)

Comment 65

13 years ago
> 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. ***

Comment 67

13 years ago
*** Bug 295433 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 68

13 years ago
*** Bug 247057 has been marked as a duplicate of this bug. ***
(Assignee)

Updated

13 years ago
Flags: blocking1.8b3?
Flags: blocking-aviary1.1?
(Assignee)

Comment 69

13 years ago
*** Bug 296163 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 70

13 years ago
Created attachment 185913 [details] [diff] [review]
v1 patch

ta-da !
Attachment #185913 - Flags: review?(cbiesinger)
(Assignee)

Comment 71

13 years ago
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).
(Assignee)

Comment 72

13 years ago
*** Bug 297225 has been marked as a duplicate of this bug. ***

Comment 73

13 years ago
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.
> 
>   

(Assignee)

Comment 79

13 years ago
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.
(Assignee)

Comment 80

13 years ago
William, thanks for the feedback.  I need to make the HTTP code failover to
direct when the PAC file fails to be loaded.
(Assignee)

Comment 81

13 years ago
Created attachment 186124 [details] [diff] [review]
v1.1 patch

This version should handle failure to load the PAC URL properly.
Attachment #185913 - Attachment is obsolete: true
Attachment #186124 - Flags: review?(cbiesinger)
(Assignee)

Updated

13 years ago
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.

Comment 83

13 years ago
When will this appear in the nightly builds?
(Assignee)

Comment 84

13 years ago
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.

Comment 86

13 years ago
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.
(Assignee)

Comment 87

13 years ago
(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.

Updated

13 years ago
Flags: blocking1.8b4+
Flags: blocking1.8b3?
Flags: blocking1.8b3-

Comment 88

13 years ago
*** 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.

Comment 90

13 years ago
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+
(Assignee)

Comment 93

13 years ago
> 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.
(Assignee)

Comment 94

13 years ago
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?

Updated

13 years ago
Attachment #186124 - Flags: approval1.8b3?
Attachment #186124 - Flags: approval1.8b3+
Attachment #186124 - Flags: approval-aviary1.1a2?
Attachment #186124 - Flags: approval-aviary1.1a2+
(Assignee)

Comment 95

13 years ago
fixed-on-trunk
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED

Comment 96

13 years ago
Was the fix reverted?  This 20050627 build still has the bug.

Comment 97

13 years ago
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.
(Assignee)

Comment 98

13 years ago
I just tested the 20050627 linux build, and it appears to work great for me.

Comment 99

13 years ago
I have tested this on a Windows XP workstation and it works as it should.
(Tested build: 20050626)

Comment 100

13 years ago
(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 → ---
Created attachment 187617 [details]
http log of stopped communication
(Assignee)

Comment 103

13 years ago
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?
(Assignee)

Comment 105

13 years ago
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.
(Assignee)

Comment 106

13 years ago
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?
(Assignee)

Comment 108

13 years ago
Created attachment 187702 [details] [diff] [review]
v1 followup patch

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!
(Assignee)

Comment 110

13 years ago
yes, that is true.
Created attachment 187820 [details] [diff] [review]
alternative patch?

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+
(Assignee)

Comment 114

13 years ago
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 115

13 years ago
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?
(Assignee)

Comment 116

13 years ago
"v1 followup patch" is now checked in on the trunk.  marking FIXED
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago13 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

Comment 118

13 years ago
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.

Updated

12 years ago
Blocks: 185644

Comment 120

12 years ago
*** 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.