Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LDAP-46: DirContextAdapter.getModificatioItems() doesnt ignore case when collecting values to be removed and added. #89

Closed
spring-projects-issues opened this issue Mar 16, 2007 · 5 comments
Milestone

Comments

@spring-projects-issues
Copy link

Vikash Trivedi(Migrated from LDAP-46) said:

DirContextAdapter.getModicationItems() calls collectModifications() to collect the modification items. The section handling multi-valued attributes uses CollectionUtils to return a new list that will contain a substraction of the old values from the new values, but the substraction doesnt handle case insensitivity thus resulting in a list of items that require to be added which already exist in the Directory Server with a different case and resulting in an ATTRIBUTE_ALREADY_EXISIT error. See below:
else {
// Collect all modifications to attribute individually (this also
// covers additions to a previously non-existant attribute).
Collection oldValues = new LinkedList();
Collection newValues = new LinkedList();

```
collectAttributeValues(oldValues, currentAttribute);
collectAttributeValues(newValues, changedAttr);
Collection myModifications = new LinkedList();

Collection addedValues = CollectionUtils.subtract(newValues, oldValues); // WILL ALSO RETURN VALUES TO BE ADDED THAT ARE ALREADY IN LDAP IF CASE IS DIFFERENT. Collection removedValues = CollectionUtils.subtract(oldValues, newValues); // WILL ALSO RETURN VALUES TO BE ADDED THAT ARE ALREADY IN LDAP IF CASE IS DIFFERENT. collectModifications(DirContext.ADD_ATTRIBUTE, changedAttr, addedValues, myModifications); collectModifications(DirContext.REMOVE_ATTRIBUTE, changedAttr, removedValues, myModifications); if (myModifications.isEmpty()) { // This means that the attributes are not equal, but the // actual values are the same – thus the order must have // changed. This should result in a REPLACE_ATTRIBUTE operation. myModifications.add(new ModificationItem( DirContext.REPLACE_ATTRIBUTE, changedAttr)); }

```

@spring-projects-issues
Copy link
Author

Mattias Hellborg Arthursson said:

This issue should be addressed, but I wouldn’t want to do a hack just to make string comparisons be case-insensitive. First of all, while it is the most common case, not all Attributes are case-insensitive. Secondly, the same problem (or at least a similar one) applies to other Attributes as well. E.g. ‘uniqueMember’ represents a DN, but the same DN can be represented by different strings (i.e. with spaces etc.), so the same problem would apply there as well (if the Directory Server is grumpy).

We should address these types of problems in a generic and configurable way. Configuration won’t be too easy, the main problem being that DirContextAdapter instances are created all over the place (from DefaultDirObjectFactory as well as by users in their Dao methods). Bottom line, we’ll need to think more about this to get it done properly.

@spring-projects-issues
Copy link
Author

Mattias Hellborg Arthursson said:

Maybe we don’t need a complicated generic solution to this after all. An idea came up on the forum (http://forum.springframework.org/showthread.php?p=113359#post113359), which proposes to just make sure that the REMOVE_ATTRIBUTE items come first. While this will change Attribute value that really didn’t syntactically change, that’s actually not such a bad thing – after all what will be stored is the value in the case or format that the user actually specified.

@spring-projects-issues
Copy link
Author

Mattias Hellborg Arthursson said:

Did a fix for this – it is available in the nightly snapshots (as of tomorrow). Haven’t been able to verify it though, as the problem doesn’t present itself on ApacheDS (where we run our integration tests. I would appreciate it if you could verify it on your platform.

@spring-projects-issues
Copy link
Author

Mattias Hellborg Arthursson said:

Btw, the nightly snapshots are here:
http://static.springframework.org/downloads/nightly/spring-ldap.php

@spring-projects-issues
Copy link
Author

Mattias Hellborg Arthursson said:

Reworked getModificationItems() to use Attribute.contains() to determine attribute value equality. Also, removed values are now always returned first in the ModificationItem array, avoiding the ATT_ALREAY_EXIST problems even if the Attribute implementation should be unable to determine syntactically correct Attribute equality.

In the future we might look further into this and use getAttrubuteSyntaxDefinition/getAttributeDefinition, but for the time being this will have to do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants
@spring-projects-issues and others