While testing an in-house LDAP server, I observed that mozilla fails to connect to port number 33400 i.e. a value greater than 32k. The port number is defined as a short whilst the underlying c-sdk supports an int. There is some further ambiguity where the Address Book uses integers and the GetPort() method of nsILDAPURL also supports integers. I propose that we support integer values as the ldap c-sdk appears to use 64k as the maximum port value allowed. I will post a patch if this is agreeable.

For the life of me, I can't figure out why I did that. I must have been on crack. A patch would be great. Also, feel free to take this bug if you wish.

taking ownership of this bug.

Comment on attachment 99050 [details] [diff] [review] proposed patch to convert port from short to long This looks good for the XPCOM SDK itself, but the patch will also need to change all the callers in the tree as well, I think.

Dan, I am open to correction on this, but i have searched the tree and all the callers are correctly using the data type PRInt32 for the port value prior to calling nsLDAPConnection::Init(). This is due to the fact that the GetPort() method of nsILDAPURL uses PRInt32 and the value of the port is extracted using that prior to calling the Init(). This includes the following: directory/xpcom/base/src/nsLDAPService.cpp mailnews/addrbook/src/nsAbLDAPReplicationQuery.cpp mailnews/addrbook/src/nsAbLDAPDirectoryQuery.cpp extensions/pref/autoconfig/nsLDAPSyncQuery.cpp xpfe/components/autocomplete/src/nsLDAPAutoCompleteSession.cpp

Assigning this bug to sean.gao@sun.com

I am convinced that the callers' code are not affected by this patch as John said. Thus I will seek for review for this.

Comment on attachment 99050 [details] [diff] [review] proposed patch to convert port from short to long My sr only applies if you've verified all the callers.

wind, please continue the rest work

There are 6 callers in mozilla tree.They are all correctly using the data type PRInt32 for the port value prior to calling nsLDAPConnection::Init().They are : 1.directory/xpcom/base/src/nsLDAPService.cpp caller code: rv = conn->Init(host.get(), port, (options & nsILDAPURL::OPT_SECURE) ? PR_TRUE : PR_FALSE, nsnull, this); port define as: PRInt32 port; 2.mailnews/addrbook/src/nsAbLDAPReplicationQuery.cpp caller code: // initialize the LDAP connection return mConnection->Init(host.get(), port, (options & nsILDAPURL::OPT_SECURE) ? PR_TRUE : PR_FALSE, PromiseFlatString(aAuthDN).get(), listener); port define as: PRInt32 port; 3.mailnews/addrbook/src/nsAbLDAPDirectoryQuery.cpp caller code: rv = ldapConnection->Init(host.get(), port, options, nsnull, messageListener); port define as: PRInt32 port; 4.extensions/pref/autoconfig/nsLDAPSyncQuery.cpp caller code: rv = mConnection->Init(host.get(), port, (options & nsILDAPURL::OPT_SECURE) ? PR_TRUE : PR_FALSE, 0, selfProxy); port define as: PRInt32 port; 5.xpfe/components/autocomplete/src/nsLDAPAutoCompleteSession.cpp caller code: rv = mConnection->InitLDAP(host.get(), port, (options & nsILDAPURL::OPT_SECURE) ? PR_TRUE : PR_FALSE, nsnull, selfProxy); port define as: PRInt32 port; 6.directory/xpcom/datasource/nsLDAPDataSource.js code like this: connection.initLDAP(queryURL.host, queryURL.port, null, generateGetTargetsBoundCallback()) queryURL is a object of nsILDAPURL,the "port" property is inherited from nsIURL and define as: attribute long port;

Peter, Does your sr apply now?

If those are all the callers in the tree, yes.

Yes,these are all the callers in the tree. I change the name of function fom Init to InitLDAP,then do a full build.When I got an error,I record and modify the caller.I got all c/c++ caller by this way. And I search all calls to Init function in js/xul files,check every file to see if it is a call to nsLDAPConnection.Init.I got only one.

update to current trunk to check in

Comment on attachment 132918 [details] [diff] [review] update to current trunk Seems ok. r=Henry and sr=Peter (according to comment #12)

Has beed checked in.