362619 - merging Sun and Mozilla libldap: merging common srcs.

Status: RESOLVED FIXED

(Directory Graveyard :: LDAP C SDK, defect)

Description

[Anton Bobrov]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.1) Gecko/20061010 Firefox/2.0
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.1) Gecko/20061010 Firefox/2.0

this is to merge the rest of libldap. ok, i got scared by looking at request/
result/abandon diffs at first and i told you so already but its not as bad as
i thought. the major changes were introduced by the fix for Bug 140182 "async 
I/O improvements" by Mark. after checking out pre bugfix 140182 revisions of
all the files affected and doing diffs with current Sun code it appears they
are almost identical. the patch to follow should bring them insync completely.

changes proposed are mostly lightweight :

- new ldap_url_parse_no_defaults() public API.

- some minor cleanup where it felt like needed.

- better quotes handling in ldap_explode() 
  [ Sun CR 6341090 ].

- EPIPE error handling when sending requests.

the last one does require special attention because it might not be clear
at first what all that code is for. basically it handles the cases where
the server will half close for one reason or another and the client gets
broken pipe error in the middle of ber flush due to server side closed 
for writing. this is no problem normally unless before closing for writes
the server sends us some response ie Unsolicited Notification explaining
why its closing but we are still in the middle of ber flush and since it
fails we gonna assume that connection has terminated, abort the request
and return Server Down to the user without trying to fetch anything from
response queue.

what new code does in situations like described above is looking out for
EPIPE condition and if it happens trying to do one more poll/read to see
if we got anything from the server before broken pipe happened while we
were transmitting.

one particular use case for this is when nsslapd-maxbersize or similar
parameter is set on the server side. this is what it does if you are not
familiar with it :

"Defines the maximum size in bytes allowed for an incoming message. This limits the size of LDAP requests that can be handled by the Directory Server. Limiting the size of requests prevents some kinds of denial of service attacks."

and here is what you get when trying to send > nsslapd-maxbersize with
ldapsearch for example :

ldap_search: Can't contact LDAP server

and with new EPIPE handling code in place you get this :

ldap_search: Administrative limit exceeded
ldap_search: additional info: B1 - Client request contains an ASN.1 BER tag that is corrupt or connection aborted -See Directory Server error log for more information.

this is mainly why i wrote that EPIPE handler but there can be other
use cases like server shutting down while we are still in ber flush
or server reporting some other limit/s where its unable to continue.
things of that nature.

the code might look a wee bit funny and maybe there are better ways
of doing it. i had to come up with this hack to make it work but it
does look kinda heavy and out of place when i look at it now so if
you have a better idea speak you mind please coz i'm ready to redo
it if there is a better way.

so thats kinda it really. please review at the earliest opportunity
.


Reproducible: Always

Comment 1

[Anton Bobrov]

Attachment: the first cut of the patch

Comment 2

[Anton Bobrov]

i should also mention that this patch has been tested on Solaris and Linux.

Comment 3

[Rich Megginson]

Diff looks good.

Comment 4

[Mark Smith [:mcs]]

Comment on attachment 247308 [details] [diff] [review]
the first cut of the patch

I think this is OK.  My only concern is:  is there a chance that the call to nsldapi_result_nolock() inside nsldapi_send_server_request() will hang?  It worries me to add "get response" kind of code inside the "send request" code.  I am not an expert on EPIPE, but I guess it will only be raised on a TCP connection of the other end has gone away?

Comment 5

[Anton Bobrov]

(In reply to comment #4)

> I think this is OK.  My only concern is:  is there a chance that the call to
> nsldapi_result_nolock() inside nsldapi_send_server_request() will hang?  It

i know what you mean. thing is FIN/RST/error is still an event and therefore 
will trigger poll to wake and return. subsequent read will get us -1/0 if no
data is in there but we have almost immediate return in any case so i do not
perceive this as a problem.
 
> I am not an expert on EPIPE, but I guess it will only be raised on a TCP
> connection of the other end has gone away?
 
yeah, we gonna get it when we are trying to write to a socket [ upon send
request / flush ] that has received an RST.

Comment 6

[Mark Smith [:mcs]]

OK, please commit.

Comment 7

[Anton Bobrov]

done. thanx for reviews!

Checking in mozilla/directory/c-sdk/ldap/include/ldap-extension.h;
/cvsroot/mozilla/directory/c-sdk/ldap/include/ldap-extension.h,v  <--  ldap-extension.h
new revision: 5.8; previous revision: 5.7
done
Checking in mozilla/directory/c-sdk/ldap/libraries/libldap.ex;
/cvsroot/mozilla/directory/c-sdk/ldap/libraries/libldap.ex,v  <--  libldap.ex
new revision: 5.5; previous revision: 5.4
done
Checking in mozilla/directory/c-sdk/ldap/libraries/libldap/abandon.c;
/cvsroot/mozilla/directory/c-sdk/ldap/libraries/libldap/abandon.c,v  <--  abandon.c
new revision: 5.4; previous revision: 5.3
done
Checking in mozilla/directory/c-sdk/ldap/libraries/libldap/disptmpl.c;
/cvsroot/mozilla/directory/c-sdk/ldap/libraries/libldap/disptmpl.c,v  <--  disptmpl.c
new revision: 5.4; previous revision: 5.3
done
Checking in mozilla/directory/c-sdk/ldap/libraries/libldap/getdn.c;
/cvsroot/mozilla/directory/c-sdk/ldap/libraries/libldap/getdn.c,v  <--  getdn.c
new revision: 5.3; previous revision: 5.2
done
Checking in mozilla/directory/c-sdk/ldap/libraries/libldap/getfilter.c;
/cvsroot/mozilla/directory/c-sdk/ldap/libraries/libldap/getfilter.c,v  <--  getfilter.c
new revision: 5.6; previous revision: 5.5
done
Checking in mozilla/directory/c-sdk/ldap/libraries/libldap/ldap-int.h;
/cvsroot/mozilla/directory/c-sdk/ldap/libraries/libldap/ldap-int.h,v  <--  ldap-int.h
new revision: 5.9; previous revision: 5.8
done
Checking in mozilla/directory/c-sdk/ldap/libraries/libldap/request.c;
/cvsroot/mozilla/directory/c-sdk/ldap/libraries/libldap/request.c,v  <--  request.c
new revision: 5.8; previous revision: 5.7
done
Checking in mozilla/directory/c-sdk/ldap/libraries/libldap/srchpref.c;
/cvsroot/mozilla/directory/c-sdk/ldap/libraries/libldap/srchpref.c,v  <--  srchpref.c
new revision: 5.4; previous revision: 5.3
done
Checking in mozilla/directory/c-sdk/ldap/libraries/libldap/unbind.c;
/cvsroot/mozilla/directory/c-sdk/ldap/libraries/libldap/unbind.c,v  <--  unbind.c
new revision: 5.5; previous revision: 5.4
done
Checking in mozilla/directory/c-sdk/ldap/libraries/libldap/url.c;
/cvsroot/mozilla/directory/c-sdk/ldap/libraries/libldap/url.c,v  <--  url.c
new revision: 5.4; previous revision: 5.3
done