Tapestry 5
  1. Tapestry 5
  2. TAP5-745

Remove Woodstox-specific Stax implementation usage

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Won't Fix
    • Affects Version/s: 5.1.0.0, 5.1.0.1, 5.1.0.2, 5.1.0.3, 5.1.0.4, 5.1.0.5, 5.1
    • Fix Version/s: None
    • Component/s: tapestry-core
    • Labels:
      None

      Description

      Tapestry uses some special extensions to StaX (out of Woodstox) in the template parser. This leads to the problem that Tapestry will usually not run on any application server because the appservers will use their own implementation of Stax.

      There is a workaround but a main stream web application framework should run on JEE compatible web and application servers without tweaking.

      The main problem is in org.apache.tapestry5.internal.services.TemplateParserImpl.<init>(TemplateParserImpl.java:44). Here, XMLInputFactory2 is asked for an instance - but XMLInputFactory2 does not implement the method newInstance. This is delegated to XMLInputFactory. So, the original XMLInputFactory is used - which returns the platform implementation of Stax.

      Workaround:
      Add the system property below to Application Server (either via startup script or admin console):
      -Djavax.xml.stream.XMLInputFactory=com.ctc.wstx.stax.WstxInputFactory

      1. TAP5-745-5.1.0.5.patch
        5 kB
        Christian Köberl

        Activity

        Christian Köberl created issue -
        Hide
        Christian Köberl added a comment -

        Here is the patch of the changes described in http://derkoe.wordpress.com/2009/04/16/tapestry-51-woodstox/

        Show
        Christian Köberl added a comment - Here is the patch of the changes described in http://derkoe.wordpress.com/2009/04/16/tapestry-51-woodstox/
        Christian Köberl made changes -
        Field Original Value New Value
        Attachment TAP5-745-5.1.0.5.patch [ 12410405 ]
        Hide
        Christian Köberl added a comment -

        Changed to bug, since the new WAS-CE 2.1 will not start with option "-Djavax.xml.stream.XMLInputFactory=com.ctc.wstx.stax.WstxInputFactory"

        Show
        Christian Köberl added a comment - Changed to bug, since the new WAS-CE 2.1 will not start with option "-Djavax.xml.stream.XMLInputFactory=com.ctc.wstx.stax.WstxInputFactory"
        Christian Köberl made changes -
        Issue Type Improvement [ 4 ] Bug [ 1 ]
        Hide
        Thiago H. de Paula Figueiredo added a comment -

        Duplicate of https://issues.apache.org/jira/browse/TAP5-713. Different reasons, same problem, same solution.

        Show
        Thiago H. de Paula Figueiredo added a comment - Duplicate of https://issues.apache.org/jira/browse/TAP5-713 . Different reasons, same problem, same solution.
        Thiago H. de Paula Figueiredo made changes -
        Status Open [ 1 ] Closed [ 6 ]
        Resolution Duplicate [ 3 ]
        Hide
        Christian Köberl added a comment -

        No, this is not the same problem as TAP5-713!!

        The problem with TAP5-713 is that there is no StAX available (e.g. on GAE) - the problem here is that Tapestry uses extensions to StAX (which it should not do). Using extensions to XML standard implementations in Java brings us back to XML hell some years ago where you had to exactly use XML parser X in version Z to make some app work.

        The patch provided will fix this problem (eliminating the dependency to Woodstox but not to StAX)! The patch can be easily applied (just some lines of code are changed) - a fix for TAP5-713 will need a major rewrite of the template parser!

        Show
        Christian Köberl added a comment - No, this is not the same problem as TAP5-713 !! The problem with TAP5-713 is that there is no StAX available (e.g. on GAE) - the problem here is that Tapestry uses extensions to StAX (which it should not do). Using extensions to XML standard implementations in Java brings us back to XML hell some years ago where you had to exactly use XML parser X in version Z to make some app work. The patch provided will fix this problem (eliminating the dependency to Woodstox but not to StAX)! The patch can be easily applied (just some lines of code are changed) - a fix for TAP5-713 will need a major rewrite of the template parser!
        Christian Köberl made changes -
        Resolution Duplicate [ 3 ]
        Status Closed [ 6 ] Reopened [ 4 ]
        Hide
        Thiago H. de Paula Figueiredo added a comment -

        I didn't notice the patch and the solution to TAP5-713 will solve this issue, so I thought I could close this one as a duplicate. You're right about saying that a solution for TAP5-713 will take some time. I'm sorry.

        Show
        Thiago H. de Paula Figueiredo added a comment - I didn't notice the patch and the solution to TAP5-713 will solve this issue, so I thought I could close this one as a duplicate. You're right about saying that a solution for TAP5-713 will take some time. I'm sorry.
        Hide
        Christian E Gruber added a comment -

        +1 (well, actually my vote is registed on the side). It seems to me that this would be a fast fix, a correct fix, and it would allow more compatibility with more app-servers before the TAP5-713 solution. This way if google implements javax.xml.stream before T5.2 (or whenever the StAX removal is done) it'll just start working.

        Thiago, you're a committer now, aren't you?

        Show
        Christian E Gruber added a comment - +1 (well, actually my vote is registed on the side). It seems to me that this would be a fast fix, a correct fix, and it would allow more compatibility with more app-servers before the TAP5-713 solution. This way if google implements javax.xml.stream before T5.2 (or whenever the StAX removal is done) it'll just start working. Thiago, you're a committer now, aren't you?
        Hide
        Thiago H. de Paula Figueiredo added a comment -

        Christian: Yes, I'm a committer, unfortunately too busy to contribute to Tapestry lately.

        Show
        Thiago H. de Paula Figueiredo added a comment - Christian: Yes, I'm a committer, unfortunately too busy to contribute to Tapestry lately.
        Hide
        Christian E Gruber added a comment -

        Any other committers able to apply this patch?

        Show
        Christian E Gruber added a comment - Any other committers able to apply this patch?
        Robert Zeigler made changes -
        Assignee Robert Zeigler [ ongakugainochi ]
        Hide
        Robert Zeigler added a comment -

        Hm. The patch appears to require java 6 to compile. Investigating.

        Show
        Robert Zeigler added a comment - Hm. The patch appears to require java 6 to compile. Investigating.
        Hide
        Robert Zeigler added a comment -

        So, after dealing with various dependency issues, I've run into the following interesting situation.

        If I supply the stax dependency with woodstox (scope: testing), all of the dtd-related tests fail. Looking closer, the dtd element is recognized, but reader.getText() returns an empty string (according to the stax api, it should return the DTD string). If I supply the stax dependency with stax:stax (1.2.0), I get an exception, with the parser complaining that "this implementation does not resolve external entities". I'll look at this more tomorrow, but I'm open to suggestions.

        Show
        Robert Zeigler added a comment - So, after dealing with various dependency issues, I've run into the following interesting situation. If I supply the stax dependency with woodstox (scope: testing), all of the dtd-related tests fail. Looking closer, the dtd element is recognized, but reader.getText() returns an empty string (according to the stax api, it should return the DTD string). If I supply the stax dependency with stax:stax (1.2.0), I get an exception, with the parser complaining that "this implementation does not resolve external entities". I'll look at this more tomorrow, but I'm open to suggestions.
        Hide
        Robert Zeigler added a comment -

        Complicated.

        http://jira.codehaus.org/browse/WSTX-78 (closed)

        talks about the spec saying that getText() on the dtd element returns the "internal subset" of the DTD (which is true in java 1.6 docs; 1.5 docs (via webservices) says simply: String value of the DTD). Basically, what this means is that there is no implementation-independent manner of dealing with dtd's right now. Incidentally, the maintenance release of the stax jsr mentioned in WSTX-78 has been withdrawn. There's a pending maintenance release now, but, it doesn't touch dtd's.

        I've now tried running the full tapestry-core test suite with the patch with:

        a) woodstox as the underlying parser
        b) stax:stax:1.2.0 as the underlying parser
        c) recompiling with java 1.6 and using the built-in parser

        a: fails due to getText() returning the empty string for the DTD
        b: likewise fails, but additionally fails due to not supporting external entities
        c: worst of all. Can't handle namespace prefixes; at least, it ignores them as the code is now (plus it would make tapestry require java 1.6, which is a nogo; this was more for my curiosity than anything else).

        At this point, it appears that there is no good way to deal with DTD's via the standard stax API, but properly dealing with DTD's is critically important since client interpretation of css depends on the doctype used. Thus, I'm closing this issue as "won't fix" and woodstox-independence will have to wait until either:

        1) TAP5-713 is resolved or
        2) A user who cares about woodstox independence is willing to step up and provide a patch that actually works (please: be sure to run all of the tapestry-core tests, at least, and make sure they pass before submitting! And be sure to test with java 1.5!)

        Show
        Robert Zeigler added a comment - Complicated. http://jira.codehaus.org/browse/WSTX-78 (closed) talks about the spec saying that getText() on the dtd element returns the "internal subset" of the DTD (which is true in java 1.6 docs; 1.5 docs (via webservices) says simply: String value of the DTD). Basically, what this means is that there is no implementation-independent manner of dealing with dtd's right now. Incidentally, the maintenance release of the stax jsr mentioned in WSTX-78 has been withdrawn. There's a pending maintenance release now, but, it doesn't touch dtd's. I've now tried running the full tapestry-core test suite with the patch with: a) woodstox as the underlying parser b) stax:stax:1.2.0 as the underlying parser c) recompiling with java 1.6 and using the built-in parser a: fails due to getText() returning the empty string for the DTD b: likewise fails, but additionally fails due to not supporting external entities c: worst of all. Can't handle namespace prefixes; at least, it ignores them as the code is now (plus it would make tapestry require java 1.6, which is a nogo; this was more for my curiosity than anything else). At this point, it appears that there is no good way to deal with DTD's via the standard stax API, but properly dealing with DTD's is critically important since client interpretation of css depends on the doctype used. Thus, I'm closing this issue as "won't fix" and woodstox-independence will have to wait until either: 1) TAP5-713 is resolved or 2) A user who cares about woodstox independence is willing to step up and provide a patch that actually works (please: be sure to run all of the tapestry-core tests, at least, and make sure they pass before submitting! And be sure to test with java 1.5!)
        Robert Zeigler made changes -
        Status Reopened [ 4 ] Closed [ 6 ]
        Resolution Won't Fix [ 2 ]
        Hide
        Christian E Gruber added a comment -

        I'll be giving it a shot.

        Christian.

        Christian Edward Gruber
        christianedwardgruber@gmail.com
        http://www.geekinasuit.com/

        Show
        Christian E Gruber added a comment - I'll be giving it a shot. Christian. Christian Edward Gruber christianedwardgruber@gmail.com http://www.geekinasuit.com/
        Hide
        Christian E Gruber added a comment -

        I'll be giving it a shot.

        Christian.

        Christian Edward Gruber
        christianedwardgruber@gmail.com
        http://www.geekinasuit.com/

        Show
        Christian E Gruber added a comment - I'll be giving it a shot. Christian. Christian Edward Gruber christianedwardgruber@gmail.com http://www.geekinasuit.com/
        Hide
        Christian Köberl added a comment -

        I'm sorry - i tried my patch only on Java6/Win (where it works - including tests).

        I'm also investigating on this - this Stax stuff seems to be a real mess! The RI is not supporting stuff that is defined in the standard (i.e XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES set to true).

        I saw that Howard was already trying the same stuff:
        http://jira.codehaus.org/browse/STAX-54

        Hmm - let's see how this can be fixed.

        Chris

        Show
        Christian Köberl added a comment - I'm sorry - i tried my patch only on Java6/Win (where it works - including tests). I'm also investigating on this - this Stax stuff seems to be a real mess! The RI is not supporting stuff that is defined in the standard (i.e XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES set to true). I saw that Howard was already trying the same stuff: http://jira.codehaus.org/browse/STAX-54 Hmm - let's see how this can be fixed. Chris

          People

          • Assignee:
            Robert Zeigler
            Reporter:
            Christian Köberl
          • Votes:
            2 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development