On Sun 12 Jun 2011, Charles-François Natali wrote:

> The patches look good to me, except that instead of passing
> (addrlen > buflen) ? buflen : addrlen
> as addrlen argument every time makesockaddr is called, I'd
> prefer if this min was done inside makesockaddr itself,
> i.e. perform min(addrlen, sizeof(struct sockaddr_un)) in the
> AF_UNIX switch case (especially since addrlen is only used for
> AF_UNIX).

Actually, I think it should be clamped at the top of the
function, since the branch for unknown address families ought to
use the length as well (it doesn't, but that's a separate issue).
I'm attaching new patches to do the check in makesockaddr(),
which also change the length parameters from int to socklen_t, in
case the OS returns a really huge value.

I'm also attaching new return-unterminated patches to handle the
possibility that addrlen is unsigned (socklen_t may be unsigned,
and addrlen *is* now unsigned in 3.x).  This entailed specifying
what to do if addrlen < offsetof(struct sockaddr_un, sun_path),
i.e. if the address is truncated at least one byte before the
start of sun_path.

This may well never happen (Python's existing code would raise
SystemError if it did, due to calling
PyString_FromStringAndSize() with a negative length), but I've
made the new patches return None if it does, as None is already
returned if addrlen is 0.  As another precedent of sorts, Linux
currently returns None (i.e. addrlen = 0) when receiving a
datagram from an unbound Unix socket, despite returning an empty
string (i.e. addrlen = offsetof(..., sun_path)) for the same
unbound address in other situations.

(I think the decoders for other address families should also
return None if addrlen is less than the size of the appropriate
struct, but again, that's a separate issue.)

Also, I noticed that on Linux, Python 3.x currently returns empty
addresses as bytes objects rather than strings, whereas the
patches I've provided make it return strings.  In case this
change isn't acceptable for the 3.x maintenance branches, I'm
attaching return-unterminated-3.x-maint-new.diff which still
returns them as bytes (on Linux only).

To sum up the patch order:

2.x:
linux-pass-unterminated-4spc.diff
test-2.x-new.diff
return-unterminated-2.x-new.diff
addrlen-makesockaddr-2.x.diff (or addrlen-2.x-4spc.diff)

3.2:
linux-pass-unterminated-4spc.diff
test-3.x-new.diff
return-unterminated-3.x-maint-new.diff
addrlen-makesockaddr-3.x.diff (or addrlen-3.x-4spc.diff)

default:
linux-pass-unterminated-4spc.diff
test-3.x-new.diff
return-unterminated-3.x-trunk-new.diff
addrlen-makesockaddr-3.x.diff (or addrlen-3.x-4spc.diff)