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 (JIRA)" <se...@james.apache.org> on 2006/09/19 01:30:22 UTC
[jira] Created: (JAMES-622) move ignoreCase configuration to the
UsersRepository and remove containsIgnoreCase
move ignoreCase configuration to the UsersRepository and remove containsIgnoreCase
----------------------------------------------------------------------------------
Key: JAMES-622
URL: http://issues.apache.org/jira/browse/JAMES-622
Project: James
Issue Type: Improvement
Components: James Core, UsersStore & UsersRepository
Reporter: Stefano Bagnara
Assigned To: Stefano Bagnara
Fix For: Trunk
'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 (when using ignoreCase=false)
5) Make sure that our current implementations switch to all lowercase names where ignoreCase is activated: add a comment to let the user know of problems when having 2 users with different capitalization
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).
See http://mail-archives.apache.org/mod_mbox/james-server-dev/200609.mbox/%3c5127012.83921158508122583.JavaMail.root@elysia.void.it%3e for the complete (ongoing) thread.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
[jira] Resolved: (JAMES-622) move ignoreCase configuration to the
UsersRepository and remove containsIgnoreCase
Posted by "Stefano Bagnara (JIRA)" <se...@james.apache.org>.
[ https://issues.apache.org/jira/browse/JAMES-622?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Stefano Bagnara resolved JAMES-622.
-----------------------------------
Resolution: Fixed
The core thing has been committed months ago: i'm not willing to work the future details of this change, so I mark is as resolved. Details/further changes deserve their own issue.
> move ignoreCase configuration to the UsersRepository and remove containsIgnoreCase
> ----------------------------------------------------------------------------------
>
> Key: JAMES-622
> URL: https://issues.apache.org/jira/browse/JAMES-622
> Project: James
> Issue Type: Improvement
> Components: James Core, UsersStore & UsersRepository
> Reporter: Stefano Bagnara
> Assigned To: Stefano Bagnara
> Fix For: Next Major
>
>
> '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 (when using ignoreCase=false)
> 5) Make sure that our current implementations switch to all lowercase names where ignoreCase is activated: add a comment to let the user know of problems when having 2 users with different capitalization
> 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).
> See http://mail-archives.apache.org/mod_mbox/james-server-dev/200609.mbox/%3c5127012.83921158508122583.JavaMail.root@elysia.void.it%3e for the complete (ongoing) thread.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
[jira] Commented: (JAMES-622) move ignoreCase configuration to the
UsersRepository and remove containsIgnoreCase
Posted by "Stefano Bagnara (JIRA)" <se...@james.apache.org>.
[ http://issues.apache.org/jira/browse/JAMES-622?page=comments#action_12443835 ]
Stefano Bagnara commented on JAMES-622:
---------------------------------------
a) moved the AbstractUsersRepository methods to AbstractJdbcUsersRepository: 2 abstract classes and the second one was the only implementation of the first.
b) introduced a new AbstractUsersRepository that is extended by the file implementation, too, and moved there the common code: getMappings()/setIgnoreCase/setEnableForwarding/setEnableAliases.
c) created a new interface "JamesUsersRepository" that extends UsersRepository and add VirtualUserTable and ignorecase/alias/forwarding methods (this is needed only because Phoenix create proxies over blocks and only interfaces are visibile from other components, and I need to access this methods to keep backward compatibility in <James> block configuraiton.
d) removed specific handling of ignoreCase/alias/forwarding from James.java that now does not publish this attributes in the context anymore and does not use it. It simply check wether a "usernames" config is present and if present try to cast the usersrepository to JamesUsersRepository and update its configuration.
e) refactored LocalDelivery/UsersRepositoryAliasingForwardingMailet to ignore the context parameters (no longer present) and to make full use of the new VirtualUserTable services (if provided by the UsersRepository service found).
f) Added a LocalJamesUsersRepository that act as a wrapper to LocalUsers but expose the extended JamesUsersRepository service instead of the simple UsersRepository service (needed to provide backward compatibility).
> move ignoreCase configuration to the UsersRepository and remove containsIgnoreCase
> ----------------------------------------------------------------------------------
>
> Key: JAMES-622
> URL: http://issues.apache.org/jira/browse/JAMES-622
> Project: James
> Issue Type: Improvement
> Components: James Core, UsersStore & UsersRepository
> Reporter: Stefano Bagnara
> Assigned To: Stefano Bagnara
> Fix For: Trunk
>
>
> '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 (when using ignoreCase=false)
> 5) Make sure that our current implementations switch to all lowercase names where ignoreCase is activated: add a comment to let the user know of problems when having 2 users with different capitalization
> 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).
> See http://mail-archives.apache.org/mod_mbox/james-server-dev/200609.mbox/%3c5127012.83921158508122583.JavaMail.root@elysia.void.it%3e for the complete (ongoing) thread.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org