90535 - default searchFilterTemplate misses people with a middle name in their sn

Status: VERIFIED FIXED

(MailNews Core :: LDAP Integration, defect, P3)

Description

[Dan Mosedale (:dmosedale, :dmose)]

In .directory, [e-mail] (Ken Lui) wrote:

> I've noticed that LDAP autocompletion works differently from
> Communicator 4.76 and Mozilla 0.9.1/0.9.2. In Communicator, if
> I enter "John Smith" and the person in the directory is "John E.
> Smith", it finds it no problem, but Mozilla doesn't find it
> unless I enter "John*Smith".

The problem here is that the default searchFilterTemplate doesn't deal correctly
with this; I think I misunderstood the equivalent code in 4.x slightly when I
was figuring out what default to use.  This should be pretty straightforward to
fix, I think, because the template format should be able to handle this
out-of-the-box.

Comment 1

[Dan Mosedale (:dmosedale, :dmose)]

Another interesting thing this brings up is that chars in the input string that
have special meaning in LDAP search filters are neither quoted nor stripped;
they probably ought to be, though one could conceivably argue that this is a
feature.  My gut feeling is that we probably want to quote them.

Comment 2

[Dan Mosedale (:dmosedale, :dmose)]

The rationale behind the default search filter currently in place:

(|(cn=%v*)(mail=%v*)(sn=%v*))

is documented in bug 85315.

My current theory is that maybe we want the following search filter as the default:

(|(cn=%v1*%v2-*)(mail=%v1*%v2-*)(sn=%v1*%v2-*))

Any thoughts?

Comment 3

[Dan Mosedale (:dmosedale, :dmose)]

OK, after some testing, that last proposal turns up way too many false hits. 
I'll have to play around some more.  

Additionally, the quoting thing is going to need to be a separate bug, in part
because the quoting for LDAP v2 and LDAP v3 search filters is different.

Comment 4

[Dan Mosedale (:dmosedale, :dmose)]

Adding bug finder to the CC.

Comment 5

[Dan Mosedale (:dmosedale, :dmose)]

Turns out the reason I was seeing lots of extraneous returns with the proposed
change was that there was bug in nsLDAPService::CountTokens().  After fixing
this, it appears to me to fix the problem, and seems (so far) like it's
generating reasonable results.

Comment 6

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: Patch, v1

Comment 7

[Dan Mosedale (:dmosedale, :dmose)]

Leif, can I get a review?

Comment 8

[Leif Hedstrom]

r=leif, looks good.

-- Leif

Comment 9

[Darin Fisher]

rs=darin

Comment 10

[Dan Mosedale (:dmosedale, :dmose)]

Fix checked in.

Comment 11

[yulian chang]

Verified with 2001080603 Windows trunck build.