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

Re: (ITS#4879) URL parse/unparse troubles



h.b.furuseth@usit.uio.no wrote:
> Full_Name: Hallvard B Furuseth
> Version: HEAD
> OS: Linux
> URL: 
> Submission from: (NULL) (129.240.202.105)
> Submitted by: hallvard
> 
> 
> Here are some URLs parsing/building bugs in libldap/url.c.
> Some may be design bugs.  Possibly some fixes will need backwards
> bug-compatibility.
> 
> I show URLs converted from string to LDAPURLDesc format with
> ldap_url_parse(), back to string format with ldap_url_desc2str()
> when the result differs from the original, back to LDAPURLDesc...
> 
> ================
> 
> The url.c Novell kludge does not work (for the example URL).
> If the bug is old and nobody has complained, maybe it should be
> removed.  If it's fixed instead, note that the code looks like
> it'll handle Novell URLs with '/' in components after hostport:
> ldap_url_parse_ext() does strchr( url, '/' ) very early to find
> the end of the hostport.

Hm, I think I see what you mean, but I don't see what to do about that.

> ldap://111.222.333.444:389??cn=abc,o=company ->
>         error 5 (LDAP_URL_ERR_BADURL)

This now works.
> 
> Example URL without port number:
> 
> ldap://111.222.333.444??cn=abc,o=company ->
>         host "111.222.333.444??cn=abc,o=company", DN NULL ->
> ldap://111.222.333.444??cn=abc,o=company:389/??base

This now works.
> 
> ================
> 
> ldap_url_desc2str() outputs IPv6 host addresses as "addr" rather
> than "[addr]".  (LDAPURLDesc does not contain the '[]'s in
> .lud_host, nor any flag which says it's an IPv6 host address).
> 
> I don't know the syntax of such addresses, but I seem to remember
> they contains ':'s.  If so the parser could fail if an IPv6
> address does not contains ':', and the URL builder could test for
> ':' to determine if there should be [] around the host part.

This now works.
> 
> Also, what does stuff between the ']' and ':port' mean?
> Should anything be allowed there?
> 
> ldap://[::1]what's this:222/    -> host "::1", port 222 ->

This is now correctly rejected.

> ldap://::1:222/??base           -> error 5 (LDAP_URL_ERR_BADURL)
> 
> ================
> 
> Converting "%00" to char* should return an error, since the char*
> will not match the source URL:
> 
> ldapi://a%00b/cn=d%00e          -> host "a", DN "cn=d" ->
> ldapi://a/cn=d??base

I've left this alone for now.

> ================
> 
> The RFCs 4516/2255 hostport part has grammar [host [":" port]],
> but ldap_url_desc2str() produces [host][":" port].
> 
> ldap://                         -> host NULL, port 389 ->
> ldap://:389/??base
> 
> That's OK as an ldap_url_parse() OpenLDAP extension, but not as
> ldap_url_desc2str() output given to non-OpenLDAP applications.

This is a mess. The ldap: scheme definition has always been broken, and 
it certainly does not conform to the basic URI syntax in RFC3986 section 
3. In particular, RFC3986 forbids a URI from containing "//" when the 
authority component is absent. Looks like RFC4516 should not have been 
approved in its current state.

> ================
> 
> While I'm writing, I wonder if OpenLDAP 2.4 could omit defaults
> and final "?"s from the ldap_url_desc2str() output.  E.g. ldap://
> would become ldap:// and not ldap://:389/??base.  But maybe we get
> into trouble if ldap.conf has another port or something.

I thought about changing this, but left it alone for now. Agreed, the 
interaction with ldap.conf isn't clear.

> ================
> 
> ldapi URL (un)parsing has several problems:
> 
> ldap_url_parse() always treats ':' as a host:port separator.
> Thus one can produce an ldapi LDAPURLDesc with a meaningless port.
> However ldap_url_desc2str() does not URL-escape ':', and it also
> takes port!=0 to mean it is not an ldapi URL and needs no
> '/'-escaping.  Similarly filenames with initial '[' do not work.
> 
> ldapi://%2Ftmp%2Ffoo%3A222/     -> host "/tmp/foo:222" (correct) ->
> ldapi://%2Ftmp%2Ffoo:222/??base -> host "/tmp/foo", port 222 ->
> ldapi:///tmp/foo:222/??base     -> host NULL, DN "tmp/foo:222/"
> 
> ldapi://%2Ftmp%2Ffoo:000/       -> host "/tmp/foo" ->
> ldapi://%2Ftmp%2Ffoo/??base
> 
> ldapi://%2Ftmp%2Ffoo:bar/       -> error 5 (LDAP_URL_ERR_BADURL)

These are all fixed. Note that the "ldapi:///tmp/foo:222/" case simply 
means an ldapi:// URL with a DN of "tmp/foo:222/" which is legal in a 
URL parsing sense, even though DN validation would reject it.
> 
> ====
> 
> The current implementation would handle some non-Unix ldapi
> filenames wrong.  (They'll need well-defined URL-escaping rules
> if they get implemented, so it might as well be fixed now.)  Of
> course these can also be strange Unix relative filenames...
> 
> Windows-like filenames:
> 
> ldapi://C%3A%2Fldapi/           -> host "C:/ldapi" ->
> ldapi://C:%2Fldapi/??base       -> error 5 (LDAP_URL_ERR_BADURL)
> 
> Tops-20-like filenames resemble IPv6 addresses (not that I expect
> to port OpenLDAP to Tops-20:-)
> 
> ldapi://[ldap.var.run]ldapi/    -> host "ldap.var.run" ->
> ldapi://ldap.var.run/??base
> 
> URL-escaping the '[' and ']' does not help; the '[' is lost:
> 
> ldapi://%5Bldap.var.run%5Dldapi/ -> host "ldap.var.run]ldapi" ->
> ldapi://ldap.var.run%5Dldapi/??base
> 
These are all fixed.

-- 
   -- 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/