Commons Digester
  1. Commons Digester
  2. DIGESTER-42

[digester] NodeCreateRule failing with Kaffe JVM (Aelfred xml parser)

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.6
    • Fix Version/s: None
    • Labels:
      None
    • Environment:

      Operating System: All
      Platform: All

      Description

      This test case - http://www.sfu.ca/~jdbates/tmp/commons-digester/Main.java

      • consistently produces this error -

      myhost% jikes-kaffe -classpath
      /usr/share/java/commons-beanutils.jar:/usr/share/java/commons-collections.jar:/usr/share/java/commons-digester.jar:/usr/share/java/commons-logging.jar:.
      Main.java
      myhost% /usr/lib/kaffe/bin/java -classpath
      /usr/share/java/commons-beanutils.jar:/usr/share/java/commons-collections.jar:/usr/share/java/commons-digester.jar:/usr/share/java/commons-logging.jar:.
      Main
      May 9, 2005 8:30:16 AM org.apache.commons.digester.Digester startElement
      SEVERE: Begin event threw exception
      java.lang.IllegalStateException: already parsing
      at org.apache.commons.digester.Digester.createSAXException (Digester.java:2540)
      at org.apache.commons.digester.Digester.createSAXException (Digester.java:2566)
      at org.apache.commons.digester.Digester.startElement (Digester.java:1276)
      at gnu.xml.aelfred2.SAXDriver.startElement (SAXDriver.java:1108)
      at gnu.xml.aelfred2.XmlParser.parseElement (XmlParser.java:1387)
      at gnu.xml.aelfred2.XmlParser.parseDocument (XmlParser.java:578)
      at gnu.xml.aelfred2.XmlParser.doParse (XmlParser.java:463)
      at gnu.xml.aelfred2.SAXDriver.parse (SAXDriver.java:364)
      at gnu.xml.aelfred2.XmlReader.parse (XmlReader.java:365)
      at org.apache.commons.digester.Digester.parse (Digester.java:1586)
      at Main.main (Main.java:21)
      myhost%

      I noticed this problem running the commons digester test.node build test

      I tried commons digester 1.5 & 1.6 with same results

      I'm running jikes 1.22 & kaffe 1.1.5

      Many thanks for your work on these great tools!

      Jack

        Activity

        Hide
        Simon Kitching added a comment -

        A fix has been applied to SetNestedPropertiesRule. Digester now passes all unit
        tests when run with Kaffe 1.1.6! I'm therefore closing this bugreport.

        Show
        Simon Kitching added a comment - A fix has been applied to SetNestedPropertiesRule. Digester now passes all unit tests when run with Kaffe 1.1.6! I'm therefore closing this bugreport.
        Hide
        Simon Kitching added a comment -

        I'm testing now with Kaffe 1.1.6 (released september 2005) and things are
        looking better.

        Currently, unit-test
        SetNestedPropertiesRuleTestCase.testUnknownChildrenCausesException
        is failing. This test ensures that an exception is thrown in the
        SetNestedPropertiesRule. One side-effect is that an AnyChildRules instance is
        left as the Rules object for the digester when the exception is thrown.

        In Sun Java 1.5, ContentHandler.endDocument is not called when an endElement
        method throws a SAXException, so the AnyChildRules instance causes no problems.

        In Kaffe 1.1.6, ContentHandler.endDocument is called when the SAXException is
        thrown. The endDocument method currently throws an exception because it tells
        the digester's Rules object to clean up and the AnyChildRules object doesn't
        support the necessary methods.

        From the javadoc for the org.sax.xml.ContentHandler class, it would appear that
        Kaffe is right and Sun is wrong about the invocation of endDocument after a
        SAXException. I'm going to file a bug against the Sun jdk, then think about the
        best way to handle this in Digester.

        Show
        Simon Kitching added a comment - I'm testing now with Kaffe 1.1.6 (released september 2005) and things are looking better. Currently, unit-test SetNestedPropertiesRuleTestCase.testUnknownChildrenCausesException is failing. This test ensures that an exception is thrown in the SetNestedPropertiesRule. One side-effect is that an AnyChildRules instance is left as the Rules object for the digester when the exception is thrown. In Sun Java 1.5, ContentHandler.endDocument is not called when an endElement method throws a SAXException, so the AnyChildRules instance causes no problems. In Kaffe 1.1.6, ContentHandler.endDocument is called when the SAXException is thrown. The endDocument method currently throws an exception because it tells the digester's Rules object to clean up and the AnyChildRules object doesn't support the necessary methods. From the javadoc for the org.sax.xml.ContentHandler class, it would appear that Kaffe is right and Sun is wrong about the invocation of endDocument after a SAXException. I'm going to file a bug against the Sun jdk, then think about the best way to handle this in Digester.
        Hide
        Simon Kitching added a comment -

        I finally decided to install kaffe 1.1.5 myself and look into this.

        The fact that for Kaffe/Classpath/Aelfred,
        Element e = doc.createElement("tagname");
        String localName = e.getLocalName()
        returns a non-null value is a violation of the standard. The javadoc for
        Document.createElement is quite clear that when a node is created with just a
        "qname", then getLocalName is expected to return null.

        I have raised this as bug#13387 in savannah classpath bugtracker.

        I've also been getting some very weird problems with one of the unit tests
        hanging in the DigesterLoaderTest when using kaffe. Adding a System.out.println
        or commenting out one of the tests (doesn't matter which one!) stops the hang.
        This is truly weird, and basically undebuggable as Kaffe doesn't support remote
        debugging AFAIK and adding print/logging statements into the code makes the
        problem go away!

        And of course this is just a problem with the unit tests; the code probably
        works fine.

        So I'm planning to leave things as they are at the moment, and revisit this when
        the next Kaffe release is made.

        Show
        Simon Kitching added a comment - I finally decided to install kaffe 1.1.5 myself and look into this. The fact that for Kaffe/Classpath/Aelfred, Element e = doc.createElement("tagname"); String localName = e.getLocalName() returns a non-null value is a violation of the standard. The javadoc for Document.createElement is quite clear that when a node is created with just a "qname", then getLocalName is expected to return null. I have raised this as bug#13387 in savannah classpath bugtracker. I've also been getting some very weird problems with one of the unit tests hanging in the DigesterLoaderTest when using kaffe. Adding a System.out.println or commenting out one of the tests (doesn't matter which one!) stops the hang. This is truly weird, and basically undebuggable as Kaffe doesn't support remote debugging AFAIK and adding print/logging statements into the code makes the problem go away! And of course this is just a problem with the unit tests; the code probably works fine. So I'm planning to leave things as they are at the moment, and revisit this when the next Kaffe release is made.
        Hide
        ms419 added a comment -

        (In reply to comment #12)
        > Sorry, I don't understand your last comment.

        Sorry I didn't have time for this last week - but I think I now have a better
        understanding

        http://www.sfu.ca/~jdbates/tmp/commons-digester/TestCreateElement.java - this
        test case shows how aelfred's getLocalName() always returns the tag name, even
        of elements created using the DOM1 createElement(String) method -

        fis% jikes-sablevm TestCreateElement.java
        fis% java TestCreateElement
        getNodeName(): foo
        getLocalName(): foo
        fis%

        Consequently, I don't think - assertNull(((Element)element).getLocalName()); -
        can ever succeed?

        I thought this is what you meant by -

        > However it seems that Aelfred provides localname even when namespace-aware is
        > false. And the specs say this is allowed.

        -

        > If you've applied the attached patch to the latest SVN code, and are still
        > getting the assert failures then I've made a mistake somewhere.

        That's correct - I applied your patch to latest SVN & asserts on lines 209, 240,
        248 & 256 of NodeCreateRuleTestCase.java still fail - but I gathered that
        non-null values were permissable, according to the spec?

        Many thanks, again, for your time on this problem!

        Jack

        Show
        ms419 added a comment - (In reply to comment #12) > Sorry, I don't understand your last comment. Sorry I didn't have time for this last week - but I think I now have a better understanding http://www.sfu.ca/~jdbates/tmp/commons-digester/TestCreateElement.java - this test case shows how aelfred's getLocalName() always returns the tag name, even of elements created using the DOM1 createElement(String) method - fis% jikes-sablevm TestCreateElement.java fis% java TestCreateElement getNodeName(): foo getLocalName(): foo fis% Consequently, I don't think - assertNull(((Element)element).getLocalName()); - can ever succeed? I thought this is what you meant by - > However it seems that Aelfred provides localname even when namespace-aware is > false. And the specs say this is allowed. - > If you've applied the attached patch to the latest SVN code, and are still > getting the assert failures then I've made a mistake somewhere. That's correct - I applied your patch to latest SVN & asserts on lines 209, 240, 248 & 256 of NodeCreateRuleTestCase.java still fail - but I gathered that non-null values were permissable, according to the spec? Many thanks, again, for your time on this problem! Jack
        Hide
        Simon Kitching added a comment -

        Sorry, I don't understand your last comment.

        With this patch I was hoping that all the unit tests would pass with Aelfred.
        I thought the issue was that the test assumes that namespace parsing is off, and
        therefore NodeCreateRule will generate DOM1 nodes, and therefore querying these
        nodes for localname will always return null.

        Namespace parsing is off, but because Aelfred provides localname info anyway,
        NodeCreateRule was assuming that namespace parsing was on, and therefore
        generating DOM2 nodes with localname info in them. The unit test then checks
        these DOM2 nodes, finds localname is set and (correctly) reports a test failure.

        The patch attempts to correct NodeCreateRule to actually check whether
        namespace parsing is enabled. If not, then it creates DOM1 nodes which have no
        localname info so those asserts should now pass.

        If you've applied the attached patch to the latest SVN code, and are still
        getting the assert failures then I've made a mistake somewhere.

        Show
        Simon Kitching added a comment - Sorry, I don't understand your last comment. With this patch I was hoping that all the unit tests would pass with Aelfred. I thought the issue was that the test assumes that namespace parsing is off, and therefore NodeCreateRule will generate DOM1 nodes, and therefore querying these nodes for localname will always return null. Namespace parsing is off, but because Aelfred provides localname info anyway, NodeCreateRule was assuming that namespace parsing was on, and therefore generating DOM2 nodes with localname info in them. The unit test then checks these DOM2 nodes, finds localname is set and (correctly) reports a test failure. The patch attempts to correct NodeCreateRule to actually check whether namespace parsing is enabled. If not, then it creates DOM1 nodes which have no localname info so those asserts should now pass. If you've applied the attached patch to the latest SVN code, and are still getting the assert failures then I've made a mistake somewhere.
        Hide
        ms419 added a comment -

        (In reply to comment #9)
        > Jack, I've attached a patch. Could you try this with Aelfred and see what happens?

        Cool - thanks for looking into the specification - your solution seems totally
        accurate

        The assertions on lines 209, 240, 248 & 256 of NodeCreateRuleTestCase.java -
        assertNull(((Element)element).getLocalName()); - still fail, but I gather they
        are allowed, with aelfred?

        Thanks again!

        Jack

        Show
        ms419 added a comment - (In reply to comment #9) > Jack, I've attached a patch. Could you try this with Aelfred and see what happens? Cool - thanks for looking into the specification - your solution seems totally accurate The assertions on lines 209, 240, 248 & 256 of NodeCreateRuleTestCase.java - assertNull(((Element)element).getLocalName()); - still fail, but I gather they are allowed, with aelfred? Thanks again! Jack
        Hide
        Simon Kitching added a comment -

        Created an attachment (id=15183)
        patch to NodeCreateRule

        Show
        Simon Kitching added a comment - Created an attachment (id=15183) patch to NodeCreateRule
        Hide
        Simon Kitching added a comment -

        Ok, I checked the SAX specs and it is clear that when namespace parsing is off,
        it is OPTIONAL for the parser to provide localname in callbacks. See the javadoc
        for method startElement here:
        http://www.saxproject.org/apidoc/org/xml/sax/ContentHandler.html

        But NodeCreateRule assumes that if localname is non-null then namespace
        parsing is enabled and it should therefore generate DOM2 nodes. Clearly Xerces
        doesn't provide localname, so parsing with namespace-aware=false results in DOM1
        nodes.

        However it seems that Aelfred provides localname even when namespace-aware is
        false. And the specs say this is allowed.

        So question: should we generate DOM1 or DOM2 nodes when parsing with Aelfred and
        namespace-aware false? We do have the information available to generate DOM2
        nodes (I think). But this would be inconsistent with the behaviour of other
        parsers, so I think that we should generate DOM1 always when namespace-aware
        is false.

        So I believe that this qualifies as a bug in NodeCreateRule.

        Jack, I've attached a patch. Could you try this with Aelfred and see what happens?

        Note: there is potentially a change in behaviour (ie breaks existing code). I
        don't think it does, but I would appreciate a review of this code by someone
        before I commit it to Digester.

        Show
        Simon Kitching added a comment - Ok, I checked the SAX specs and it is clear that when namespace parsing is off, it is OPTIONAL for the parser to provide localname in callbacks. See the javadoc for method startElement here: http://www.saxproject.org/apidoc/org/xml/sax/ContentHandler.html But NodeCreateRule assumes that if localname is non-null then namespace parsing is enabled and it should therefore generate DOM2 nodes. Clearly Xerces doesn't provide localname, so parsing with namespace-aware=false results in DOM1 nodes. However it seems that Aelfred provides localname even when namespace-aware is false. And the specs say this is allowed. So question: should we generate DOM1 or DOM2 nodes when parsing with Aelfred and namespace-aware false? We do have the information available to generate DOM2 nodes (I think). But this would be inconsistent with the behaviour of other parsers, so I think that we should generate DOM1 always when namespace-aware is false. So I believe that this qualifies as a bug in NodeCreateRule. Jack, I've attached a patch. Could you try this with Aelfred and see what happens? Note: there is potentially a change in behaviour (ie breaks existing code). I don't think it does, but I would appreciate a review of this code by someone before I commit it to Digester.
        Hide
        Simon Kitching added a comment -

        (In reply to comment #7)
        > Both assertions fail because ((Element)element).getLocalName() is "alpha", not
        > null. I haven't yet figured out if this is OK or a bug in gnu classpath...

        When xml namespaces are enabled, there are two parts to an element name:
        namespaceURI and localname (eg "http://acme.com/ns", "item")

        When xml namespaces are disabled, there is one part to an element name:
        qname (eg "ns:item")

        Quite what localname should be when namespaces are disabled, or what qname
        should be when namespaces are enabled is very ambiguous in the javadoc for
        sax/dom at least. I suspect that Xerces reports NULL for qname when namespaces
        are enabled, and NULL for localname when namespaces are disabled, while Aelfred
        populates qname even when namespaces are disabled (reasonable) and populates
        localname even when namespaces are enabled. Which XML parser is actually
        complying with the SAX/DOM specs I don't know. Or whether the specs don't say
        anything on this issue, in which case we should not be asserting one way or the
        other.

        And quite how localname/qname map to the DOM "NodeName" isn't clear from the
        javadoc at least.

        Writing a simple test harness and running it with Aelfred and Xerces should show
        if this is the cause. Then we would have to figure out whether one of the
        parsers was actually non-compliant with the specs. And if so, whether we should
        try to "fix" the data the parser is reporting in NodeCreateRule or not. All
        looks like a fair bit of work.

        But the fact that those are the only test failures on kaffe/gnu-classpath is
        great! [esp. as they might just be over-zealous asserts anyway, if the relevant
        specs allow localname to be set when namespaces are enabled etc].

        Show
        Simon Kitching added a comment - (In reply to comment #7) > Both assertions fail because ((Element)element).getLocalName() is "alpha", not > null. I haven't yet figured out if this is OK or a bug in gnu classpath... When xml namespaces are enabled, there are two parts to an element name: namespaceURI and localname (eg "http://acme.com/ns", "item") When xml namespaces are disabled, there is one part to an element name: qname (eg "ns:item") Quite what localname should be when namespaces are disabled, or what qname should be when namespaces are enabled is very ambiguous in the javadoc for sax/dom at least. I suspect that Xerces reports NULL for qname when namespaces are enabled, and NULL for localname when namespaces are disabled, while Aelfred populates qname even when namespaces are disabled (reasonable) and populates localname even when namespaces are enabled. Which XML parser is actually complying with the SAX/DOM specs I don't know. Or whether the specs don't say anything on this issue, in which case we should not be asserting one way or the other. And quite how localname/qname map to the DOM "NodeName" isn't clear from the javadoc at least. Writing a simple test harness and running it with Aelfred and Xerces should show if this is the cause. Then we would have to figure out whether one of the parsers was actually non-compliant with the specs. And if so, whether we should try to "fix" the data the parser is reporting in NodeCreateRule or not. All looks like a fair bit of work. But the fact that those are the only test failures on kaffe/gnu-classpath is great! [esp. as they might just be over-zealous asserts anyway, if the relevant specs allow localname to be set when namespaces are enabled etc].
        Hide
        ms419 added a comment -

        (In reply to comment #6)
        > If it doesn't, please let
        > me know.

        Awesome! We're getting somewhere - only problem is failed assertion -

        test.node:
        [echo] Running NodeCreateRule tests ...
        [java] ..F.F....
        [java] Time: 0.652
        [java] There were 2 failures:
        [java] 1) testElement(org.apache.commons.digester.NodeCreateRuleTestCase)ju
        nit.framework.AssertionFailedError
        [java] at org.apache.commons.digester.NodeCreateRuleTestCase.testElement
        (NodeCreateRuleTestCase.java:209)
        [java] at java.lang.reflect.Method.invokeNative (Method.java)
        [java] at java.lang.reflect.Method.invoke (Method.java:616)
        [java] at java.lang.VirtualMachine.invokeMain (VirtualMachine.java)
        [java] at java.lang.VirtualMachine.main (VirtualMachine.java:108)
        [java] 2) testDocumentFragment(org.apache.commons.digester.NodeCreateRuleTe
        stCase)junit.framework.AssertionFailedError
        [java] at org.apache.commons.digester.NodeCreateRuleTestCase.testDocumen
        tFragment (NodeCreateRuleTestCase.java:240)
        [java] at java.lang.reflect.Method.invokeNative (Method.java)
        [java] at java.lang.reflect.Method.invoke (Method.java:616)
        [java] at java.lang.VirtualMachine.invokeMain (VirtualMachine.java)
        [java] at java.lang.VirtualMachine.main (VirtualMachine.java:108)

        Both assertions fail because ((Element)element).getLocalName() is "alpha", not
        null. I haven't yet figured out if this is OK or a bug in gnu classpath...

        Thanks again!

        Jack

        Show
        ms419 added a comment - (In reply to comment #6) > If it doesn't, please let > me know. Awesome! We're getting somewhere - only problem is failed assertion - test.node: [echo] Running NodeCreateRule tests ... [java] ..F.F.... [java] Time: 0.652 [java] There were 2 failures: [java] 1) testElement(org.apache.commons.digester.NodeCreateRuleTestCase)ju nit.framework.AssertionFailedError [java] at org.apache.commons.digester.NodeCreateRuleTestCase.testElement (NodeCreateRuleTestCase.java:209) [java] at java.lang.reflect.Method.invokeNative (Method.java) [java] at java.lang.reflect.Method.invoke (Method.java:616) [java] at java.lang.VirtualMachine.invokeMain (VirtualMachine.java) [java] at java.lang.VirtualMachine.main (VirtualMachine.java:108) [java] 2) testDocumentFragment(org.apache.commons.digester.NodeCreateRuleTe stCase)junit.framework.AssertionFailedError [java] at org.apache.commons.digester.NodeCreateRuleTestCase.testDocumen tFragment (NodeCreateRuleTestCase.java:240) [java] at java.lang.reflect.Method.invokeNative (Method.java) [java] at java.lang.reflect.Method.invoke (Method.java:616) [java] at java.lang.VirtualMachine.invokeMain (VirtualMachine.java) [java] at java.lang.VirtualMachine.main (VirtualMachine.java:108) Both assertions fail because ((Element)element).getLocalName() is "alpha", not null. I haven't yet figured out if this is OK or a bug in gnu classpath... Thanks again! Jack
        Hide
        Simon Kitching added a comment -

        Oops - using a java1.5 method in the unit test wasn't intentional. I have fixed
        this. Hopefully the HEAD version now compiles for you. If it doesn't, please let
        me know.

        Show
        Simon Kitching added a comment - Oops - using a java1.5 method in the unit test wasn't intentional. I have fixed this. Hopefully the HEAD version now compiles for you. If it doesn't, please let me know.
        Hide
        ms419 added a comment -

        (In reply to comment #4)
        > Could you please pull the latest source from subversion and test?
        > Or get a nightly build and test that...

        Hmm - compile.tests in latest svn fails because gnu classpath doesn't yet
        implement String.contains(CharSequence) -

        [...]
        [javac] Found 1 semantic error compiling
        "/home/jablko/src/tmp/libcommons-digester-java/export/digester/trunk/src/test/org/apache/commons/digester/SetNestedPropertiesRuleTestCase.java":

        [javac] 182. if (msg.contains("badprop")) {
        [javac] ---------------------
        [javac] *** Semantic Error: No accessible method with signature
        "contains(java.lang.String)" was found in type "java.lang.String".
        [...]

        Not really worth reporting against classpath, however, since
        Strin.contains(CharSequence) was added in java 1.5 & classpath isn't yet 1.5
        compatible...

        I may try back patching your changes to digester 1.6 & see how that works

        Thanks sincerely for working on this!

        Jack

        Show
        ms419 added a comment - (In reply to comment #4) > Could you please pull the latest source from subversion and test? > Or get a nightly build and test that... Hmm - compile.tests in latest svn fails because gnu classpath doesn't yet implement String.contains(CharSequence) - [...] [javac] Found 1 semantic error compiling "/home/jablko/src/tmp/libcommons-digester-java/export/digester/trunk/src/test/org/apache/commons/digester/SetNestedPropertiesRuleTestCase.java": [javac] 182. if (msg.contains("badprop")) { [javac] --------------------- [javac] *** Semantic Error: No accessible method with signature "contains(java.lang.String)" was found in type "java.lang.String". [...] Not really worth reporting against classpath, however, since Strin.contains(CharSequence) was added in java 1.5 & classpath isn't yet 1.5 compatible... I may try back patching your changes to digester 1.6 & see how that works Thanks sincerely for working on this! Jack
        Hide
        Simon Kitching added a comment -

        I've just committed a patch that should fix this.
        See subversion revision 178343.

        Could you please pull the latest source from subversion and test?
        Or get a nightly build and test that..

        Show
        Simon Kitching added a comment - I've just committed a patch that should fix this. See subversion revision 178343. Could you please pull the latest source from subversion and test? Or get a nightly build and test that..
        Hide
        Simon Kitching added a comment -

        (In reply to comment #2)
        > Huh - I enabled debugging output, as described - yet I don't see any more
        > detailed output

        I think the issue is that you are passing those params to ant, and they aren't
        being passed through to the code that ant (presumably) executes via a <java>
        task. The and <java> task can take subtags for passing system properties to the
        executed code; I suggest you try using those.

        Or maybe it's the junit libs that are hiding these properties from the code;
        junit fiddles around with classloaders in order to make the executed tests think
        they are running in a "clean" VM. Not sure what the effect is on system
        properties, but I wouldn't be surprised...

        > >And secondly, could you please write a simple SAX app which attempts to set the
        > >content handler after parsing has started and see what happens?
        >
        > I think your right - aelfred pukes on setting the handler when parsing has
        > started - even if it's setting to the current value - setDTDHandler(this)
        >
        > This test case reproduces the problem, I think -
        > http://www.sfu.ca/~jdbates/tmp/commons-digester/Test.java

        Well, the issue is setting the contentHandler, but if setDTDHandler fails for
        aelfred, then setContentHandler probably does too.

        >
        > But this shouldn't be a difficult fix - is there any reason
        > reader.setDTDHandler(this) (Digester.java:905) shouldn't depend on the 'if
        > (reader == null)' (Digester.java:901) clause?

        No, unfortunately it's quite difficult to fix. Digester creates a parser, then
        sets itself as the content handler for SAX events from that parser (line 895
        of Digester.java) so that it can do its work.

        NodeCreateRule, however, wants to receive the raw SAX events itself, so it
        temporarily changes the ContentHandler set for that same parser object to its
        own custom content handler. After processing of the node is complete, it sets
        the ContentHandler back to being the digester instance again. Between these two
        steps, the digester object is completely inactive; it doesn't get any sax events.

        In order to handle aelfred, it would be necessary to put a layer of indirection
        in there somewhere. As it happens, the digester2 code (see subversion
        repository) handles this case nicely (see the SAXHandler class). It has a field
        that the user can set to point to a custom handler. Therefore, rather than
        NodeCreateRule reconfiguring the parser's contenthandler, it sets a member in
        the digester object to forward calls to itself, and all of digester's SAX
        callbacks check whether this member is set.

        eg in the "characters" method of Digester
        if (customContentHandler != null)

        { // forward calls instead of handling them here customContentHandler.characters(buffer, start, length); return; }

        and the same in all the other sax callback methods.

        If you wished to put together a patch to implement this, I would be happy to
        review it and commit it (assuming no objections from anyone else).

        Regards,

        Simon

        Show
        Simon Kitching added a comment - (In reply to comment #2) > Huh - I enabled debugging output, as described - yet I don't see any more > detailed output I think the issue is that you are passing those params to ant , and they aren't being passed through to the code that ant (presumably) executes via a <java> task. The and <java> task can take subtags for passing system properties to the executed code; I suggest you try using those. Or maybe it's the junit libs that are hiding these properties from the code; junit fiddles around with classloaders in order to make the executed tests think they are running in a "clean" VM. Not sure what the effect is on system properties, but I wouldn't be surprised... > >And secondly, could you please write a simple SAX app which attempts to set the > >content handler after parsing has started and see what happens? > > I think your right - aelfred pukes on setting the handler when parsing has > started - even if it's setting to the current value - setDTDHandler(this) > > This test case reproduces the problem, I think - > http://www.sfu.ca/~jdbates/tmp/commons-digester/Test.java Well, the issue is setting the contentHandler , but if setDTDHandler fails for aelfred, then setContentHandler probably does too. > > But this shouldn't be a difficult fix - is there any reason > reader.setDTDHandler(this) (Digester.java:905) shouldn't depend on the 'if > (reader == null)' (Digester.java:901) clause? No, unfortunately it's quite difficult to fix. Digester creates a parser, then sets itself as the content handler for SAX events from that parser (line 895 of Digester.java) so that it can do its work. NodeCreateRule, however, wants to receive the raw SAX events itself, so it temporarily changes the ContentHandler set for that same parser object to its own custom content handler. After processing of the node is complete, it sets the ContentHandler back to being the digester instance again. Between these two steps, the digester object is completely inactive; it doesn't get any sax events. In order to handle aelfred, it would be necessary to put a layer of indirection in there somewhere. As it happens, the digester2 code (see subversion repository) handles this case nicely (see the SAXHandler class). It has a field that the user can set to point to a custom handler. Therefore, rather than NodeCreateRule reconfiguring the parser's contenthandler, it sets a member in the digester object to forward calls to itself, and all of digester's SAX callbacks check whether this member is set. eg in the "characters" method of Digester if (customContentHandler != null) { // forward calls instead of handling them here customContentHandler.characters(buffer, start, length); return; } and the same in all the other sax callback methods. If you wished to put together a patch to implement this, I would be happy to review it and commit it (assuming no objections from anyone else). Regards, Simon
        Hide
        ms419 added a comment -

        >Firstly, could you please run your test again with logging enabled?
        >See the section titled "How do I enable debugging output" in the FAQ:
        > http://wiki.apache.org/jakarta-commons/Digester/FAQ

        Huh - I enabled debugging output, as described - yet I don't see any more
        detailed output

        Here's the full output of digester build tests with debugging enabled -
        http://www.sfu.ca/~jdbates/tmp/commons-digester/screenlog

        It's wiered because I see the call to log.error("Begin event threw exception",
        e) (Digester.java:1275) - & according to the commons logging docs,
        -Dorg.apache.commons.logging.Log=org.apache.commons.logging.impl.SimpleLog
        -Dorg.apache.commons.logging.simplelog.defaultlog=debug should do the trick...

        >And secondly, could you please write a simple SAX app which attempts to set the
        >content handler after parsing has started and see what happens?

        I think your right - aelfred pukes on setting the handler when parsing has
        started - even if it's setting to the current value - setDTDHandler(this)

        This test case reproduces the problem, I think -
        http://www.sfu.ca/~jdbates/tmp/commons-digester/Test.java

        But this shouldn't be a difficult fix - is there any reason
        reader.setDTDHandler(this) (Digester.java:905) shouldn't depend on the 'if
        (reader == null)' (Digester.java:901) clause?

        Thanks again for your work on this!

        Jack

        Show
        ms419 added a comment - >Firstly, could you please run your test again with logging enabled? >See the section titled "How do I enable debugging output" in the FAQ: > http://wiki.apache.org/jakarta-commons/Digester/FAQ Huh - I enabled debugging output, as described - yet I don't see any more detailed output Here's the full output of digester build tests with debugging enabled - http://www.sfu.ca/~jdbates/tmp/commons-digester/screenlog It's wiered because I see the call to log.error("Begin event threw exception", e) (Digester.java:1275) - & according to the commons logging docs, -Dorg.apache.commons.logging.Log=org.apache.commons.logging.impl.SimpleLog -Dorg.apache.commons.logging.simplelog.defaultlog=debug should do the trick... >And secondly, could you please write a simple SAX app which attempts to set the >content handler after parsing has started and see what happens? I think your right - aelfred pukes on setting the handler when parsing has started - even if it's setting to the current value - setDTDHandler(this) This test case reproduces the problem, I think - http://www.sfu.ca/~jdbates/tmp/commons-digester/Test.java But this shouldn't be a difficult fix - is there any reason reader.setDTDHandler(this) (Digester.java:905) shouldn't depend on the 'if (reader == null)' (Digester.java:901) clause? Thanks again for your work on this! Jack
        Hide
        Simon Kitching added a comment -

        Hi Jack.

        Interesting. My initial thought is that NodeCreateRule is doing something that
        Xerces is happy with but that Aelfred doesn't want to allow.

        My guess is that
        getDigester().getXMLReader().setContentHandler(...)
        is the issue, ie that Aelfred doesn't like people to modify the content handler
        after parsing has started. I've checked the Aelfred javadoc online, but it
        doesn't say anything relevant.

        Firstly, could you please run your test again with logging enabled?
        See the section titled "How do I enable debugging output" in the FAQ:
        http://wiki.apache.org/jakarta-commons/Digester/FAQ

        And secondly, could you please write a simple SAX app which attempts to set the
        content handler after parsing has started and see what happens?

        Have you run the complete digester test suite using kaffe? I should do that
        sometime to see what happens..

        Show
        Simon Kitching added a comment - Hi Jack. Interesting. My initial thought is that NodeCreateRule is doing something that Xerces is happy with but that Aelfred doesn't want to allow. My guess is that getDigester().getXMLReader().setContentHandler(...) is the issue, ie that Aelfred doesn't like people to modify the content handler after parsing has started. I've checked the Aelfred javadoc online, but it doesn't say anything relevant. Firstly, could you please run your test again with logging enabled? See the section titled "How do I enable debugging output" in the FAQ: http://wiki.apache.org/jakarta-commons/Digester/FAQ And secondly, could you please write a simple SAX app which attempts to set the content handler after parsing has started and see what happens? Have you run the complete digester test suite using kaffe? I should do that sometime to see what happens..

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development