Closed Bug 527478 Opened 15 years ago Closed 9 years ago

System Proxy Settings prefer SOCKS over HTTP

Categories

(Core :: Networking, defect)

All
Linux
defect
Not set
normal

Tracking

()

RESOLVED WONTFIX

People

(Reporter: ian, Unassigned)

References

Details

Attachments

(1 file)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.5) Gecko/20091105 Fedora/3.5.5-1.fc11 Firefox/3.5.5
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.5) Gecko/20091105 Fedora/3.5.5-1.fc11 Firefox/3.5.5

In the case of using "system" proxy settings and /system/http_proxy/use_same_proxy is not set, the SOCKS proxy is used in preference to the HTTP or FTP proxy.

This behaviour is wrong because the protocol specific protocols are more specific. For example, if one wants to specify an HTTP proxy but fall back to a SOCKS proxy for other protocols.

The relevant code is in nsUnixSystemProxySettings::GetProxyFromGConf(). It would be better (and simpler) IMO to only set the type to SOCKS if there is no protocol specific proxy setting

Reproducible: Always

Steps to Reproduce:
1. On a network with a suitable http proxy <proxy host> Set system proxy settings (using Gconf Editor, or Network Proxy Preferences applet etc) to:

/system/http_proxy/host <proxy host>
/system/http_proxy/port <proxy port>
/system/http_proxy/use_http_proxy true
/system/http_proxy/use_same_proxy false
/system/proxy/socks_host <invalid hostname>

2. Try to connect to a well known public web site
Actual Results:  
Connection fails.

Expected Results:  
Connection should have succeeded

As far as I can see, the logic for handling manually set proxies in the preferences UI is the same as I am proposing for system preferences.
Attachment #411217 - Flags: review?(ventnor.bugzilla)
Comment on attachment 411217 [details] [diff] [review]
Patch to fix System proxy Setting logic in unix type systems

Is this patch for 3.5?
Attachment #411217 - Flags: review?(ventnor.bugzilla) → review+
Status: UNCONFIRMED → NEW
Ever confirmed: true
Looks like it, but it still has to go through the usual "land on trunk first, then on branches if approved" cycle. But what's more important is that this patch is apparently reversed - Michael, did you take that into account when you reviewed it?

Also, did you test this patch, Ian?
I don't know this code, but with your patch I'm not sure how the fallback to the SOCKS proxy can work when no specific protocol proxy is set. For example, when looking for a proxy for the http protocol, doesn't the control flow always take the first "if" branch and leave the whole if block after unsuccessfully trying to get an http proxy, without ever entering the SOCKS proxy "else" branch?
Whoops. Yes it is reversed, and yes you are right it doesn't cater for the case where there is no HTTP proxy, but there is a SOCKS proxy. I haven't got a socks proxy ;-(

I think the following fixes it though:

  rv = NS_ERROR_FAILURE;
  if (aScheme.LowerCaseEqualsLiteral("http") || useHttpProxyForAll) {
    rv = SetProxyResultFromGConf("/system/http_proxy/", "PROXY", aResult);
  } else if (aScheme.LowerCaseEqualsLiteral("https")) {
    rv = SetProxyResultFromGConf("/system/proxy/secure_", "PROXY", aResult);
  } else (aScheme.LowerCaseEqualsLiteral("ftp")) {
    rv = SetProxyResultFromGConf("/system/proxy/ftp_", "PROXY", aResult);
  }
  if (NS_FAILED(rv)) {
    rv = SetProxyResultFromGConf("/system/proxy/socks_", "SOCKS", aResult);
  }
Comment on attachment 411217 [details] [diff] [review]
Patch to fix System proxy Setting logic in unix type systems

I only asked because it looked like, to me, these changes were already on trunk.

If it is wrong, then we need a new patch on mozilla-central
Attachment #411217 - Flags: review+ → review-
Ian, do you want to create a new patch for the mozilla-central repository?

https://developer.mozilla.org/En/Developer_Guide/Source_Code/Mercurial
use pac
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: