226615 - PAC: cannot specify SOCKS5 proxy

Status: VERIFIED FIXED

(Core :: Networking, defect)

Description

[Darin Fisher]

The 8tre wrote:

>Hi all,
>
>I'm using Firebird 0.7 on windows.
>I'm using a proxy.pac file containing SOCKS4 and SOCKS5 proxies.
>Something like :
>function FindProxyForURL(url, host)
>{
>   return "SOCKS5 14.147.6.155:3560; SOCKS4 258.165.349.12:2412";
>}
>
>Firebird has no problem using proxies specified as SOCKS4 but when I
>the first proxy in the list is a SOCKS5 one a dialog box shows up
>asking to install PSM.
>
>I found a PSM test page on the web telling me that my PSM version is
>2.4 and that the manager is currently running.
>
>It would be nice if one of you could give some hints on this Problem.
>
>Thanks in advance,
>--
>The 8tre
>

there is code to support SOCKS5, but it appears to be broken.  it looks like we
need to map the PAC result string "SOCKS5" to a necko socket type of just plain
"socks".  currently, we map it to "socks5" which is not registered as a socket type.

Comment 1

[Darin Fisher]

Attachment: v1 patch

this patch should do the trick.  i still need to verify it though.

Comment 2

[Darin Fisher]

Attachment: v1.1 patch

v1 patch works.  this patch fixes a two warnings.

Comment 3

[Darin Fisher]

Comment on attachment 137280 [details] [diff] [review]
v1.1 patch

bz: i think this patch is a good candidate for 1.6 final.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 137280 [details] [diff] [review]
v1.1 patch

sr=bzbarsky.  Does that make kProxyType_SOCKS5 unused?	If so, can we kill it
off?

Comment 5

[Darin Fisher]

bz: nope, kProxyType_SOCKS5 is still used.  it is used in ExtractProxyInfo.

Comment 6

[Darin Fisher]

Comment on attachment 137280 [details] [diff] [review]
v1.1 patch

i tested this patch against a Dante SOCKS5 proxy w/ a custom PAC file to verify
that i was able to direct mozilla to use my SOCKS5 proxy.

i think this is a safe and good patch for 1.6.

Comment 7

[tor]

Comment on attachment 137280 [details] [diff] [review]
v1.1 patch

a=tor for 1.6 checkin.

Comment 8

[Asa Dotzler [:asa]]

Has this been landed yet? It's going to need to get in quickly if it's going to
make the 1.6 branch.

Comment 9

[Darin Fisher]

fixed-on-trunk for 1.6 final

Comment 10

[benc]

VERIFIED per Darin's comments.