78119 - SOCKS - "network.proxy.socks" needs to be version specific.

Status: VERIFIED FIXED

(Core :: Networking, defect, P3)

Description

[benc]

networking feature needs help, from bug 50679

Communicator 4 supported SOCKS V4, which was described in the prefs UI as 
"Socks". The preference was saved to:

user_pref "network.proxy.socks"

mozilla/NS6.x supports SOCKS V5. The two protocol versions do not have 
handshake rules or reverse compatability, so we are completely incompatible 
with SOCKS V4 proxy servers (bug 65583). 

The prefs UI is going to be updated to reflect the new version "probably the 
string "SOCKS V5", but we also need to decide if we are going to change the 
prefs in prefs.js

There seem to be two ways to implement this:

Migrate the curent user_pref "network.proxy.socks" into two values:

user_pref "network.proxy.socks4"
user_pref "network.proxy.socks5"

-OR-

Create a new pref just for SOCKS V5 (and leave the old one implicit for V4)

user_pref "network.proxy.socks5"

By writing SOCKS V5 prefs into the existing entry from Communicator, we go deaf
to the SOCKS server if it doesn't support both versions. (the original problem
here). Unless someone wants to EOL SOCKS V4, I think we should leave some 
legacy value behind, someday we might support that value again.

NOTE: NIM doesn't have this problem because it uses a prefs structure that 
won't work here (a pair of hostname+port values + a protocol type selector 
[SOCKS4 SOCKS5 HTTPS]).

Comment 1

[Daniel Veditz [:dveditz]]

I think Brian owns prefs now. I know I don't

Comment 2

[Hong Kwon]

isn't the owner of pref the owner of the code that handles proxy support? 

Comment 3

[Jussi-Pekka Mantere]

Gagan?

Comment 4

[Brian Nesse (gone)]

This is not a preferences-backend bug. Addition or modification of preferences
is up to the module owner(s) using them.

Comment 5

[benc]

This is a standards implementation and design question.

Can we get a decision by nsbeta1 so this will be working in Netscape 6.5? m0.9.3
is too far away.

Comment 6

[benc]

mass move, v2.
qa to me.

Comment 7

[Mitch Faatz]

We will need to support both SOCKS V4 and V5 for enterprise customers.  We
should at least get this fix in before 6.1 ships, and we will address other
SOCKS V4/V5 support / completeness issues as soon as possible in future
releases.  We should implement benc's suggestion using two values, one for each
version.

Comment 8

[benc]

I don't care which proposal we use, as long as we don't RTM with the current
situation. I think we all want to avoid having connectivity problems with
NS6/eClient and Microsoft servers (which are SOCKS 4 only) if possible.

The prefs UI was fixed already.

Comment 9

[Steve Meredith (gone)]

If we migrate the curent user_pref "network.proxy.socks" into two values:

user_pref "network.proxy.socks4"
user_pref "network.proxy.socks5"

What do we do with the value of the existing "network.proxy.socks" if it exists?
Do we assume it is a socks 5 value entered via the Mozilla preferences or a
socks 4 value migrated from a version 4.x installation?

Comment 10

[Mitch Faatz]

if there is an existing "network.proxy.socks" I think we would be right more
times than not if we assumed it was for Socks V4 prefs and moved them over to
"network.proxy.socks4".  Does the prefs UI allow viewing and setting of V4 and
V5 prefs separately?

Comment 11

[Steve Meredith (gone)]

In the current version, you can only set SOCKS v5 prefs.

Possible answer to my previous question that just occurred to me: copy the 
user_pref "network.proxy.socks" to BOTH "network.proxy.socks4" 
and "network.proxy.socks5". That seems like the best solution. 

All this assumes that we will support SOCKS v4 in the future.

Comment 12

[benc]

For compatability, there is a good case for leaving "socks" for v4, and then
read-migrating into a "socks5" for v5.

Now that I have stared at this bug for a while, its strikes me as needlessly
literal to create a "socks4" pref that we don't support.

We got into this problem because the SOCKS documentation was non-existent.
Probably a brief paragraph or two (which I can provide), can undo most of the
damage.

Comment 13

[Steve Meredith (gone)]

Why not leave it at "network.proxy.socks" and add a preference which specifies
v4 or v5? 

Comment 14

[Steve Meredith (gone)]

I am taking ownership of this.

Comment 15

[benc]

That sounds good to me. For manual, we only allow specification of one
hostname/IP for SOCKS, and it is also consistent with our SOCKS implementations
in AIM and NIM.

For PAC, it solves the problem in bug 78176.

In the long run, we might need to actually support the two protocols explicity &
simulatenously, but that would be best implemented via PAC or would require a
wholesale manual config re-write.
 

Comment 16

[Steve Meredith (gone)]

I will make this change as part of 65583. There is no point in doing it until then.

Comment 17

[Steve Meredith (gone)]

Fixed by fix for 65583.

Comment 18

[Trix Supremo]

reassigning QA Contact to myself

Comment 19

[Hong Kwon]

changing QA contact to [e-mail]

Comment 20

[benc]

on a UI level fixed in bug 90989.