[Date Prev][Date Next] [Chronological] [Thread] [Top]

Re: commit: ldap/libraries/libldap url.c

Hallvard B Furuseth wrote:
Howard Chu writes:
hallvard@OpenLDAP.org wrote: 
	url.c  1.103 -> 1.104
Declare enough buffer space for out-of-range URL port numbers

It would have been better simply to never accept out-of-range port numbers.
lud_port should have been an unsigned short instead of int. Or just test for
the correct range on assignment and return an error as necessary.


Good point.  Not quite sure if we can expect port numbers to be < 65536
though.  (Some OS could have a non-TCP/IP network protocol and an API
with larger port numbers.)

But LDAP is only defined over TCP, so this wouldn't matter unless/until something other than TCP comes along with an LDAP spec.

BTW, has IPv6 only 16-bit ports?  A brief browse suggests yes; I was a
bit surprised.  I can imagine that might not scale well in the future.

Yes, ports are part of TCP/UDP, not the IP layer. IPv6 just expands the host address, doesn't change the semantics of TCP or UDP.

Anyway, I don't think this is worth breaking binary compatibility for
now, and I just noticed that this is in desc2str_len() which does not
even produce a string, it just checks its length.  So I suggest

	if ( u->lud_port ) {
		/* ":port" or "host:port" */
		unsigned	p = u->lud_port;
		if ( p > 65535 )
			return -1;
		len += (p > 999 ? 5 + (p > 9999) : p > 99 ? 4 : 2 + (p > 9));
		...


That would be fine.
--
  -- Howard Chu
  Chief Architect, Symas Corp.  http://www.symas.com
  Director, Highland Sun        http://highlandsun.com/hyc/
  Chief Architect, OpenLDAP     http://www.openldap.org/project/