void UpdateJSProxyInfo() should check hostname and port at once

RESOLVED WORKSFORME

Status

SeaMonkey
Installer
RESOLVED WORKSFORME
14 years ago
9 years ago

People

(Reporter: benc, Unassigned)

Tracking

Trunk
x86
Windows XP

Firefox Tracking Flags

(Not tracked)

Details

(URL)

(Reporter)

Description

14 years ago
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.

Updated

14 years ago
OS: MacOS X → Windows XP
Hardware: Macintosh → PC
(Reporter)

Updated

14 years ago
Blocks: 88211
(Reporter)

Updated

14 years ago
No longer blocks: 88211
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.

Updated

14 years ago
Summary: void UdpateJSProxyInfo() should check hostname and port at once → void UpdateJSProxyInfo() should check hostname and port at once
(Reporter)

Comment 2

14 years ago
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.
Product: Browser → Seamonkey

Updated

12 years ago
QA Contact: bugzilla → general
Blocks: 84732
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
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.