PR_StringToNetAddr should zero the socket address first.

ASSIGNED
Assigned to

Status

NSPR
NSPR
P3
normal
ASSIGNED
18 years ago
11 years ago

People

(Reporter: Wan-Teh Chang, Assigned: Wan-Teh Chang)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Assignee)

Description

18 years ago
It is good programming practice, and required on
some platforms, to zero a socket address (e.g.,
struct sockaddr_in) before use.

In Bug #54796, we fixed PR_InitializeNetAddr and
PR_SetNetAddr to zero the socket address when possible.
PR_EnumerateHostEnt already zeros the socket address.
So the only remaining function that sets a socket
address is PR_StringToNetAddr.

A simple patch is to zero the whole PRNetAddr in
the beginning of PR_StringToNetAddr, as is done
in PR_EnumerateHostEnt.  However, this will break
callers of PR_StringToNetAddr who incorrectly passes
in a pointer to a struct sockaddr_in, which is smaller
than PRNetAddr but compatible with the 'inet' member
of PRNetAddr.

Another issue is that it is possble that the caller
may have already set the 'port' field before calling
PR_StringToNetAddr (the 'port' field happens to be
at the same offset in both the 'inet' and 'ipv6' union
members), so if PR_StringToNetAddr zeros the socket
address, even if just the 'inet' or 'ipv6' member,
it will wipe out the value of 'port'.

So it's not clear to me whether we should "fix" this.
Perhaps documenting that callers of PR_StringToNetAddr
must zero the PRNetAddr before calling is sufficient.

Comments?
(Assignee)

Updated

18 years ago
Status: NEW → ASSIGNED
Priority: -- → P3
Target Milestone: --- → Future

Comment 1

14 years ago
Unfortunately the function isn't documented, so obviously callers will have had
to figure out correct usage themselves. Documentation should be added whatever
is done.

Re: breakages caused by incorrect usage of the API
Is it valid to worry about callers who are misusing the API? Use of a function
in other than the documented way (i.e. function signature) can always cause
problems later when behaviour is changed. I don't think this should prevent the
change going ahead.

Re: clients who set the port first
I guess there are 2 options:
a) change behaviour at major version, document it and break clients
b) zero everything except the port (e.g. save/restore)

Since anyone wanting to use the address will need to set the port at some stage,
perhaps (b) is best?

Comment 2

14 years ago
Wan-Teh,

Do you know of any specific programs that are using this API without passing a
pointer to a structure of sufficient size ?

No longer depends on: 344809
QA Contact: wtchang → nspr
You need to log in before you can comment on or make changes to this bug.