refactor: Remove Xerces/xml-apis dependencies and modernize SAX readers

[tonygermano]

Removes the dependency on xercesImpl and xml-apis to resolve classloader conflicts and reduce security risks in Java 17+ environments. Refactors custom Reader classes to use the standard JDK XMLReader interface instead of extending the Apache Xerces SAXParser.

Changes:

• Dependency Removal: Removed xercesImpl and xml-apis from the build configuration.
• New Base Class: Introduced AbstractXMLReader to centralize common XMLReader logic (handler management, feature/property compliance) and reduce code duplication across plugins.
• Reader Refactoring: Updated DelimitedReader, EDIReader, ER7Reader, and NCPDPReader:
  • Replaced inheritance (extends SAXParser) with extends AbstractXMLReader.
  • Added standard parse(String) overload via the base class.
  • Implemented strict SAX2 compliance for getFeature/setFeature to correctly report namespace support to downstream transformers.
• Code Cleanup: Removed unused imports of org.apache.xerces.parsers.SAXParser.
• Documentation Cleanup: Removed references to removed libraries in THIRD-PARTY-README.txt

This change ensures the application uses the built-in JDK XML parser, reducing the artifact size and aligning with modern Java best practices.

[Copilot]

Pull request overview

This PR successfully removes the Xerces and xml-apis dependencies from the codebase, replacing them with a custom AbstractXMLReader base class that provides standard JDK XMLReader functionality. This modernization eliminates classloader conflicts, reduces security risks, and decreases artifact size by leveraging the built-in Java XML parser.

Key changes:

• Introduced AbstractXMLReader to centralize XMLReader interface boilerplate and provide strict SAX2 compliance
• Refactored 4 custom reader classes (DelimitedReader, EDIReader, ER7Reader, NCPDPReader) to extend AbstractXMLReader instead of Xerces SAXParser
• Removed xercesImpl and xml-apis JARs from both client and server dependencies

Reviewed changes

Copilot reviewed 8 out of 12 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
server/src/org/openintegrationengine/engine/plugins/datatypes/AbstractXMLReader.java	New base class providing XMLReader interface implementation with handler management and SAX2 feature compliance
server/src/com/mirth/connect/plugins/datatypes/ncpdp/NCPDPReader.java	Refactored to extend AbstractXMLReader, added ensureHandlerSet() call, updated license header
server/src/com/mirth/connect/plugins/datatypes/hl7v2/ER7Reader.java	Refactored to extend AbstractXMLReader, added ensureHandlerSet() call, updated license header
server/src/com/mirth/connect/plugins/datatypes/edi/EDIReader.java	Refactored to extend AbstractXMLReader, added ensureHandlerSet() call, updated license header
server/src/com/mirth/connect/plugins/datatypes/delimited/DelimitedReader.java	Refactored to extend AbstractXMLReader, added ensureHandlerSet() call, removed unused ContentHandler import, updated license header
server/docs/thirdparty/THIRD-PARTY-README.txt	Removed documentation for xml-apis and Xerces dependencies
server/.classpath	Removed xercesImpl-2.12.2.jar and xml-apis-1.4.01.jar entries
client/.classpath	Removed xercesImpl-2.12.2.jar and xml-apis-1.4.01.jar entries

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[mgaffigan]

I'll approve after I can kick the tires a bit - nothing obvious apart from the bool comparisons copilot already called out.

I assume this is a breaking change for any data type plugin. Should be called out in release notes.

[tonygermano]

I'll approve after I can kick the tires a bit - nothing obvious apart from the bool comparisons copilot already called out.

I assume this is a breaking change for any data type plugin. Should be called out in release notes.

Yes, it would be a breaking change for anything that specifically imports from packages below org.apache.xerces or perhaps relies on behavior specific to the older parsers or transformers. If another datatype had cloned one of these as a starting point it would be affected. These are all datatypes that convert from something that isn't xml to xml. It doesn't affect the DICOM or strict hl7 types since those use dcm4che and hapi for the xml conversion rather than something implemented in this project.

I am going to have follow-up PRs which are dependent on this one to remove other outdated xml dependencies, but this one had to go first since the standalone xerces release only supports JAXP version 1.4, which is what was bundled with Java 6. The internal implementation has been at JAXP 1.6 since Java 8. With this jar on the classpath, standard javax.xml.parsers factory classes were utilizing the older implementation which didn't recognize current features.

To test this I created channels where the source inbound type was the target datatype and the outbound type was XML. The destination reversed this and went from xml to the target type. I created a sample message and verified the transformations to and from xml so the final output was the same as the original. I did this for delimited, hl7v2, and edi. I did not know where to find a sample message to test NCPDP, but I assume it's fine since the others all worked.

The bool comparison thing I went back and forth on before I opened the PR. I decided to keep it with the equality comparison since the parameter represents a value to be set, and is not meant for flow control, so it made more sense to me to do the explicit comparison as I would do for any value in a key/val pair that wasn't a boolean. I don't have a strong preference either way, though.

[mgaffigan]

Tested by creating type to xml channels on Mirth 4.4.0 and this branch. Compared results given same source messages.

Here's a NCPDP Telecom Sample File. Note: File includes non-printable characters.

[kayyagari]

Built with Java 17 and tested using a channel with both inbound and outbound datatypes set to a combination of Hl7v2.x and XML.

[NicoPiel]

Since the motivation includes “correctly report namespace support to downstream transformers,” can we add a regression test that runs a reader through the actual transformation path (SAXSource/Transformer) to confirm no additional feature/property probes break execution?

[tonygermano]

@NicoPiel

Since the motivation includes “correctly report namespace support to downstream transformers,” can we add a regression test that runs a reader through the actual transformation path (SAXSource/Transformer) to confirm no additional feature/property probes break execution?

"correctly report namespace support to downstream transformers" is alluding to the fact that the XMLReader.setFeature method states:

All XMLReaders are required to support setting http://xml.org/sax/features/namespaces to true and http://xml.org/sax/features/namespace-prefixes to false.

These features affect the argument values used when the XMLReader calls ContentHandler.startElement and ContentHandler.endElement. None of the plugins that extend this class actually read XML, but read other file types and convert them to xml, so nearly all xml features and properties will be unsupported. The existing plugins were hard coded (for the most part) to respond as if the two required features are in their minimum required state. Subclasses that wish to add support for additional features or properties can override the methods in this abstract class.

The main purpose of this PR was to remove the imports of org.apache.xerces.parsers.SAXParser. The proper way to do this seemed to be to implement XMLReader instead of extending org.apache.xerces.parsers.SAXParser. I created the abstract class to avoid having the same boilerplate implementations in all four plugins.

can we add a regression test that runs a reader through the actual transformation path (SAXSource/Transformer) to confirm no additional feature/property probes break execution?

I pushed some additional updates that now include tests for the new class and some additional changes after reading more of the interface javadocs to make sure the plugins follow the contract (mainly passing an empty Attributes object instead of null.) I think the current behavior of throwing exceptions for any probes for unsupported options is correct per the spec, and to pretend that we support a feature/property that we don't support could be worse for compatibility.

When merging I plan to squash the first 4 commits and keep the last one separate.

[jonbartels]

but read other file types and convert them to xml, so nearly all xml features and properties will be unsupported.

How to say this ... Does Mirth do anything stupiud or non-standard when it renders XML with Xerces that we have to either explicitly deprecate, explicitly test, or explicitly document as no longer supported?

IOW it's not doing anything hacky with Xerces.

I searched for code references and talked to the AI a bit. It does not seem to do anything custom with Xerces.

[jonbartels]

I'm arguing with JARs to render server/code-coverage-reports/jacoco.exec to HTML. My goal being to see if the code BEFORE the change covered this code and if it was covered AFTER the change.

I have to put it down for the night but it looks like I have to get a different CLI tool to view it.

EDIT - figured it out

Code coverage on these classes isn't good enough for my strategy to provide any useful insight

HOWTO:
Download the ZIP from https://www.jacoco.org/jacoco/

From repo root run:

code-coverage-reports/jacoco.exec \
--html code-coverage-reports \
 --classfiles classes```