james-server-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bernd Fondermann" <bernd.fonderm...@googlemail.com>
Subject Re: [PROPOSAL] move ignoreCase configuration to the UsersRepository and remove containsIgnoreCase
Date Mon, 18 Sep 2006 08:08:57 GMT
On 9/17/06, Stefano Bagnara <apache@bago.org> wrote:
> I've made a code review on the usage of case sensitivity inside James
> and I found a lot of inconsistencies.
>
> We currenlty have an <ignoreCase> option inside the James block and have
> 2 methods in the UsersRepository interface: contains and
> containsCaseInsensitive.
>
> This would be ok if every piece of code using the UsersRepository to
> check for users was checking ignoreCase first and then call one or the
> other method, but this does not happen in out code.
>
> Here a few inconsistencies:
> 1) our default "addUser" method for AbstractUsersRepository uses
> containesIgnoreCase before adding an user: this mean you can't add 2
> users with the same name but different letter-cases even if you have
> ignoreCase=false.
>
> 2) fetchmail checks for localusers always using the caseInsensitive
> search, so it could do the wrong things when ignoreCase is false.
>
> 3) few places use the MailetContext.isLocalEmail, implemented by James
> and following the ignoreCase directive, but most code use directly the
> userrepository.contains() that is case sensitive: you understand how
> many problems this could lead to. (as an example if you have ignorecase
> on and using remotemanager you try to add an user that is already
> present using the same lettercase remotemanager tell you that the user
> already exists, while if you use a different lettercase you receive a
> generic error).
>
> That said my proposal is:
>
> 1) Move the ignoreCase configuration to the UsersRepository
>
> 2) Remove the containsIgnoreCase from the UsersRepository interface (we
> don't need it anymore). Maybe we should keep this as deprecate as the
> first step and let it revert to contains(name.toLowerCase), so we keep a
> better backward compatibility.

if it is not called anywhere, please feel free remove it completely.

>
> 3) Remove/deprecate getUserByNameCaseInsensitive: we don't use this
> anywhere.

+1

> 4) Allow the administrator to add 2 users with the same name but
> different letter-case.

... and if UsersRepository is case-insensitive, the second is rejected?

>
> 5) Make sure that our current implementations switch to all lowercase
> names where ignoreCase is activated.

+1
what happens if this configuration is changed from caseSensitive to
ignoreCase for a non-empty user base?

>
> 6) We should also deprecate addUser(User user) and addUser(String
> username,Object attributes) as all of our code now use only
> addUser(String username, String password).

+1

  Bernd

---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org


Mime
View raw message