bbaetz says this has been broken for some time, but I haven't bothered to write a bug until now: STEPS: On just about any post 0.9.0 version of mozilla, PAC directives "DIRECT" and "PROXY" worked, but "SOCKS" never has.

It not broken, it was just never checked in, because PAC was done before socks4 support was. Its about 5 lines of code arround line 88 of nsProxyAutoConfig.js - basic, do you want this? Don't forget bug 78176.

PAC

Comment on attachment 54129 [details] [diff] [review] patch (updated version of attachment 54125 [details] [diff] [review]) >- // TODO warn about SOCKS >+ // we ignore everything else past the first proxy. >+ // we could theoretically check isResolvable now and continue >+ // parsing (see bug 84798). but for now... >+ var hostport = /^(.....) ([^:]+)(:\d+)?/(proxy); I'd prefer something which just grabbed the first word, rather than expecting 5 characters. You should do some sort of error handling if you get an unparsable string, right? Or do we just use DIRECT?

currently we just do DIRECT (by not passing a type back). What sort of error handling do you expect? A javascript error/warning/message?

silly me, forgot to '+' the '\w'

improved the regexp a bit, made proxy type matching case insesitive and made sure that we really ignore everything else past the first proxy.

tingley: mind taking another look?

I think this is ready for review Note that if this is checked in bug 112969 will probably need a new testcase or something.

Comment on attachment 61242 [details] [diff] [review] patch (take 5) [includes fix for bug 91630] >+ type.value = "http"; >+ } >+ // besides http proxy all other proxy types need a port number >+ else if(typehostport[3] != null) { >+ port.value = typehostport[3].substr(1); Are we not defaulting to 1080 for socks? r=bbaetz in any event - do what ns4 did.

okay I tested PAC with nn4 and it turns out that SOCKS proxy defaults to port 1080 so here is take 6 (hopefully the last one)

the latest attachment is not a patch :P

Ooooopsssss sorry about that. Here is the patch

Comment on attachment 61492 [details] [diff] [review] patch (take 6) [includes fix for bug 91630] (the real thing) sr=darin

assigning to tingley to get patch in after trunk opens

Fix checked in.

+verifyme I've added a sample PAC file. You need to download this file and modify it for your test environment, b/c the SOCKS server mentioned does not exist anymore.

VERIFIED: linux 2002-04-09-08 (pre branch) Actually, you only need to map the hostname in your /etc/hosts to make the file work if you want to try it yourself. After realizing that Linux uses the TCP option for timestamps, was able to use snoop on my server to verify the version number is 04. # snoop -i pac -p 4 -x 66 4 0.00000 h-10-169-111-116.nscp.aoltw.net -> carbuncle SOCKS C port=1233 0: 0401 0051 3fc9 391d 4d4f 5a00 ...Q?.9.MOZ. I'll verify the other platforms some other time.

uh, this didn't work in the 0.9.4 branch right?