Issue Details (XML | Word | Printable)

Key: DIRSERVER-191
Type: Bug Bug
Status: Closed Closed
Resolution: Fixed
Priority: Critical Critical
Assignee: Alex Karasulu
Reporter: Jacob S. Barrett
Votes: 0
Watchers: 1
Operations

If you were logged in you would be able to see more operations.
Directory ApacheDS

LdapName.getPrefix(int) does not return prefix.

Created: 19/Mar/05 03:49 AM   Updated: 21/Apr/07 11:07 AM
Return to search
Component/s: ldap
Affects Version/s: None
Fix Version/s: 1.5.0

Time Tracking:
Not Specified

File Attachments:
  Size
Text File Licensed for inclusion in ASF works LdapName.patch 2005-03-30 07:00 AM Jacob S. Barrett 2 kB
Text File Licensed for inclusion in ASF works NamespaceTools.patch 2005-03-30 08:09 AM Jacob S. Barrett 0.6 kB
Java Source File Licensed for inclusion in ASF works NamespaceToolsTest.java 2005-03-30 08:09 AM Jacob S. Barrett 0.7 kB
Java Source File Licensed for inclusion in ASF works NameTest.java 2005-03-30 07:00 AM Jacob S. Barrett 7 kB
Environment: NA

Resolution Date: 25/Jun/06 07:52 PM


 Description  « Hide
The JavaDoc for javax.naming.Name.getPrefix(int) [1] states that it returns "a name consisting of the components at indexes in the range [0,posn)." The implementation in org.apache.ldap.common.name.LdapName returns [size() - posn, size()). This is a suffix starting from the right like the getSuffix(int) which is a suffix from from the left. The correct implementation should return the prefix from the left to the specified position. Attached is the appropiate patch for LdapName.java and LdapNameTest.java.

References:
1) http://java.sun.com/j2se/1.4.2/docs/api/javax/naming/Name.html#getPrefix(int)

Patch:
Index: shared/ldap/trunk/common/src/test/org/apache/ldap/common/name/LdapNameTest.java
===================================================================
--- shared/ldap/trunk/common/src/test/org/apache/ldap/common/name/LdapNameTest.java (revision 158112)
+++ shared/ldap/trunk/common/src/test/org/apache/ldap/common/name/LdapNameTest.java (working copy)
@@ -545,10 +545,10 @@
         Name l_name =
             m_parser.parse( "cn=HomeDir,cn=John,ou=Marketing,ou=East" ) ;
         assertEquals( "", l_name.getPrefix( 0 ).toString() ) ;
- assertEquals( "ou=East", l_name.getPrefix( 1 ).toString() ) ;
- assertEquals( "ou=Marketing,ou=East",
+ assertEquals( "cn=HomeDir", l_name.getPrefix( 1 ).toString() ) ;
+ assertEquals( "cn=HomeDir,cn=John",
             l_name.getPrefix( 2 ).toString() ) ;
- assertEquals( "cn=John,ou=Marketing,ou=East",
+ assertEquals( "cn=HomeDir,cn=John,ou=Marketing",
             l_name.getPrefix( 3 ).toString() ) ;
         assertEquals( "cn=HomeDir,cn=John,ou=Marketing,ou=East",
             l_name.getPrefix( 4 ).toString() ) ;
Index: shared/ldap/trunk/common/src/java/org/apache/ldap/common/name/LdapName.java
===================================================================
--- shared/ldap/trunk/common/src/java/org/apache/ldap/common/name/LdapName.java (revision 158112)
+++ shared/ldap/trunk/common/src/java/org/apache/ldap/common/name/LdapName.java (working copy)
@@ -327,7 +327,7 @@
     public Name getPrefix( int a_posn )
     {
         ArrayList list = new ArrayList();
- list.addAll( m_list.subList( size() - a_posn, size() ) );
+ list.addAll( m_list.subList( 0, a_posn ) );
         return new LdapName( list ) ;
     }
 


 All   Comments   Work Log   Change History   Subversion Commits      Sort Order: Ascending order - Click to sort in descending order
Alex Karasulu added a comment - 19/Mar/05 04:23 AM
Let's put this into 0.9 release.

Alex Karasulu made changes - 19/Mar/05 04:23 AM
Field Original Value New Value
Fix Version/s 0.9.0 [ 11004 ]
Alex Karasulu added a comment - 30/Mar/05 02:07 AM
Jacob I could really use a patch file attachment and a JUnit test again. Also would be nice to show how a JDK Name implementation behaves in comparison so we can see what's happening in the test case. If you can get this to me soon I can cut the 0.9 release tarballs for all to test. Thanks.

Jacob S. Barrett added a comment - 30/Mar/05 07:00 AM
The patch fixes the backwards getSuffix() call and the attached file is JUnit that compare LdapName against the one in JDK 1.5. The JUnit must be compiled and executed with the rt.jar from JDK 1.5.

Jacob S. Barrett made changes - 30/Mar/05 07:00 AM
Attachment LdapName.patch [ 19415 ]
Attachment NameTest.java [ 19414 ]
Jacob S. Barrett added a comment - 30/Mar/05 08:09 AM
The patch fixes a bug introduced by fixing the getSuffix() in the previous patch. In addition there is a JUnit to test this patch.

Jacob S. Barrett made changes - 30/Mar/05 08:09 AM
Attachment NamespaceTools.patch [ 19417 ]
Attachment NamespaceToolsTest.java [ 19418 ]
Alex Karasulu added a comment - 30/Mar/05 08:12 AM
The bug with getSuffix is way serious and it drills down deep. We've worked around the bug it seems. Regardless 4 major classes depend on getSuffix working the wrong way:

org.apache.ldap.server.exceptionorg.apache.ldap.server.exception.ExceptionService
org.apache.ldap.server.jndi.ServerContext
org.org.apache.ldap.server.db.jdbm.JdbmDatabase
org.apache.ldap.seorg.apache.ldap.server.operational.OperationalAttributeService

This is going to require some serious testing to make sure we are not totally messing up the server. Instead of taking this on before the next feature release I will leave it for 0.9.1 and handle it properly then. Currently it is not producing a negative effect in the server so it should be ok.

Alex Karasulu made changes - 30/Mar/05 08:12 AM
Fix Version/s 0.9.0 [ 11004 ]
Affects Version/s 0.9.1 [ 11126 ]
Fix Version/s 0.9.1 [ 11126 ]
Alex Karasulu added a comment - 30/Mar/05 08:16 AM
Increasing criticality - making sure I'm going to take care of this right after the 0.9 release.

Alex Karasulu made changes - 30/Mar/05 08:16 AM
Priority Major [ 3 ] Critical [ 2 ]
Alex Karasulu added a comment - 16/Jun/05 03:05 PM
Jacob points out DIRLDAP-112 supercedes this on.

Alex Karasulu made changes - 16/Jun/05 03:05 PM
Fix Version/s 0.9.1 [ 11126 ]
Resolution Duplicate [ 3 ]
Status Open [ 1 ] Resolved [ 5 ]
Trustin Lee added a comment - 26/Sep/05 02:56 PM
I guess this issue is not resolved right now? I'm reopening...

Trustin Lee made changes - 26/Sep/05 02:56 PM
Resolution Duplicate [ 3 ]
Status Resolved [ 5 ] Reopened [ 4 ]
Alex Karasulu made changes - 07/Feb/06 02:40 PM
Affects Version/s 0.8.0 [ 11000 ]
Affects Version/s 0.9.0 [ 11004 ]
Project Directory LDAP [ 10514 ] ApacheDS [ 12310260 ]
Component/s ldap [ 12310715 ]
Component/s Common [ 11085 ]
Key DIRLDAP-37 DIRSERVER-191
Emmanuel Lecharny made changes - 12/Feb/06 06:57 PM
Description The JavaDoc for javax.naming.Name.getPrefix(int) [1] states that it returns "a name consisting of the components at indexes in the range [0,posn)." The implementation in org.apache.ldap.common.name.LdapName returns [size() - posn, size()). This is a suffix starting from the right like the getSuffix(int) which is a suffix from from the left. The correct implementation should return the prefix from the left to the specified position. Attached is the appropiate patch for LdapName.java and LdapNameTest.java.

References:
1) http://java.sun.com/j2se/1.4.2/docs/api/javax/naming/Name.html#getPrefix(int)

Patch:
Index: shared/ldap/trunk/common/src/test/org/apache/ldap/common/name/LdapNameTest.java
===================================================================
--- shared/ldap/trunk/common/src/test/org/apache/ldap/common/name/LdapNameTest.java (revision 158112)
+++ shared/ldap/trunk/common/src/test/org/apache/ldap/common/name/LdapNameTest.java (working copy)
@@ -545,10 +545,10 @@
         Name l_name =
             m_parser.parse( "cn=HomeDir,cn=John,ou=Marketing,ou=East" ) ;
         assertEquals( "", l_name.getPrefix( 0 ).toString() ) ;
- assertEquals( "ou=East", l_name.getPrefix( 1 ).toString() ) ;
- assertEquals( "ou=Marketing,ou=East",
+ assertEquals( "cn=HomeDir", l_name.getPrefix( 1 ).toString() ) ;
+ assertEquals( "cn=HomeDir,cn=John",
             l_name.getPrefix( 2 ).toString() ) ;
- assertEquals( "cn=John,ou=Marketing,ou=East",
+ assertEquals( "cn=HomeDir,cn=John,ou=Marketing",
             l_name.getPrefix( 3 ).toString() ) ;
         assertEquals( "cn=HomeDir,cn=John,ou=Marketing,ou=East",
             l_name.getPrefix( 4 ).toString() ) ;
Index: shared/ldap/trunk/common/src/java/org/apache/ldap/common/name/LdapName.java
===================================================================
--- shared/ldap/trunk/common/src/java/org/apache/ldap/common/name/LdapName.java (revision 158112)
+++ shared/ldap/trunk/common/src/java/org/apache/ldap/common/name/LdapName.java (working copy)
@@ -327,7 +327,7 @@
     public Name getPrefix( int a_posn )
     {
         ArrayList list = new ArrayList();
- list.addAll( m_list.subList( size() - a_posn, size() ) );
+ list.addAll( m_list.subList( 0, a_posn ) );
         return new LdapName( list ) ;
     }
 
The JavaDoc for javax.naming.Name.getPrefix(int) [1] states that it returns "a name consisting of the components at indexes in the range [0,posn)." The implementation in org.apache.ldap.common.name.LdapName returns [size() - posn, size()). This is a suffix starting from the right like the getSuffix(int) which is a suffix from from the left. The correct implementation should return the prefix from the left to the specified position. Attached is the appropiate patch for LdapName.java and LdapNameTest.java.

References:
1) http://java.sun.com/j2se/1.4.2/docs/api/javax/naming/Name.html#getPrefix(int)

Patch:
Index: shared/ldap/trunk/common/src/test/org/apache/ldap/common/name/LdapNameTest.java
===================================================================
--- shared/ldap/trunk/common/src/test/org/apache/ldap/common/name/LdapNameTest.java (revision 158112)
+++ shared/ldap/trunk/common/src/test/org/apache/ldap/common/name/LdapNameTest.java (working copy)
@@ -545,10 +545,10 @@
         Name l_name =
             m_parser.parse( "cn=HomeDir,cn=John,ou=Marketing,ou=East" ) ;
         assertEquals( "", l_name.getPrefix( 0 ).toString() ) ;
- assertEquals( "ou=East", l_name.getPrefix( 1 ).toString() ) ;
- assertEquals( "ou=Marketing,ou=East",
+ assertEquals( "cn=HomeDir", l_name.getPrefix( 1 ).toString() ) ;
+ assertEquals( "cn=HomeDir,cn=John",
             l_name.getPrefix( 2 ).toString() ) ;
- assertEquals( "cn=John,ou=Marketing,ou=East",
+ assertEquals( "cn=HomeDir,cn=John,ou=Marketing",
             l_name.getPrefix( 3 ).toString() ) ;
         assertEquals( "cn=HomeDir,cn=John,ou=Marketing,ou=East",
             l_name.getPrefix( 4 ).toString() ) ;
Index: shared/ldap/trunk/common/src/java/org/apache/ldap/common/name/LdapName.java
===================================================================
--- shared/ldap/trunk/common/src/java/org/apache/ldap/common/name/LdapName.java (revision 158112)
+++ shared/ldap/trunk/common/src/java/org/apache/ldap/common/name/LdapName.java (working copy)
@@ -327,7 +327,7 @@
     public Name getPrefix( int a_posn )
     {
         ArrayList list = new ArrayList();
- list.addAll( m_list.subList( size() - a_posn, size() ) );
+ list.addAll( m_list.subList( 0, a_posn ) );
         return new LdapName( list ) ;
     }
 
Fix Version/s 1.1.0 [ 12310790 ]
Emmanuel Lecharny added a comment - 25/Jun/06 07:52 PM
This issue has been solved a long time ago, but when?

Tests have been added into LdapDNTest class.

Emmanuel Lecharny made changes - 25/Jun/06 07:52 PM
Resolution Fixed [ 1 ]
Status Reopened [ 4 ] Resolved [ 5 ]
Repository Revision Date User Message
ASF #418529 Sat Jul 01 23:48:03 UTC 2006 elecharny Added tests for DIRSERVER-191
Files Changed
MODIFY /directory/branches/shared/optimization/ldap/src/test/java/org/apache/directory/shared/ldap/name/LdapDNTest.java

Emmanuel Lecharny added a comment - 21/Apr/07 11:07 AM
Closing all issues created in 2005 and before which are marked resolved

Emmanuel Lecharny made changes - 21/Apr/07 11:07 AM
Status Resolved [ 5 ] Closed [ 6 ]