BasicParserPool is not correctly resetting builder ErrorHandler and EntityResolver on second and subsequent checkout
Description
Environment
Activity
Brent Putman March 14, 2015 at 1:57 AM
Done, JSPT-56.
Scott Cantor March 14, 2015 at 1:35 AM
I think we should, otherwise we're just pushing null checks out all over the code.
I don't know if having error() just log on a schema error would somehow avoid a null document being returned. It seems like the parser can't be depending on the pluggable error handler to decide whether to actually return a document, but this stuff is so badly designed who knows.
Brent Putman March 14, 2015 at 1:32 AM
Ok, yeah, we crossed streams. I guess we could enforce the @Nonnull that way.
Brent Putman March 14, 2015 at 1:31 AM
Both of the ParserPool parse(...) methods are marked @Nonull. Clearly that is dependent on the DocumentBuilder ErrorHandler in use. Maybe that interface contract is not enforceable.
Scott Cantor March 14, 2015 at 1:30 AM
And I guess the schema validation of metadata is totally different and not during the parse, so we have control over the result there.
One thing I think we should do is defend the return statement in the parse() method and make sure it never returns null no matter what.
Bug report from dev list.
On creating a new builder, createBuilder() sets the entity resolver and error handler.
On check-in, these get reset via DocumentBuilder#reset(), and are never set again.
Should ensure that we always set these 2 builder properties before returning a builder from the pool.