Wrong error messages for some operations

• Key: DIRSERVER-379
• Type: Bug
• Status: Closed
• Resolution: Fixed
• Priority: Minor
• Assignee: Alex Karasulu
• Reporter: Stefan Zoerner

Description

If a client tries to remove an entry, and the deletion fails because the entry is not a leaf (error code 66), the LDAP message is wrong.
Status code and Exception type within the server are correct (as detected by the corresponding test case). But the message text itself contains the words "failed to add entry". This information is sent to the client, which may display it, e.g.
    * JNDI includes it during creation of appropriate Exception (com.sun.jndi.ldap.LdapCtx.mapErrorCode(66, "failed to add entry ...").
    * Command line tools like ldapmodify display it on the console.

Example: If the following LDIF is applied to ApacheDS

---

# create entry
dn: ou=noLeaf,ou=system
changetype: add
ou: noLeaf
objectclass: top
objectclass: organizationalUnit

# create entry below the first
dn: ou=leaf,ou=noLeaf,ou=system
changetype: add
ou: leaf
objectclass: top
objectclass: organizationalUnit

# delete subtree
dn: ou=noLeaf,ou=system
changetype: delete

---

it goes

$ ldapmodify -h magritte -p 10389 -D uid=admin,ou=system -w **** -f deleteSubtree.ldif

adding new entry ou=noLeaf,ou=system

adding new entry ou=leaf,ou=noLeaf,ou=system

deleting entry ou=noLeaf,ou=system
ldap_delete: Operation not allowed on nonleaf
ldap_delete: additional info: failed to add entry ou=noLeaf,ou=system:
... (very long Stack trace deleted) ...
$

I modified the error message within class org.apache.ldap.server.protocol.DeleteHandler to "failed to delete entry ..." (attached), although I would prefer a more detailed message (like "Not Allowed On Non-leaf", as Sun Directory does). But this is not possible at that general place without an ugly switch or so.

The same "defect" (probably a cut/copy/paste of error message from AddHandler) is present in CompareHandler, ModifyDnHandler, and ModifyHandler.

I would also suggest not to send the complete stack trace back to the client. I have removed it in the attached version of the DeleteHandler and had a better command line experience with ApacheDS (the stack traces should go in a server log only from my point of view). This applies to other handlers (e.g. Add) as well.

Comments

[Stefan Zoerner]

Here is the updated DeleteHandler class mentiones in the defect description.

[Alex Karasulu]

Thanks a bunch Stefan but next time please submit a diff instead of the file itself. These are easier to review and apply but hey this is better than most :).

I agree about the stack trace however that was for debuging purposes. I will use the log level to not show this on the client as well as print the stack trace out to the server log file. It's pretty helpful to get the trace on the client side when you're working on separate machines. However for production this should not be the case. Again using nlog4j should help us out considerably in terms of controlling this behavoir.

[Alex Karasulu]

If you look at the diff for committed revision 230698 you'll see that I modified your changes a bit. I still send the stack trace back but only when logging is in full debug mode. I really need to work on the logging in these modules and reviewing the messages they return. Perhaps you can lend a hand so we can do this together. You interested?

Here's the diff of my commits:

xhttp://svn.apache.org/viewcvs.cgi?rev=230698&view=rev

BTW I will build and upload the new jars for this and apacheds built with it.

I changed all wrong messages in the handler classes (or at least all wrong messages) in package org.apache.ldap.server.protocol. "svn diff > patch.txt" is attached.

Tests were made against an ApacheDS (theses changes applied) with Solaris command line tools with wrong arguments (add, delete, search).
I also added logging like Alex demonstrated it within the DeleteHandler. Probably not a perfect solution yet, although it is consistent within the package now.

[Stefan Zoerner]

I meant "(or at least all wrong messages I found)" ;-)
Btw. I also moved the newlines between error message and Stacktrace in the optional part (i.e.if the Stacktrace is included) for better command line tool and JNDI experience (if DEBUG is disabled).

[Alex Karasulu]

Committed these latest patch.txt changes on revision 230873. Here's the url:

http://svn.apache.org/viewcvs.cgi?rev=230873&view=rev

I have compiled and deployed these changes to the maven repository. Thanks for the patches!

[Stefan Zoerner]

With the application of the patch some time ago (thank you, Alex), the error message for the different actions (add, delete, ...) now reflect the correct operation. I have retested the behaviour. High time to close the issue.