You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by Stefano Bagnara <ap...@bago.org> on 2006/09/17 17:48:27 UTC
[PROPOSAL] move ignoreCase configuration to the UsersRepository and
remove containsIgnoreCase
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.
3) Remove/deprecate getUserByNameCaseInsensitive: we don't use this
anywhere.
4) Allow the administrator to add 2 users with the same name but
different letter-case.
5) Make sure that our current implementations switch to all lowercase
names where ignoreCase is activated.
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).
Does this make sense?
Stefano
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: [PROPOSAL] move ignoreCase configuration to the UsersRepository
and remove containsIgnoreCase
Posted by Vincenzo Gianferrari Pini <vi...@praxis.it>.
I think yes, it makes sense. This inconsistency probably never came out
as probably almost everybody ignores case to avoid problems.
Vincenzo
Stefano Bagnara 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.
>
> 3) Remove/deprecate getUserByNameCaseInsensitive: we don't use this
> anywhere.
>
> 4) Allow the administrator to add 2 users with the same name but
> different letter-case.
>
> 5) Make sure that our current implementations switch to all lowercase
> names where ignoreCase is activated.
>
> 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).
>
> Does this make sense?
> Stefano
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
> For additional commands, e-mail: server-dev-help@james.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: [PROPOSAL] move ignoreCase configuration to the UsersRepository and remove containsIgnoreCase
Posted by Bernd Fondermann <be...@googlemail.com>.
On 9/17/06, Stefano Bagnara <ap...@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
Re: [PROPOSAL] move ignoreCase configuration to the UsersRepository and remove containsIgnoreCase
Posted by Bernd Fondermann <be...@googlemail.com>.
On 9/18/06, Stefano Bagnara <ap...@bago.org> wrote:
> Bernd Fondermann wrote:
> >> 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?
>
> Right: it should reply "The user already exists" or something similar.
>
> >> 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?
>
> This would depend from the implementation: I guess that we should
> simply ignore non lower-case users and use only the uppercase.
do you mean to say: "... simply ignore non lower-case users and use
only the LOWERcase."?
we could also generate an error when inconsistencies are found.
> Otherwise the implementation should take care on how to disambiguate 2
> users with the same name but different capitalization.
>
> I think we can't do more than adding a comment over <ignoreCase> telling
> the user: "keep in mind that your existing userbase may become invalid
> if you change this".
ok, fine.
Bernd
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: [PROPOSAL] move ignoreCase configuration to the UsersRepository
and remove containsIgnoreCase
Posted by Stefano Bagnara <ap...@bago.org>.
Bernd Fondermann wrote:
>> 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?
Right: it should reply "The user already exists" or something similar.
>> 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?
This would depend from the implementation: I guess that we should
simply ignore non lower-case users and use only the uppercase.
Otherwise the implementation should take care on how to disambiguate 2
users with the same name but different capitalization.
I think we can't do more than adding a comment over <ignoreCase> telling
the user: "keep in mind that your existing userbase may become invalid
if you change this".
Stefano
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: [PROPOSAL] move ignoreCase configuration to the UsersRepository
and remove containsIgnoreCase
Posted by Norman Maurer <nm...@byteaction.de>.
+1
Norman
Vincenzo Gianferrari Pini schrieb:
> I think yes, it makes sense. This inconsistency probably never came
> out as probably almost everybody ignores case to avoid problems.
>
> Vincenzo
>
> Stefano Bagnara 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.
>>
>> 3) Remove/deprecate getUserByNameCaseInsensitive: we don't use this
>> anywhere.
>>
>> 4) Allow the administrator to add 2 users with the same name but
>> different letter-case.
>>
>> 5) Make sure that our current implementations switch to all lowercase
>> names where ignoreCase is activated.
>>
>> 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).
>>
>> Does this make sense?
>> Stefano
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
>> For additional commands, e-mail: server-dev-help@james.apache.org
>>
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
> For additional commands, e-mail: server-dev-help@james.apache.org
>
> !EXCUBATOR:1,450d76ab53071679276840!
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: [PROPOSAL] move ignoreCase configuration to the UsersRepository
and remove containsIgnoreCase
Posted by Norman Maurer <nm...@byteaction.de>.
Stefano Bagnara schrieb:
> Bernd Fondermann wrote:
>>> 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?
>
> Right: it should reply "The user already exists" or something similar.
See next comment.
>
>>> 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?
>
> This would depend from the implementation: I guess that we should
> simply ignore non lower-case users and use only the uppercase.
>
> Otherwise the implementation should take care on how to disambiguate 2
> users with the same name but different capitalization.
>
> I think we can't do more than adding a comment over <ignoreCase>
> telling the user: "keep in mind that your existing userbase may become
> invalid if you change this".
>
> Stefano
>
IMMO it make no sense to use case sensitive usernames at all.. Just my
thought.
bye
Norman
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org