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

Already on GitHub? Sign in to your account

ModifyDN fixes #2

Merged
merged 2 commits into from Oct 25, 2012

Conversation

Projects
None yet
2 participants
Contributor

rporres commented Oct 25, 2012

I've added some code to handle the case in which newSuperior attribute doesn't come from decoded content. If it doesn't come we still need the base DN to move the rdn to otherwise we would move it to the root of the tree. This is better understood with an example:

$ldap->moddn('msisdn=34678001122,app=test', newrdn => 'msisdn=34999000111');

If there's no newsuperior attribute, what we are doing is renaming the object from

'msisdn=34678001122,app=test'

to

'msisdn=34999000111,app=test'

The code as it was, was moving it to a different place in the directory tree 'msisdn=34999000111'

As the code is working now, I've also patched the tests to make sure they are ok.

In any case, I feel that the code shouldn't be necessary. moddn operation in Net::LDAP always adds newSuperior attribute. Hence, I don't understand why I needed to add code to handle the case in which it is not defined...

rporres added some commits Oct 25, 2012

@rporres rporres ModifyDN was not working correctly as newSuperior attribute doesn't a…
…lways come from decoded moddn request.
1948854
@rporres rporres Removed part of the comments on ModifyDN. Net::LDAP encodes the messa…
…ge with newSuperior set to undef if we don't specify it in the options of moddn. Once encoded it doesn't get to the server in the message, so it's not a hack to find the base DN to which the newrdn is relative to since we cannot assume newSuperior is going to be in the moddn. This is the expected behaviour described in RFC 2251
0072673
Contributor

rporres commented Oct 25, 2012

I've taken a look to the Net::LDAP code on modn and discovered that in case newsuperior is not in the options it will get to the encoder as undef. The encoder seems then to remove it from the message that Net::LDAP sends to the server. The code then is necessary as newSuperior can come or not. This is consistent with the behaviour described in RFCs 2251 and 2849. I've removed part of the comments as it's not a hack anymore :)

@karpet karpet added a commit that referenced this pull request Oct 25, 2012

@karpet karpet Merge pull request #2 from rporres/master
ModifyDN fixes
fa363a6

@karpet karpet merged commit fa363a6 into karpet:master Oct 25, 2012

Owner

karpet commented Oct 25, 2012

Thanks. This makes perfect sense to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment