242690 - void UpdateJSProxyInfo() should check hostname and port at once

Status: RESOLVED WORKSFORME

(SeaMonkey :: Installer, defect)

Description

[benc]

I've been looking at the open all-proxy.js bugs, and I think I see where a piece
of the problem is:

The general logic of the code is:

1134       if(*diAdvancedSettings.szProxyServer != '\0')

(write out the server info)

1146       if(*diAdvancedSettings.szProxyPort != '\0')

(write out the port info)

The proxy prefs will ignore any prefs that do not form a hostip:port tuple, so
this is sort of okay, but doesn't make any sense.

If the installer used a proxy server, then write out the all the info. If not,
then why bother? It isn't like the proxy code in this module is so bug-free that
you know that some strange combination of info here isn't going to pass and send
bad info to the profile.

Comment 1

[Daniel Veditz [:dveditz]]

I don't understand this bug... the installer already doesn't write out
all-proxy.js unless you entered proxy information while installing. None of my
installations have this file.

Comment 2

[benc]

This bug doesn't make sense, because you haven't actually installed in a proxy
and looked at the all-proxy.js file. Look at bug 88211, where the first line is
never written after installing through a proxy.

I've done by best to isolate the problem w/ the file writes in that bug, but
while doing that I noticed what I think is a flawed design to this function.

A proxy config is only valid if you have a hostname and a port (both installer
and browser have this requirement, which I have confirmed by testing).

The two lines I clipped above are preceeded by:

1118   if((*diAdvancedSettings.szProxyServer != '\0') ||
(*diAdvancedSettings.szProxyPort != '\0'))

So it doesn't make sense to me...

if there is proxy server info -or- port info (#1118),

then 
{
if there is server info, write it to disk (#1134)
if there is port info, write it to disk (#1146).

A proxy config won't work w/o BOTH pieces of info, you should be doing:

if((*diAdvancedSettings.szProxyServer != '\0') &&
(*diAdvancedSettings.szProxyPort != '\0'))

...and removing the other if's (if a and b are present, write a, write b).

You should never write just a port or just a server. We do that now, and it can
change a profiles networking prefs and cut off users.

Comment 3

[Matthias Versen [:Matti]]

Seamonkey and Firefox are using a new NSIS based installer. resolving this old bug, please reopen if you still get this with the new installer