Last Comment Bug 231965 - LDAP Download User Interface and function broken
: LDAP Download User Interface and function broken
Status: RESOLVED FIXED
: crash, fixed1.8
Product: MailNews Core
Classification: Components
Component: Address Book (show other bugs)
: Trunk
: x86 Linux
: -- normal with 16 votes (vote)
: ---
Assigned To: Mark Banner (:standard8)
:
Mentors:
: 140131 270156 284602 (view as bug list)
Depends on:
Blocks: 311632
  Show dependency treegraph
 
Reported: 2004-01-23 09:59 PST by Hadmut Danisch
Modified: 2008-12-10 05:16 PST (History)
13 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Change to Download All (2.45 KB, patch)
2005-10-08 01:53 PDT, Mark Banner (:standard8)
no flags Details | Diff | Splinter Review
Patch v2 (3.55 KB, patch)
2005-10-08 02:27 PDT, Mark Banner (:standard8)
dmose: review+
mozilla: superreview+
asa: approval1.8rc1+
Details | Diff | Splinter Review

Description Hadmut Danisch 2004-01-23 09:59:55 PST
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.5) Gecko/20031107 Debian/1.5-3
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6) Gecko/20040122 Debian/1.6-1

Hi,

I just stepped to Mozilla 1.6 and still found that LDAP 
address book download for offline working does still not work. 
This might be related to bugs 140131, 144204, 230211, but 
it covers more and might not be the same.

I found several problems which indicate that the whole 
LDAP address book module does not properly fit into the
current version of Mozilla and needs some update:

- When trying to download the LDAP directory, an window
  is opened and asks for username and password. 
  In contrast to the bug reports mentioned above I found
  that Mozilla _is_ doing something after entering these, 
  but the user interface is misleading:

  In a first step, mozilla tries to find an object where
  the "mail" attribute is equal to the given username. 
  A normal user has no chance to know this and that such 
  a user has to exist. When found, Mozilla indeed binds as
  this particular user.

  I'd suggest to complete remove this username/password dialog,
  since the BINDDN and password used to query the LDAP server
  is already known and stored in the password repository. 
  Asking again for username/password does not make sense.

- Even if Mozilla does bind successfully, it ignores the 
  basedn given in the configuration. This needs to be fixed.
  The basedn is NULL which obviously does not give any results.

- I'd like to remember the Filter bug, which is mentioned in 
  bugs 135273, 138478, 180012, 215958, 227482, and also applies
  here. This bug is ages old, Mozilla still does not use the
  filter expression.

So the LDAP part in common needs some refurbishment.

If you need some debugging support, just ask me.

regards
Hadmut


Reproducible: Always

Steps to Reproduce:
1. Enter an LDAP addressbook
2. Try to download for offline work
3.




A pretty good way to debug LDAP queries is to use
ethereal.
Comment 1 james davis 2004-03-12 06:59:58 PST
I might add that with Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7b)
Gecko/20040310 it is crashing. Right after entering the username/pw to bind to,
moz crashes. Talkback isn't avail.
Comment 2 Robin Monks 2004-06-29 09:55:40 PDT
*** Bug 140131 has been marked as a duplicate of this bug. ***
Comment 3 dju` 2004-08-17 10:26:45 PDT
I can confirm this behaviour with Thunderbird 0.7.3 (20040812).
Comment 4 Adam Doppelt 2004-10-29 14:05:49 PDT
Agreed - LDAP download seems very broken. It asks for a password and pops up a
progress dialog but doesn't appear to actually download anything. Other users
have reported the same problem with our corporate ldap server (win2k active
directory).
Comment 5 Matt Roca 2005-03-30 11:03:09 PST
Running Mozilla 1.7.5 on Windows 2000.  

I can confirm this behaviour in this release.

We use anonymous LDAP for address lookup, so the username/password dialog that
comes up when you click "Download" doesn't work for me.

I'd like to see this fixed.  I agree with all of the suggestions and findings of
the original poster.
Comment 6 Barton Phillips 2005-10-07 10:22:43 PDT
Please fix this. It is a needed frature.
Comment 7 Mark Banner (:standard8) 2005-10-08 01:53:41 PDT
Created attachment 198903 [details] [diff] [review]
Change to Download All

This patch changes the default replication function from Change Log to Download
All. Whilst this isn't the ideal fix, Dan & I want to try and get at least
something working wrt replication for Thunderbird 1.5/SeaMonkey 1.0.

I have raised bug 311632 to cover changing back to the "Change Log" protocol
once we get this fix in - I couldn't see an easy way to fix that protocol.

The patch also fixes a crash in nsAbLDAPReplicationData by passing a nsCOMPtr
wrapped card rather than just a pointer to one.
Comment 8 Mark Banner (:standard8) 2005-10-08 02:03:20 PDT
Comment on attachment 198903 [details] [diff] [review]
Change to Download All

Sorry, I've just noticed a couple of problems with this patch - new one coming
up.
Comment 9 Mark Banner (:standard8) 2005-10-08 02:27:23 PDT
Created attachment 198906 [details] [diff] [review]
Patch v2

This is the revised patch. We now don't use nsAbLDAPCard as there isn't much
point as all we do with it is copy it to a mdb card. The
SetCardPropertiesFromLDAPMessage function takes a nsIAbCard so we are ok there
as well.
Comment 10 Mark Banner (:standard8) 2005-10-08 02:29:23 PDT
*** Bug 270156 has been marked as a duplicate of this bug. ***
Comment 11 Dan Mosedale (:dmose) 2005-10-12 13:07:33 PDT
Comment on attachment 198906 [details] [diff] [review]
Patch v2

> 
>Index: mailnews/addrbook/src/nsAbLDAPReplicationData.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/mailnews/addrbook/src/nsAbLDAPReplicationData.cpp,v
>retrieving revision 1.13
>diff -u -r1.13 nsAbLDAPReplicationData.cpp
>--- mailnews/addrbook/src/nsAbLDAPReplicationData.cpp	26 Jul 2005 15:27:17 -0000	1.13
>+++ mailnews/addrbook/src/nsAbLDAPReplicationData.cpp	8 Oct 2005 09:24:06 -0000
>@@ -44,8 +44,7 @@
> #include "nsIAddrBookSession.h"
> #include "nsAbBaseCID.h"
> #include "nsAbUtils.h"
>-#include "nsAbMDBCard.h"
>-#include "nsAbLDAPCard.h"
>+#include "nsIAbMDBCard.h"
> #include "nsAbLDAPReplicationQuery.h"
> #include "nsProxiedService.h"
> #include "nsCPasswordManager.h"
>@@ -334,9 +333,24 @@
>     if(!mReplicationDB || !mDBOpen) 
>         return NS_ERROR_FAILURE;
> 
>-    nsAbLDAPCard card;
>+  nsresult rv = NS_OK;
> 
>-    nsresult rv = mAttrMap->SetCardPropertiesFromLDAPMessage(aMessage, &card);
>+  // Although we would may naturally create an nsIAbLDAPCard here, we don't

Don't need the "may" here, i think.

r=dmose
Comment 12 Mark Banner (:standard8) 2005-10-12 13:27:09 PDT
Comment on attachment 198906 [details] [diff] [review]
Patch v2

This patch fixes ldap replication/download (in address book) for offline use.
It also fixes a crash in that code that was due to not handling allocation
correctly.

At the moment ldap download/replication doesn't work at all, repeatedly
prompting the user for a username/password and never getting anywhere.

The fix is partially masking the problem in that the address book will download
all data. We'll restore change log download capability for future releases.
Comment 13 Mark Banner (:standard8) 2005-10-13 09:14:08 PDT
Comment on attachment 198906 [details] [diff] [review]
Patch v2

Patch checked into trunk. I'm leaving this bug open until we get a resolution
on approval request.
Comment 14 Mark Banner (:standard8) 2005-10-13 11:44:38 PDT
Fix checked in to trunk and branch.
Comment 15 Mark Banner (:standard8) 2005-10-26 05:04:42 PDT
*** Bug 284602 has been marked as a duplicate of this bug. ***
Comment 16 Fletcher 2005-11-06 19:37:55 PST
(In reply to comment #14)
> Fix checked in to trunk and branch.
> 

Are you sure this is fixed?  I can't seem to get 1.5 RC1 or the nightly build from Friday to work properly.  More specifically, it doesn't use the data I entered to bind to the server.  I guess if I make my server give out data without authenticating, it will work, but that isn't going to happen.  Here's part of my log file, note the BIND dn part:

Nov  6 19:23:29 thread slapd[31559]: conn=24 fd=10 ACCEPT from IP=26.71.161.84:64977 (IP=0.0.0.0:389) 
Nov  6 19:23:29 thread slapd[31559]: conn=24 op=0 BIND dn="" method=128 
Nov  6 19:23:29 thread slapd[31559]: conn=24 op=0 RESULT tag=97 err=0 text= 
Nov  6 19:23:29 thread slapd[31559]: conn=24 op=1 SRCH base="ou=staff,dc=example,dc=com" scope=1 deref=0 filter="(objectClass=*)" 
Nov  6 19:23:29 thread slapd[31559]: conn=24 op=1 SEARCH RESULT tag=101 err=0 nentries=0 text= 
Nov  6 19:23:29 thread slapd[31559]: conn=24 op=2 UNBIND 
Nov  6 19:23:29 thread slapd[31559]: conn=24 fd=10 closed 

It is certainly better than before, since it at least skips that business about trying to look up a user account from a 'mail=' filter.  I'm not confident enough in my LDAP knowledge to reopen the bug, but if someone could look at before the 1.5 release this I'd appreciate it.
Comment 17 Mark Banner (:standard8) 2005-11-07 14:23:52 PST
(In reply to comment #16)
> (In reply to comment #14)
> > Fix checked in to trunk and branch.
> > 
> 
> Are you sure this is fixed?  I can't seem to get 1.5 RC1 or the nightly build
> from Friday to work properly.  More specifically, it doesn't use the data I
> entered to bind to the server. 

This WFM on a current thunderbird trunk build. I've verified the fix has gone into the branch, but I haven't been able to test a branch build yet. Are your normal ldap lookups working correctly?
Comment 18 Fletcher 2005-11-08 13:23:56 PST
(In reply to comment #17)
> This WFM on a current thunderbird trunk build. I've verified the fix has gone
> into the branch, but I haven't been able to test a branch build yet. Are your
> normal ldap lookups working correctly?

I've run some new tests to be sure and current.

Using 1.0.7 on Windows, I can get address completion working correctly from the directory.  Of course, the download feature is pooched on 1.0.x, so that isn't worth discussing.

Using 1.6a1 (20051108) on Mac OS X, I can get address completion working correctly as well, with slapd log output like so:

Nov  8 12:57:38 thread slapd[28805]: conn=4 fd=10 ACCEPT from IP=10.187.200.86:49533 (IP=0.0.0.0:389) 
Nov  8 12:57:38 thread slapd[28805]: conn=4 op=0 BIND dn="cn=Fletcher,ou=staff,dc=example,dc=com" method=128 
Nov  8 12:57:38 thread slapd[28805]: conn=4 op=0 BIND dn="cn=Fletcher,ou=staff,dc=example,dc=com" mech=SIMPLE ssf=0 
Nov  8 12:57:38 thread slapd[28805]: conn=4 op=0 RESULT tag=97 err=0 text= 
Nov  8 12:57:38 thread slapd[28805]: conn=4 op=1 SRCH base="ou=staff,dc=example,dc=com" scope=1 deref=0 filter="(|(cn=joe*)(mail=joe*)(sn=joe*))" 
Nov  8 12:57:38 thread slapd[28805]: conn=4 op=1 SRCH attr=cn mail 
Nov  8 12:57:38 thread slapd[28805]: conn=4 op=1 SEARCH RESULT tag=101 err=0 nentries=1 text= 
Nov  8 12:57:58 thread slapd[28805]: conn=4 op=2 UNBIND 
Nov  8 12:57:58 thread slapd[28805]: conn=4 fd=10 closed 

Beautiful stuff.  I love it.  If I search for a contact in the Compose window's Contacts sidebar, I can get all the other contact info for one person as well through the Properties option.

In Address Book, I see two problems.  The lack of a BIND dn means I can only get decent results in the log file if I enable 'anonymous read' access for my contact list.  When I do this, I can get identical log file output with Thunderbird and ldapsearch.  Unfortunately, even though Thunderbird appears to connect properly, it doesn't actually populate the address book with data it should have downloaded.  Here's what a 'successful' Thunderbird download looks like:

Nov  8 13:18:27 thread slapd[28805]: conn=8 fd=10 ACCEPT from IP=10.187.200.86:49537 (IP=0.0.0.0:389) 
Nov  8 13:18:27 thread slapd[28805]: conn=8 op=0 BIND dn="" method=128 
Nov  8 13:18:27 thread slapd[28805]: conn=8 op=0 RESULT tag=97 err=0 text= 
Nov  8 13:18:27 thread slapd[28805]: conn=8 op=1 SRCH base="ou=staff,dc=example,dc=com" scope=1 deref=0 filter="(objectClass=*)" 
Nov  8 13:18:27 thread slapd[28805]: conn=8 op=1 SEARCH RESULT tag=101 err=0 nentries=23 text= 
Nov  8 13:18:29 thread slapd[28805]: conn=8 op=2 UNBIND 
Nov  8 13:18:29 thread slapd[28805]: conn=8 fd=10 closed 

Note the nentries=23, which nicely matches the number of people in the contact list.  As I said, the log file output from ldapsearch is identical, although ldapsearch actually shows the corresponding contact records.
Comment 19 Mark Banner (:standard8) 2005-11-12 04:28:21 PST
I've now moved the autheticated connection case into bug 316170 as I've been able to confirm it myself.

Note You need to log in before you can comment on or make changes to this bug.