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

[Jason Winnebeck]

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