LdapName breaks static parser after repeated use.

• Key: DIRSERVER-584
• Type: Bug
• Status: Closed
• Resolution: Fixed
• Priority: Blocker
• Assignee: Unassigned
• Reporter: Alex Karasulu
• Component/s: ldap
• Affects Version/s: 1.0-RC1
• Fix Version/s: 1.0-RC1

Description

I'm thinking some of the recent changes on the name parser may have introduced new problems.

These are really weird problems. They happen only when running regression tests in maven within the server-unit module. Yeah crazy. Basically to reproduce run "mvn -Dregression test" inside the apacheds/server-main module. These are the tests that fail:

$ mvn -Dregressions test
[INFO] ----------------------------------------------------------------------------
[INFO] Building ApacheDS Server Unit
....
T E S T S
-------------------------------------------------------
....
[surefire] Running org.apache.directory.server.AddObjectClassesToEntryTest
[surefire] Tests run: 3, Failures: 0, Errors: 3, Time elapsed: 0.113 sec <<<<<<<< FAILURE !!
[surefire] Running org.apache.directory.server.MiscTest
[surefire] Tests run: 7, Failures: 0, Errors: 7, Time elapsed: 0.177 sec <<<<<<<< FAILURE !!
[surefire] Running org.apache.directory.server.MatchingRuleCompareTest
[surefire] Tests run: 3, Failures: 0, Errors: 3, Time elapsed: 0.08 sec <<<<<<<< FAILURE !!

Now when these tests run inside eclipse there is no problem. Here's what teh surefire report
looks like for these failures. They are all pretty much the same trace:
========================================================================================================
o.a.d.shared.ldap.exception.LdapConfigurationException: Failed to normalize the suffix: ou=system
[Root exception is o.a.d.shared.ldap.exception.LdapInvalidNameException: Parser failure on name:
       ou=system
Antlr exception trace:
line 1:1: unexpected token: ou
       at o.a.d.shared.ldap.name.antlrNameParser.attributeTypeAndValue(antlrNameParser.java:192)
       at o.a.d.shared.ldap.name.antlrNameParser.nameComponent(antlrNameParser.java:120)
       at o.a.d.shared.ldap.name.antlrNameParser.name(antlrNameParser.java:69)
       at o.a.d.shared.ldap.name.DnParser.parse(DnParser.java:208)
       at o.a.d.shared.ldap.name.LdapName.<init>(LdapName.java:115)
       at o.a.d.server.core.configuration.DirectoryPartitionConfiguration.setSuffix(DirectoryPartitionConfiguration.java:203)
========================================================================================================

The suspect code is at o.a.d.server.core.configuration.DirectoryPartitionConfiguration.setSuffix(DirectoryPartitionConfiguration.java:203:

   /**
    * Sets the suffix of the {@link DirectoryPartition}.
    */
   protected void setSuffix( String suffix ) throws NamingException
   {
       suffix = suffix.trim();
       try
202: {
203: new LdapName( suffix ); <<<<<<<<================== HERE!!!
204: }
       catch ( NamingException e )
       {
           throw new LdapConfigurationException( "Failed to normalize the suffix: " + suffix, e );
       }
       this.suffix = suffix;
   }

This is crazy though because how the heck is the parser failing on "ou=system" and succeeding on all test cases I cannot understand. I think I'm loosing my mind on this one this is why I need a second opinion. Please advise.

Thanks,
Alex

Comments

[Alex Karasulu]

Ok this is part II:

I have found a way to reproduce the error in eclipse. Looks like running org.apache.directory.server.jndi.ServerContextFactoryTest before running org.apache.directory.server.AddObjectClassesToEntryTest makes the parser fail. This parser is a reused static parser by the LdapName class. I think something is going stale in the parser after the first tests are run.
Try deleting the other tests minus these two in server-main module. Then run these two tests together in elcipse using the run option for all tests in the project. Then you can reproduce the error within eclipse.

I am debugging now and for some reason the parser starts to see the first token "ou" in the case of the "ou=system" string as a WS token of type = 16. This is a whitespace token. I think deep down the lexer is messed up somewhere. I'll try to isolate what is making the lexer freak.

Alex

[Alex Karasulu]

Commenting out this test in org.apache.directory.server.jndi.ServerContextFactoryTest fixes the other test cases it seems. All the test failures go away. Now look at the string used for setting the suffix:
    
// public void testBadPartition() throws Exception
// {
// MutableDirectoryPartitionConfiguration pcfg;
//
// // Add partition 'test=testing'
// pcfg = new MutableDirectoryPartitionConfiguration();
// pcfg.setName( "testing" );
//
// try
// {
// pcfg.setSuffix( "ou=test=testing" );
// }
// catch ( LdapConfigurationException ce )
// {
// assertTrue( true );
// return;
// }
//
// fail();
// }

Here it is:
===========

pcfg.setSuffix( "ou=test=testing" );

I think this new bug is due to the recent corrections made with https://issues.apache.org/jira/browse/DIRSERVER-578. I think it shows us that the parser cannot correctly recover it's state under certain situations. I think DIRSERVER-579 did the correct tthing to fix the problem so don't get me wrong. It just uncovered another problem :( by fixing the first one.

[Alex Karasulu]

Ersin we might need your help on this one because it has something to do with resetting these reusable parsers. Basically an abrubt failure in the parser is not correcting the parser's state for reuse. I think the parser is getting stuck in a particular mode because of the exception flow bypassing some resetting.

Could you take a look at this?
  

[Alex Karasulu]

Added this test case which will fail now because of this bug:

same as below if broken ...

http://svn.apache.org/viewcvs.cgi/directory/trunks/shared/ldap/src/test/java/org/apache/directory/shared/ldap/name/DnParserDIRSERVER_584_Test.java?rev=379342&view=markup

or use ...

http://fonnicra.notlong.com/

The build will be broken. Perhaps I should not have done this bbut this is a serious bug.

[Alex Karasulu]

Here's the fix commit and it's diff:

http://svn.apache.org/viewcvs.cgi?rev=379345&view=rev
http://sabuluga.notlong.com

Ersin this might have been done in a better way. Let me know if this is unacceptable or if there is a better approach. I just figured that this is ok to do even if expensive since it happens only on exceptions. I thought this was already being done on exceptions in DnParser.parse(): I think I'm loosing my mind.

  

[Emmanuel Lecharny]

Sooner or latter we will have to get rid of the antlr DN parser ...

The soone, the better :) RC2 ?

[Alex Karasulu]

Remember our convo regarding the RCX series: meaning no major code mods just bug fixes. All features are fixed that is. This is to prevent introducing new bugs since we are trying to stabilize the server at this point for the 1.0. Let's push this replacement of the dn parser to 1.1 since it is a really big change.