[jdom-interest] SaxBuilder.setFeature Bug Report and Fix

[Bradley S. Huffman]

After looking at it, it's not going to be that hard to "fix".  But I see
potential problems. For example, as noted in internal class
JAXPParserFactory some JAXP features can only be set on the JAXP
factory, so if those features change, the only way to propagate them may be
by creating a new XMLReader.

And what about XMLFilter?  If setXMLFilter() is called multiple times or
between builds, right now setXMLFilter appends the new filter to the chain,
but doesn't remove or replace any of the filters already in the chain.

Brad

Jason Hunter writes:

> Hi Jason,
> 
> Great bug report.  I'll take a look next chance I get.  Brad might beat 
> me to it since I'm at XML 2004 this week.  (Hi to anyone else here.)
> 
> -jh-
> 
> Jason Winnebeck wrote:
> 
> > Confirmed to be working in JDOM-b9
> > Confirmed to be broken in JDOM-1.0 and JDOM nightly 11-16-2004
> > 
> > I recently upgraded from JDOM beta 9 to JDOM 1.0, and some of my parsing 
> > code broke.  I am using Xerces 2.6 with schema validation for some 
> > files, and no schema validation for others.  I reuse the same SAXBuilder 
> > to build the files, but I change features after build.
> > 
> > In JDOM b9, this code works.  Here are select lines of code from the 
> > actual program.
> > 
> > SAXBuilder builder = new SAXBuilder( 
> > "org.apache.xerces.parsers.SAXParser", false );
> > builder.setFeature( "http://apache.org/xml/features/validation/schema", 
> > true );
> > 
> > builder.setIgnoringElementContentWhitespace( true );
> > Document doc = builder.build( new File( "SystemConfigurations.xml" ) );
> > 
> > //Lines omitted that use doc
> > 
> > //Lines omitted that construct the userConfigs strings and check to see
> > //if file should be loaded
> > builder.setFeature( "http://apache.org/xml/features/validation/schema", 
> > false );
> > doc = builder.build( userConfigs );
> > 
> > In JDOM 1.0, the schema validation will not be turned off for Xerces, 
> > and the second document will not build since it cannot find the schema 
> > (and therefore not the definition for the root element).
> > 
> > Using a debugger and after examining code, I found that 
> > SAXBuilder.setFeature only sets an entry in a map which is used in 
> > createParser, which is only called once if reuseParser is true (default).
> > 
> > My first attempt to fix this problem was to add lines to setFeature and 
> > setProperty that would set the feature or property directly on 
> > this.saxParser if it was not null -- however the methods cannot throw 
> > JDOMException and I did not want to change the API.
> > 
> > So what I did was in SAXBuilder.build( InputSource ) I added a call to 
> > setFeaturesandProperties.
> > 
> > I modified both JDOM 11-16-04 nightly and JDOM 1.0 and the fix worked 
> > for both.  I decided for our organization it was best to patch 1.0 than 
> > patch and use the nightly, so the line number is based off of JDOM 1.0.
> > 
> > Insert at line 455:
> >               //If the user has changed features or properties, they 
> > need to be reset.
> >               //In the future we could have a flag to prevent extra work.
> >               setFeaturesAndProperties( parser, false );
> > 
> > Surrounding old code plus new code, starting at line 451:
> >             else {
> >                 // Reset content handler as SAXHandler instances cannot
> >                 // be reused
> >                 configureParser(parser, contentHandler);
> > 
> >               //If the user has changed features or properties, they 
> > need to be reset.
> >               //In the future we could have a flag to prevent extra work.
> >               setFeaturesAndProperties( parser, false );
> >             }
> > 
> > This seems to work for my application, but this is my first time looking 
> > at JDOM code and I only spent about 15 minutes examining the class 
> > before I made the change, so I do not know how correct the code is.  The 
> > one thing I am unsure of is the boolean parameter to 
> > setFeaturesAndProperties.  It uses true when createParser is called for 
> > my application's case, but the false seems to only set the user 
> > features, which are what I'm changing.
> > 
> > Rationale:
> > According to the API documentation, I cannot see a restriction that you 
> > are only allowed to call setFeature and setProperty before the first 
> > call to build.  Therefore I believe the code changes I proposed here 
> > better match the API documentation and expected behavior.
> > 
> > Jason Winnebeck