System-wide SOCKS proxy settings are not respected

RESOLVED FIXED in mozilla1.9.2a1

Status

()

Core
Networking
RESOLVED FIXED
10 years ago
9 years ago

People

(Reporter: Daniel Luz, Assigned: Andrew Shilliday)

Tracking

({verified1.9.1})

1.9.1 Branch
mozilla1.9.2a1
All
Mac OS X
verified1.9.1
Points:
---

Firefox Tracking Flags

(status1.9.1 .4-fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

10 years ago
User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b1pre) Gecko/20080925021213 Minefield/3.1b1pre
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b1pre) Gecko/20080925021213 Minefield/3.1b1pre

I have a network location setting where there's a SOCKS proxy setting (and no other proxies). Upon activating this location, other applications tested (Safari, Adium) recognize it immediately, without a restart. Firefox seems to completely ignore it.

I don't think this is relevant, but for completeness, this proxy is just pointing to localhost, using ssh dynamic port forwarding.

Reproducible: Always

Steps to Reproduce:
1. On OS X's Network Preferences, set to configure proxies manually, and assign a SOCKS proxy.
2. Make sure Firefox is set to "Use system proxy settings".
3. Test if the proxy is in effect. (For example, go to www.whatismyip.com and check if you are actually connecting through the remote machine.)
Actual Results:  
Firefox keeps connecting directly (or attempting to).

Expected Results:  
Firefox should respect SOCKS proxies. (I'm assuming it at the moment only considers the other proxies)
It was reported to me by a client that network connections from JavaScript *do* obey the system proxy settings, but the browser itself does not. I'll try to come up with a reproduction case on Monday.
(In reply to comment #1)
> It was reported to me by a client that network connections from JavaScript *do*
> obey the system proxy settings, but the browser itself does not. I'll try to
> come up with a reproduction case on Monday.

Alexander are you still trying ?
That ended up being a very confusing situation. I'll follow up and see what I can find out...the problems we saw might now be related.
(Assignee)

Comment 4

9 years ago
I'm experiencing the same problem. 

Firefox:
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1) Gecko/20090616 Firefox/3.5

System:
Darwin 9.7.0 Darwin Kernel Version 9.7.0: Tue Mar 31 22:52:17 PDT 2009; root:xnu-1228.12.14~1/RELEASE_I386 i386

To reproduce:
1) SSH to some machine using the -D option, e.g.,
    ssh -D <xxx> <user>@<host>
2) In System Preferences select Network -> Advanced -> Proxies
3) Under "Select a protocol to configure:" uncheck all but "SOCKS Proxy"
4) With "Socks Proxy" selected, set the SOCKS Proxy Server to localhost, and the port to <xxx>.
5) In Firefox select Preferences -> Network -> Settings
6) Select "Use System Proxy Settings"

Observed behavior: When browsing in Firefox, proxy settings are not used.
Expected behavior: That of Safari: proxy settings are used.
(Assignee)

Comment 5

9 years ago
I'm no Firefox developer, and this comment comes from all of about 30 minutes looking at the source code, but it appears that the problem is in 
toolkit/system/osxproxy/nsOSXSystemProxySettings.mm.

There are two places where the code goes wrong (I think).  First is in line 205
if (NS_FAILED(aURI->SchemeIs(keys->mScheme, &res)) || !res) {

The problem here is that keys->mScheme, as it cycles through the list, is one of
"http", "https", "ftp", "socks", and null, but aURI->SchemIs will only ever return true for the first three.  My suggestion is to add a third clause to prevent the if block from being entered if keys->mScheme is "socks" which, I suppose, should be really appropriate for all uri schemes (correct?)

That's only the first problem. Later, around line 357, we see a call out to FindSCProxyPort which looks up the osx system settings and sets the proxyHost and proxyPort appropriately (the line above is part of that function).

357   rv = FindSCProxyPort(aURI, proxyHost, proxyPort);

359   if (NS_FAILED(rv) || IsInExceptionList(host)) {
360     aResult.AssignLiteral("DIRECT");
361   } else {
362     aResult.Assign(NS_LITERAL_CSTRING("PROXY ") + proxyHost + nsPrintfCString(":%d", proxyPort));
363   }

This *seems* to be constructing what would be the results of PAC file, i.e., statements of the form "DIRECT" or "PROXY <host>:<port>".  The issue is that when a SOCKS proxy is being used, the line should not begin with PROXY, but rather SOCKS (see http://docs.sun.com/app/docs/doc/820-1652/adyrr?a=view)

Updated

9 years ago
Hardware: PowerPC → All
Steven, is it an area you can help out or who is the right person?
I don't know much about this stuff ... but I might be able to help.

I don't know if we have a "right person".

Josh, what do you think?
(Assignee)

Comment 8

9 years ago
Decided not to sleep and learn Objective C instead. Patch on its way.
(Assignee)

Comment 9

9 years ago
Created attachment 385096 [details] [diff] [review]
Patch for toolkit/system/osxproxy/nsOSXSystemProxySettings.mm

Address the issue that Firefox does not assign SOCKS system settings correctly.  There were three underlying problems. 
1) original code checked that url scheme matched either "http", "https", "ftp", or "socks", but no url will never match "socks", so the socks system settings were always ignored.  I added a clause so it would skip checking schemes against "socks".

2) When original code found a scheme match, it would check to make sure it was enabled in system preferences.  If it wasn't, it would break and return no proxy.  This means that if a url matched http, but http was disabled, the socks proxy would never be examined.  I changed code so that it continued the for loop to next proxy entry instead of breaking.  This way it checks each proxy entry for a match as well as whether it's enabled.

3) When proxy information is returned, orginal code had no way of knowing whether the proxy was SOCKS or a regular proxy, it always returned the string "PROXY <host>:<port>" but when a SOCKS proxy is used, it ought to return "SOCKS <host>:<port>" To fix this, I added an extra by-ref argument to the FindSCProxyPort function to return whether the host and port is a socks proxy.

Tested on my own machine and everything seems to work
(Assignee)

Comment 10

9 years ago
As an aside, when Firefox is ready to change its system requirements to be OSX1.5 or later instead of OSX1.4, it will make sense to redo this code to use CFProxySupport

http://developer.apple.com/documentation/CoreFoundation/Reference/CFProxySupport/Reference/reference.html

The API allows you to just ask the system what the best proxy is given a URL, this would greatly simplify this code.
(Assignee)

Updated

9 years ago
Attachment #385096 - Flags: review?(roc)
(Assignee)

Updated

9 years ago
Attachment #385096 - Flags: superreview?(roc)
Attachment #385096 - Flags: review?(roc)
Attachment #385096 - Flags: review?(jamesbunton)
(Assignee)

Comment 11

9 years ago
Comment on attachment 385096 [details] [diff] [review]
Patch for toolkit/system/osxproxy/nsOSXSystemProxySettings.mm

Added Benjamin Smedberg as reviewer
Attachment #385096 - Flags: superreview?(roc)
Attachment #385096 - Flags: review?(jamesbunton)
Attachment #385096 - Flags: review?(benjamin)

Updated

9 years ago
Attachment #385096 - Flags: review?(joshmoz)
Assignee: nobody → andrewshilliday
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
(Assignee)

Updated

9 years ago
Version: unspecified → 1.9.1 Branch
Patch looks great. Cosmetic comments only:

@@ -48,18 +49,16 @@
 #include "nsISystemProxySettings.h"
 #include "nsIGenericFactory.h"
 #include "nsIServiceManager.h"
 #include "nsPrintfCString.h"
 #include "nsNetUtil.h"
 #include "nsISupportsPrimitives.h"
 #include "nsIURI.h"
 #include "nsObjCExceptions.h"
-
-
 class nsOSXSystemProxySettings : public nsISystemProxySettings {

Leave one blank line here

+    PRBool mIsSocksProxy;

Make it PRPackedBool and move it to the end

+    // Check whether we're using a SOCKS proxy

This comment is a bit misleading, it would be more like "return whether we're using a SOCKS proxy", probably just remove it.

Updated

9 years ago
Attachment #385096 - Flags: review?(joshmoz) → review+
(Assignee)

Comment 13

9 years ago
Created attachment 385302 [details] [diff] [review]
Next version of patch file, taking ROC's comments into account
Attachment #385096 - Attachment is obsolete: true
Attachment #385302 - Flags: review?(roc)
Comment on attachment 385302 [details] [diff] [review]
Next version of patch file, taking ROC's comments into account

I think Josh wants to review this too.
Attachment #385302 - Flags: review?(roc)
Attachment #385302 - Flags: review?(joshmoz)
Attachment #385302 - Flags: review+

Updated

9 years ago
Attachment #385302 - Flags: review?(joshmoz) → review+
(Assignee)

Updated

9 years ago
Keywords: checkin-needed

Comment 15

9 years ago
pushed to mozilla-central

http://hg.mozilla.org/mozilla-central/rev/2533ef04e498
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Something we could consider to have for 1.9.1.1?
Flags: wanted1.9.1.x?
Target Milestone: --- → mozilla1.9.2a1
(Assignee)

Updated

9 years ago
Blocks: 500983
(Assignee)

Updated

9 years ago
Attachment #385302 - Flags: approval1.9.1.3?
Comment on attachment 385302 [details] [diff] [review]
Next version of patch file, taking ROC's comments into account

Approved for 1.9.1.4, a=dveditz for release-drivers
Attachment #385302 - Flags: approval1.9.1.3? → approval1.9.1.4+
status1.9.1: --- → wanted
Flags: wanted1.9.1.x?
Keywords: checkin-needed
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/4d743209b145
status1.9.1: wanted → .4-fixed
Keywords: checkin-needed
Daniel or Andrew, could you take the nightly 1.9.1 build and try it in your networked environment to see if this is fixed now?

ftp://ftp.mozilla.org/pub/firefox/nightly/latest-mozilla-1.9.1/

Thanks.
(Assignee)

Comment 20

9 years ago
Al, Done; All seems to work.
Thanks, Andrew. Marking as verified for 1.9.1.
Keywords: verified1.9.1
You need to log in before you can comment on or make changes to this bug.