Directory ApacheDS
  1. Directory ApacheDS
  2. DIRSERVER-584

LdapName breaks static parser after repeated use.

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 1.0-RC1
    • Fix Version/s: 1.0-RC1
    • Component/s: ldap
    • Labels:
      None

      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

        Unknown macro: { suffix = suffix.trim(); try202}

      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

        Activity

        Hide
        Alex Karasulu added a comment -

        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

        Show
        Alex Karasulu added a comment - 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
        Hide
        Alex Karasulu added a comment -

        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.

        Show
        Alex Karasulu added a comment - 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.
        Hide
        Alex Karasulu added a comment -

        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?

        Show
        Alex Karasulu added a comment - 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?
        Hide
        Alex Karasulu added a comment -

        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.

        Show
        Alex Karasulu added a comment - 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.
        Hide
        Alex Karasulu added a comment -

        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.

        Show
        Alex Karasulu added a comment - 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.
        Hide
        Emmanuel Lecharny added a comment -

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

        The soone, the better RC2 ?

        Show
        Emmanuel Lecharny added a comment - Sooner or latter we will have to get rid of the antlr DN parser ... The soone, the better RC2 ?
        Hide
        Alex Karasulu added a comment -

        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.

        Show
        Alex Karasulu added a comment - 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.

          People

          • Assignee:
            Unassigned
            Reporter:
            Alex Karasulu
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development