146411 - 3rd party LDAP Query may result in an invalid filter.

Status: RESOLVED FIXED

(SeaMonkey :: MailNews: Address Book & Contacts, defect)

Description

[John Marmion]

The Mozilla Address Book does not allow a blank search condition when attempting
to execute a query against an LDAP Directory Server. This is disallowed in the
StartSearch() method. But 3rd party products like OpenOffice.org integrate with
Mozilla using the DoQuery() method. Attempts to run a query using the
DoQuery()against a Mozilla LDAP  Directory requesting all the Mozilla attributes
will result in an invalid filter i.e. "(|)" getting generated. 

Investigating this behaviour further, it is not sufficient to simply generate a
blank filter as this will default to the filter (objectclass=*) thereby
returning all entries from the LDAP Server regardless. Thus I have suggested
that we default the filter in this case to (mail=*) to return entries which are
relevant to the Address Book.

Comment 1

[John Marmion]

Attachment: patch to fix this

Comment 2

[John Marmion]

Attachment: Update the patch against the head

The patch is now up-to-date and I will request an "r" for this patch.

Comment 3

[John Marmion]

Taken ownership of this bug as I own the patch.

Comment 4

[John Marmion]

Attachment: Keep patch up-to-date and use the required 'cn' as the default attribute

Testing this some more and i believe that 'cn' is a more appropriate default
attribute than 'mail'. 'cn' is a required attribute as opposed to 'mail' which
is a "may have". Also keep patch up-to-date with head.

Comment 5

[John Marmion]

Attachment: Revised version of the patch

I believe that this should be fixed here. I vacillated on the correct default
ldap attribute to use where no restriction/condition is applied to an ldap
query from a 3rdparty source. But I now believe that "mail" is the most
appropriate default where we are interested in returning address book entries.
Using "cn" returns too many entries.Ideally we might use
"objectclass=inetorgperson" but that would assume that we had agreement on a
schema.

Comment 6

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 102008 [details] [diff] [review]
Revised version of the patch

The comments you added are very helpful; thanks.

>+    if(filter.Equals(""))

How about using .IsEmpty() instead of .Equals("")?

>+    {
>+        filter += NS_LITERAL_CSTRING("(|(mail=*))");
>+    }

I'd like to vote for using objectclass=inetOrgPerson.  Although our schema is
not yet finalized, we will almost certainly do it with an auxilliary class
rather than a completely separate class (so we won't be using something instead
of inetOrgPerson).  And I think we'll almost certainly use inetOrgPerson as the
central part of what mozilla addressbook supports (as we do today)

Looks good otherwise.

Comment 7

[John Marmion]

Attachment: update patch As Dan requested

Thanks for the feedback Dan. I have made those two very valid changes you
requested.

Comment 8

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 102154 [details] [diff] [review]
update patch As Dan requested

One last question: what's the point of the "|" clause?	Wouldn't
"(objectclass=inetOrgPerson)"  be sufficient?

With or without that change, r=dmose.

Comment 9

[John Marmion]

Attachment: Remove superfluous (|)

Comment 10

[(not reading, please use [e-mail] instead)]

Comment on attachment 102318 [details] [diff] [review]
Remove superfluous (|)

sr=sspitzer

assuming you check that AB quick search and AB advanced search (and the other
consumers of this code) work fine.

(local and LDAP)

Comment 11

[Asa Dotzler [:asa]]

Comment on attachment 102318 [details] [diff] [review]
Remove superfluous (|)

a=asa for checkin to 1.2beta (on behalf of drivers)

Comment 12

[John Marmion]

Marking this bug as fixed. Yulian, as the reporter of this bug, I can verify
that this is now fixed. This can only really be verified by using a 3rdparty
product like OpenOffice.org (OOo) which integrates with Mozilla's LDAP Address
Book. This fix will appear in the next release of OOo i.e. OOo 1.0.2. For
Mozilla, there should be no change in the existing functionality of the LDAP
Simple or Advanced Search.