824341 - firefox 17, linux, use system proxy settings does not check environment variables when GConf is installed

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[promeneur]

User Agent: Mozilla/5.0 (X11; Linux i686; rv:17.0) Gecko/20100101 Firefox/17.0
Build ID: 20121128204232

Steps to reproduce:

Mandriva 2010.2 i586
kde 4.8.4

firefox 17.0 and 17.0.1
privoxy 3.0.19

privoxy is used to suppresses the advertisements

i set firefox to use system proxy settings
then
i go to www.lemonde.fr
then
i see advertisdement is not suppressed
then
i set firefox to use manualy proxy settings
then
i go to www.lemonde.fr
then
i see advertisements are suppressed

no pb with chromium or previous firefox 16.01

[root@localhost ~]# env | grep proxy
http_proxy=http://localhost:8118
no_proxy=localhost,127.0.0.1
[root@localhost ~]# 



Actual results:

firefox does not use the proxy when using "use system proxy settings"


Expected results:

firefox does use the proxy when using "use system proxy settings"

Comment 1

[Matthias Versen [:Matti]]

Please read bug 821655 (especially comment#7) and confirm that you have the same issue.

Comment 2

[promeneur]

no i don't use fedora or ubuntu but mandriva. i don't use gnome but kde

i use drakproxy to set system proxy settings

i didn't migrate to new mandriva

each time i open a kde session this script /etc/profile.d/proxy.sh is executed

http_proxy=http://localhost:8118
no_proxy=localhost,127.0.0.1
export http_proxy https_proxy ftp_proxy no_proxy

have you any proxy server to make a test , perhaps it's a privoxy pb.


mandriva does not use any /etc/environment file

chromium 21.0 has no pb

Comment 3

[promeneur]

i reinstall 16.01
then
no pb with "use sytem proxy settings"

Comment 4

[promeneur]

perhaps is similar to https://bugzilla.mozilla.org/show_bug.cgi?id=804574

Comment 5

[Patrick McManus [:mcmanus]]

definitely a dup of 821655

Comment 6

[promeneur]

ok but it's a bit tricky for me.

can anyone explain to me the solution ? a firefox fix or what else ?

Comment 7

[promeneur]

(In reply to Patrick McManus [:mcmanus] from comment #5)
> definitely a dup of 821655
> 
> *** This bug has been marked as a duplicate of bug 821655 ***

are you sure ?

i have also a pb with konqueror and rekonq using "use system proxy settings" since the paremeter values "http_proxy" and "no_proxy" will be replaced by an update (what one ?) by


HTTP_PROXY,http_proxy,HTTPPROXY,httpproxy,PROXY,proxy
and
NO_PROXY,no_proxy

but for konqueror and rekonq i changed the list by the old lists "http_proxy" and "no_proxy"

happily chromium is compliant with the new values
HTTP_PROXY,http_proxy,HTTPPROXY,httpproxy,PROXY,proxy
and
NO_PROXY,no_proxy
and chromium works well

conclusion :

i think firefox must be compliant to following values :
HTTP_PROXY,http_proxy,HTTPPROXY,httpproxy,PROXY,proxy
and
NO_PROXY,no_proxy

Comment 8

[Patrick McManus [:mcmanus]]

(In reply to promeneur from comment #7)
> (In reply to Patrick McManus [:mcmanus] from comment #5)
> > definitely a dup of 821655
> > 
> > *** This bug has been marked as a duplicate of bug 821655 ***
> 
> are you sure ?
> 

I'm pretty sure that the authors of 713802 broke the various env var handlings which is what you are seeing here. Sharing your experiences in that bug would be helpful. I haven't decided whether or not to ask them to revert/fix it yet.

Comment 9

[Karl Tomlinson (:karlt)]

It might be helpful to confirm that this behaviour changed for the expected reasons:

Is gsettings-desktop-schemas installed?
It is possible that this package has different names in different distributions, but look for a file like
/usr/share/glib-2.0/schemas/org.gnome.system.proxy.gschema.xml

If for firefox 16.01, you run "ldd /path/to/firefox/components/libmozgnome.so | grep not", do you see any entries other than libxpcom.so and libmozalloc.so?

Do you have the libgconf-2.so.4 library installed?

Comment 10

[promeneur]

(In reply to Karl Tomlinson (:karlt) from comment #9)

> Is gsettings-desktop-schemas installed?
> It is possible that this package has different names in different
> distributions, but look for a file like
> /usr/share/glib-2.0/schemas/org.gnome.system.proxy.gschema.xml
> 
there is no /usr/share/glib-2.0/schemas/org.gnome.system.proxy.gschema.xml

in schemas there are :
gschema.dtd
gschemas.compiled
org.freedesktop.gstreamer-0.10.default-elements.gschema.xml


> If for firefox 16.01, you run "ldd
> /path/to/firefox/components/libmozgnome.so | grep not", do you see any
> entries other than libxpcom.so and libmozalloc.so?
> 
[root@localhost components]# ldd libmozgnome.so | grep not
        libxpcom.so => not found
        libmozalloc.so => not found
        libnotify.so.1 => /usr/lib/libnotify.so.1 (0xb7503000)
[root@localhost components]#

in components there are :
binary.manifest
libbrowsercomps.so
libdbusservice.so
libmozgnome.so
libnkgnomevfs.so

> Do you have the libgconf-2.so.4 library installed?
yes

it's a pointer to libgconf-2.so.4.1.5

Comment 11

[Karl Tomlinson (:karlt)]

Thanks.  That leaves me unable to explain how Firefox 16.01 was respecting environment variables for you.

If mGConf is non-NULL, then GetProxyFromEnvironment() was not used here:
https://hg.mozilla.org/releases/mozilla-release/annotate/0507d387617c/toolkit/system/unixproxy/nsUnixSystemProxySettings.cpp#l502

mGConf would have been non-NULL iff nsGConfService::Init() returned NS_OK.
https://hg.mozilla.org/releases/mozilla-release/annotate/0507d387617c/toolkit/system/gnome/nsGConfService.cpp#l71
That would happen if libmozgnome.so loads fine (no missing symbols), some fundamental symbols are found in libgconf-2.so.4, and gconf_client_get_default() returns non-NULL.

gconf_client_get_default should never return NULL.

When running Firefox 16.01, does "lsof -p <firefox-process-id> | grep mozgnome" find libmozgnome.so?

Comment 12

[Karl Tomlinson (:karlt)]

This is not a duplicate of bug 821655.  That bug is a change between 17 and 18.  This bug is a change between 16 and 17.

Comment 13

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Karl, bug 821655 is also a change between 16 and 17 but only for the system Firefox installation, not for our native binaries. I'm sure that the reporter can't see this behavior with one of our 17.0.x versions.

That's something I have noticed but not reported on bug 821655. Sorry for that.

Comment 14

[promeneur]

>> When running Firefox 16.01, does "lsof -p <firefox-process-id> | grep mozgnome" find
>> libmozgnome.so?

yes

Comment 15

[Wolfgang Rosenauer [:wolfiR]]

promeneur, so you are using Firefox as delivered by Mandriva or a mozilla.org tarball? I don't think that is answered already above?

Comment 16

[promeneur]

16.0.1 is delivered by mandriva (a rpm)
17.0 and 17.0.1 i tested are the tar.gz delivered by mozilla

Comment 17

[Wolfgang Rosenauer [:wolfiR]]

So a pretty good guess is that Mandriva has a patch to not use GConf/GSettings if not running under Gnome. So Karl's explanation from comment 11 describes that original Firefox does not check environment variables at all if Gnome components are available on the system (which are for you (comment 14)). But your Mandriva Firefox probably did.

@karlt, I'm carrying a patch for that behaviour in my set since ages:
http://www.rosenauer.org/hg/mozilla/file/9124c1a643c5/mozilla-nongnome-proxies.patch
I always didn't like the $DESKTOP_SESSION evaluation as it seems a bit flaky but then again it worked for several years (more than 5 or so) w/o issues.
Can we consider to incorporate that (or something similar) into Firefox? Gnome is not the only desktop to support actually.
Then I'd file a bug about it.

Comment 18

[Karl Tomlinson (:karlt)]

Yes, we should do something to ensure environment variables are checked, and we can use this bug.

I'd prefer GNOME_DESKTOP_SESSION_ID over DESKTOP_SESSION because DESKTOP_SESSION seems to be used by the display manager, there may be inconsistencies in the names of GNOME sessions, and there can be other ways to start a GNOME session.

Other apps are using GNOME_DESKTOP_SESSION_ID and it was restored, even if deprecated, for this reason.  I'm not keen on asking DBus to look for org.gnome.SessionManager.
https://bugzilla.gnome.org/show_bug.cgi?id=542880

GNOME_DESKTOP_SESSION_ID should be absolutely fine for skipping GConf.

For GSettings, I don't know what to think.

  Perhaps GNOME_DESKTOP_SESSION_ID will be removed one day but I hope not
  because there doesn't seem to be a good replacement.

  GSettings provides a way to change settings dynamically, if for example
  the system moves from one network to another.  If GSettings is not available
  for this the a PAC file will need to be used.  (I haven't checked how well
  modifying a PAC file works while in use, though at least tests against IP
  address can be used.)

  At least the gsettings command can be used to set GSettings values, even if
  the desktop environment doesn't provide a GUI.  According to this post, even
  Qt has bindings for GSettings now.  I wonder why.
  http://lwn.net/Articles/437282/

  Is GNOME_DESKTOP_SESSION_ID still set when in Unity?
  If not, is there another variable that is?

There may also be other solutions, but I didn't find a convenient way to check whether GSettings settings have been set by the user or GSettings is just returning the default values.

Perhaps environment variables could be checked if gsettings-desktop-schemas is giving us DIRECT values.

In some ways I would like to check environment variables first and only bother with GSettings and GConf if the relevant environment variables are set.
GSettings is not designed for system/session settings - it is intended for application settings.  However, environment variables are set only prior to starting the app.  Settings can be modified while the app is running.

Comment 19

[Wolfgang Rosenauer [:wolfiR]]

(In reply to Karl Tomlinson (:karlt) from comment #18)
> I'd prefer GNOME_DESKTOP_SESSION_ID over DESKTOP_SESSION because
> DESKTOP_SESSION seems to be used by the display manager, there may be
> inconsistencies in the names of GNOME sessions, and there can be other ways
> to start a GNOME session.

agreed.

> For GSettings, I don't know what to think.

I do not really have experience with GSettings.

>   GSettings provides a way to change settings dynamically, if for example
>   the system moves from one network to another.  If GSettings is not
> available
>   for this the a PAC file will need to be used.  (I haven't checked how well
>   modifying a PAC file works while in use, though at least tests against IP
>   address can be used.)

Just as a sidenote related to that:
We have libproxy support in the tree which solves all that dynamic stuff.
My main reason for not driving this forward on Linux is that libproxy itself needs a JS interpreter for PAC file processing and depending on the setup of libproxy that might be an external Spidermonkey. We had that on openSUSE which leads to symbol clashes and crashing Firefox.
 
>   Is GNOME_DESKTOP_SESSION_ID still set when in Unity?
>   If not, is there another variable that is?

True, we need to consider Unity as well.

> There may also be other solutions, but I didn't find a convenient way to
> check whether GSettings settings have been set by the user or GSettings is
> just returning the default values.

For now I do not think that GSettings is really in use outside of Gnome? At least I haven't heard of that. Therefore it still should be save to not use GSettings if no Gnome is running. If that changes at some point we probably need to change the logic again.
 
> In some ways I would like to check environment variables first and only
> bother with GSettings and GConf if the relevant environment variables are
> set.

AFAIK (at least that was the case on openSUSE but likely upstreamed afterwards) the Gnome GConf proxy settings themselves were falling back to environment variables by default.
So under Gnome Firefox used GConf (and its settings including fallback to env variables) and outside of Gnome there were the env variables used directly.
(libproxy is doing all that transparently nowadays)

Comment 20

[Henrik Skupin [:whimboo][⌚️UTC+1]]

And this is for real not a dupe of bug 821655? All that sounds totally equivalent to what I have seen on bug 821655.

Comment 21

[Karl Tomlinson (:karlt)]

There are similarities and overlaps, but there are different issues involved.

This bug is mostly about a difference in behavior between a distro build with patches to ignore GConf settings when not running under GNOME and Mozilla builds that prefer GConf over environment variables in all desktop environments.  In this bug changing system proxy settings for the desktop environment is not affecting Firefox.  A proposed fix here may not change behaviour under GNOME.

Bug 821655 is due to the change to also check gsettings-desktop-schemas before GConf and was reported for a GNOME desktop environment (even if it is an Ubuntu variant of GNOME).  There, changing system proxy settings does affect Firefox.

Perhaps a solution will resolve both bugs, but that is not clear at this stage.

Comment 22

[Karl Tomlinson (:karlt)]

I'm thinking the following solution could be quite workable:

1) GetProxyFromGConf() and GetProxyFromGSettings() only ever return NS_OK with
   DIRECT when ignore_hosts is set.  Other cases currently returning DIRECT would
   be changed to return NS_ERROR_FAILURE.

2) On failure of either of the above methods, GetProxyForURI() returns
   GetProxyFromEnvironment().

3) To avoid wasting time with GConf when GSettings exist but don't have a proxy
   set, nsUnixSystemProxySettings::Init() is changed to only set mGconf when
   !mProxySettings.

That way environment variables will (usually) be checked even under GNOME (bug 821655), while gsettings can still be used to change settings dynamically or set SOCKS proxies under non-GNOME desktops.

Comment 23

[David Geistert (:d3f3kt)]

I wrote with karlt yesterday and will take this bug as my first bug. Wish me luck :D

Comment 24

[Karl Tomlinson (:karlt)]

(In reply to Karl Tomlinson (:karlt) from comment #22)
> 1) GetProxyFromGConf() and GetProxyFromGSettings() only ever return NS_OK
>    with DIRECT when ignore_hosts is set.  Other cases currently returning DIRECT
>    would be changed to return NS_ERROR_FAILURE.

What I mean here is to still use GConf settings or GSettings if there is a proxy
configured or if there are explicit "ignore-hosts" configured.

Comment 25

[David Geistert (:d3f3kt)]

Attachment: Patch for review

Patch is ready for review

Comment 26

[Karl Tomlinson (:karlt)]

Comment on attachment 721773 [details] [diff] [review]
Patch for review

>-  mGConf = do_GetService(NS_GCONFSERVICE_CONTRACTID);
>+  if (!mProxySettings) 
>+    mGConf = do_GetService(NS_GCONFSERVICE_CONTRACTID);

mProxySettings is not set here, so initialization of mGConf will have to move
to later in this function.

Gecko style varies between files, but follow the style within each file.
https://developer.mozilla.org/en-US/docs/Developer_Guide/Coding_Style contains
the general style guidelines, but there are different variations in different
files.

The general rule is to "always brace controlled statements" such as "if"
blocks.  This file and many others skip the braces iff the block is a single
line jump statement, but otherwise braces single line blocks.  Treating jump
statements differently helps make them visually stand out and the "dangling else" problems don't occur as there is never another statement following a jump statement.

>   if (NS_SUCCEEDED(mGConf->GetStringList(NS_LITERAL_CSTRING("/system/http_proxy/ignore_hosts"),
>                                          getter_AddRefs(ignoreList))) && ignoreList) {
>     uint32_t len = 0;
>     ignoreList->GetLength(&len);
>     for (uint32_t i = 0; i < len; ++i) {
>       nsCOMPtr<nsISupportsString> str = do_QueryElementAt(ignoreList, i);
>       if (str) {
>         nsAutoString s;
>         if (NS_SUCCEEDED(str->GetData(s)) && !s.IsEmpty()) {
>           if (HostIgnoredByProxy(NS_ConvertUTF16toUTF8(s), aHost)) {
>-            aResult.AppendLiteral("DIRECT");
>-            return NS_OK;
>+            return NS_ERROR_FAILURE;

Here, GConf has been explicitly configured to not use a proxy for this host,
so I think the code should stay as it was here, returning "DIRECT".

> {
>+  
>   if (mProxySettings) {

> NSMODULE_DEFN(nsUnixProxyModule) = &kUnixProxyModule;
>+

No need for the additional blank lines.

Have a look at
https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F
which explains how you can mark yourself as the author and include a commit
message in the patch.

Comment 27

[David Geistert (:d3f3kt)]

Attachment: Patch #2 for review

Comment 28

[Karl Tomlinson (:karlt)]

Comment on attachment 722305 [details] [diff] [review]
Patch #2 for review

>-  mGConf = do_GetService(NS_GCONFSERVICE_CONTRACTID);
>+  if (!mProxySettings) {
>+    mGConf = do_GetService(NS_GCONFSERVICE_CONTRACTID);
>+  }
>   mGSettings = do_GetService(NS_GSETTINGSSERVICE_CONTRACTID);
>   if (mGSettings) {
>     mGSettings->GetCollectionForSchema(NS_LITERAL_CSTRING("org.gnome.system.proxy"),
>                                        getter_AddRefs(mProxySettings));
>   }

mProxySettings is not set until the GetCollectionForSchema(), so the
!mProxySettings test and block needs to be after the GetCollectionForSchema()
call.

>   }
>-
>+  
>   nsCOMPtr<nsIArray> ignoreList;

Unnecessary whitespace change here.  We don't make whitespace changes unless
the indentation is misleading or the line is otherwise being modified.  This is to make searching the history of the file easier.

Comment 29

[David Geistert (:d3f3kt)]

Attachment: Patch #3 for review

Comment 30

[Karl Tomlinson (:karlt)]

Comment on attachment 722434 [details] [diff] [review]
Patch #3 for review

Thank you!

Comment 31

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c5db46df8596

Comment 32

[Wolfgang Rosenauer [:wolfiR]]

I couldn't follow that bug recently.
Actually one issue is still not addressed IMHO.
Even if not running under Gnome, and GConf (or GSettings) has an opinion and setting for the proxy, it would be used even if running under some other desktop, right?

Comment 33

[David Geistert (:d3f3kt)]

Oke, well i will explain it to you.

Firefox has an option to detect proxy automatically. When this option is used Firefox scan the GConf, GSettings and then the enviorment variables if a proxy is set.

And right there was the bug. If Firefox don't find a proxy in GConf it stops searching. So Firefox would ever ignore the environment variables. And this is now fixed ;)

I think this bug can be closed, any objections?

Comment 34

[Wolfgang Rosenauer [:wolfiR]]

(In reply to David Geistert (:d3f3kt) from comment #33)
> Firefox has an option to detect proxy automatically. When this option is
> used Firefox scan the GConf, GSettings and then the enviorment variables if
> a proxy is set.
> 
> And right there was the bug. If Firefox don't find a proxy in GConf it stops
> searching. So Firefox would ever ignore the environment variables. And this
> is now fixed ;)

You confirmed what I understood.
Now I'll explain my usecase once again (but this does not need to be handled in this bug but was kind of understood in comment 18).
On a Linux system it's quite possible that you have a proxy configuration done in GConf even if not running Gnome anymore as your current desktop but for example KDE (or whatever).
Still Firefox will always only use GConf (ignore GSettings in that case for simplicity) and the user won't have any idea where to change that.
At least after that patch I still need my patch from comment 17 to fix that usecase.
Just wanted to make that clear.

Comment 35

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/c5db46df8596

Comment 36

[promeneur]

thanks to all

Comment 37

[[e-mail] (:bhearsum)]

I started hitting an issue with proxy server settings + "use system proxy server" that I filed in bug 849792. This is the only bug that landed recently that touches proxy settings AFAICT, could it have caused it?

Comment 38

[Patrick McManus [:mcmanus]]

karlt, promenuer, should we consider a backout until 849792 can be diagnosed?

Comment 39

[promeneur]

i switched from 16.01 to 19 then now i have no more pb.
with kde environment firefox uses "system proxy settings" with success.

Comment 40

[David Geistert (:d3f3kt)]

Promeneur (In reply to promeneur from comment #39)
> i switched from 16.01 to 19 then now i have no more pb.
> with kde environment firefox uses "system proxy settings" with success.

The patch fix is only in Firefox Version 22. So the 19 is not be affected

Comment 41

[promeneur]

(In reply to David Geistert (:d3f3kt) from comment #40)
> Promeneur (In reply to promeneur from comment #39)
> > i switched from 16.01 to 19 then now i have no more pb.
> > with kde environment firefox uses "system proxy settings" with success.
> 
> The patch fix is only in Firefox Version 22. So the 19 is not be affected

yes i know this. that's why i added my comment. Perhaps something has changed in FF 19.

Comment 42

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to promeneur from comment #41)
> yes i know this. that's why i added my comment. Perhaps something has
> changed in FF 19.

You are most likely seeing bug 821655.

Comment 43

[Karl Tomlinson (:karlt)]

Reverting what happens when Bugzilla/Firefox submits a comment on a reloaded page.

(In reply to Patrick McManus [:mcmanus] from comment #38)
> karlt, promenuer, should we consider a backout until 849792 can be diagnosed?

I don't think there's enough evidence to support that at this stage.
Bug 849792 has only been seen by one (important) user and doesn't have steps to reproduce yet.  This patch is also only changing behaviour to that which some people already have, and so any bug likely already existed before changes here.