OPENNLP-1957: Use Apache Commons Secure XML for JAXP factory creation by ppkarwasz · Pull Request #1306 · apache/opennlp

ppkarwasz · 2026-09-16T06:59:09Z

Resolves OPENNLP-1957.

What

XmlUtil obtains its DocumentBuilder, SAXParser, XMLStreamReader and XPath from Apache Commons Secure XML instead of applying a hand-maintained list of vendor-specific features. The dictionary serializer uses SecureTransformerFactory directly. Existing callers are unchanged.
New tests check that external entities are not resolved through the DOM, SAX or StAX path, that entity expansion is bounded, that the SAX parser is namespace-aware and that neither parser is XInclude-aware.
forbiddenapis now rejects the static factory methods of every JAXP parsing factory, so a new XML entry point cannot bypass the secured ones. The plugin was bound to validate, which runs before compilation, so it was skipped on every clean build including CI. It now runs in verify.
SECURITY.md points at the library's threat model.

Why

After CVE-2017-12620 and CVE-2026-40682, each new XML entry point had to repeat the same OWASP checklist with per-platform fallbacks. Delegating to a component built for secure JAXP factory creation means fixes for new parser features or implementations arrive with a dependency bump.

Behavioral changes

DOCTYPE declarations are parsed instead of rejected. External fetches are blocked by the library's entity-resolver floor and internal expansion is bounded by secure processing.
setExpandEntityReferences(false) is no longer called. The setting is not a security control: it only changes the shape of the DOM, leaving each reference as an EntityReference node whose replacement text consumers may never see. It stops external entity expansion in the built-in JDK implementation only through a bug, which stops internal entity expansion as well. External entities are now blocked by the library's entity-resolver floor, and internal ones are expanded inline, bounded by secure processing.
A non-Android JAXP implementation that cannot enable secure processing fails with IllegalStateException instead of logging a warning. Android factories are passed through, as before.
XInclude stays disabled.

The new XmlUtil.createXmlStreamReader(InputStream) is meant for the WN-LMF reader of OPENNLP-1880 (#1155). It returns a coalescing reader from a single shared factory and leaves the caller's stream open, so the reader can drop both its hardenedFactory() and its NonClosingInputStream. One difference to note there: the secure factory resolves an external entity to empty content instead of throwing, unless the org.apache.commons.xml.secure.throwOnUnresolved system property is set.

🤖 Generated with Claude Code

Replace the hand-rolled XXE hardening in `XmlUtil` with the secure-by-default factories of Apache Commons Secure XML, which enables secure processing and blocks external DTD, entity, schema and XInclude fetches on every supported JAXP implementation, including Android. The public API of `XmlUtil` is unchanged; callers in `opennlp-runtime` and `opennlp-formats` need no changes. New tests check that external entities are not resolved through either the DOM or the SAX path, that entity expansion is bounded, that the SAX parser is namespace-aware and that neither parser is XInclude-aware. Assisted-By: Claude Fable 5.1 <noreply@anthropic.com>

Add forbiddenapis signatures for the static factory methods of `DocumentBuilderFactory`, `SAXParserFactory`, `XMLInputFactory`, `TransformerFactory`, `SchemaFactory`, `XPathFactory` and `XMLReaderFactory`, so that every parser goes through `XmlUtil` or Apache Commons Secure XML. Migrate the remaining call sites: `XmlUtil.createXPath()` replaces the `XPathFactory` lookups in `GeneratorFactory` and the NKJP readers, the dictionary serializer uses `SecureTransformerFactory` directly, and the tests use the secure factories as well. The plugin was bound to the `validate` phase, which runs before compilation, so on a clean checkout such as CI it skipped every module. It now runs in its default `verify` phase. Assisted-By: Claude Fable 5.1 <noreply@anthropic.com>

The `commons-secure-xml.version` property was indented with spaces in a tab-indented file. The closing `]]></signatures>` tag of the forbiddenapis block moves to column zero, matching the signature lines it closes. Assisted-By: Claude Fable 5.1 <noreply@anthropic.com>

jzonthemtn

Is commons-secure-xml a new ASF project?

garydgregory · 2026-09-16T13:04:45Z

Hi @jzonthemtn
Yes, more specifically, it is a new component of the Apache Commons project.

We recently released 1.0.0 and expected a 1.0.1 in a week or 2 based on integration feedback so far.

krickert

LGTM

mawiesne

Thx @ppkarwasz for providing this PR.

mawiesne · 2026-09-17T06:46:32Z

The StAX part of the proposal waits for OPENNLP-1880.

FYI: There is no need to wait for OPENNLP-1880. Please let it do the StAX part. 1880 PR/changes should adapt to your set of changes. So Claudius can proceed and give us the 2nd part.

rzo1

Plz check the Android comment by @mawiesne above :)

rzo1

Thanks for clarification. I am fine with it.

`XmlUtil.createXmlInputFactory()` returns a Commons Secure XML `XMLInputFactory`, so the WN-LMF reader of OPENNLP-1880 can drop its hand-rolled hardening and rely on the same entity-resolver floor as the DOM and SAX parsers. The factory keeps the StAX defaults, so callers can still set properties such as `IS_COALESCING`. Tests check that a StAX reader does not resolve external entities, that internal entities still expand, and that caller properties are honoured. Assisted-By: Claude Fable 5.1 <noreply@anthropic.com>

The class dates from 1.8.2, the CVE-2017-12620 fix release. The StAX and XPath factory methods are new in 3.0.0. Assisted-By: Claude Fable 5.1 <noreply@anthropic.com>

A caller who only needs a secure `XMLInputFactory` can call Commons Secure XML directly. What the WN-LMF reader of OPENNLP-1880 needs is a coalescing `XMLStreamReader` that leaves the caller's stream open, so `XmlUtil` now provides exactly that from a single shared factory, wrapping the stream in `UncloseableInputStream` because the JDK reader closes it on `close()`. The factory lives in a lazy holder class, so the DOM and SAX helpers still load on a platform without StAX. Assisted-By: Claude Fable 5.1 <noreply@anthropic.com>

ppkarwasz · 2026-09-17T12:14:38Z

In a412a10 I added a createXmlStreamReader method, as required by #1155.

Android does not have a StAX implementation, so the XMLInputFactory is initialized lazily.

ppkarwasz added 2 commits September 16, 2026 09:04

ppkarwasz force-pushed the OPENNLP-1957 branch from dd2e81d to 013db85 Compare September 16, 2026 07:04

rzo1 requested review from jzonthemtn, krickert, mawiesne and rzo1 and removed request for rzo1 September 16, 2026 08:31

rzo1 assigned ppkarwasz Sep 16, 2026

rzo1 approved these changes Sep 16, 2026

View reviewed changes

garydgregory reviewed Sep 16, 2026

View reviewed changes

Comment thread pom.xml Outdated

jzonthemtn approved these changes Sep 16, 2026

View reviewed changes

krickert approved these changes Sep 16, 2026

View reviewed changes

mawiesne approved these changes Sep 17, 2026

View reviewed changes

mawiesne reviewed Sep 17, 2026

View reviewed changes

Comment thread opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/XmlUtil.java

Comment thread pom.xml

mawiesne added java Pull requests that update Java code dependency labels Sep 17, 2026

rzo1 requested changes Sep 17, 2026

View reviewed changes

rzo1 approved these changes Sep 17, 2026

View reviewed changes

ppkarwasz added 3 commits September 17, 2026 12:58

fix: Add @SInCE tags to XmlUtil

2d318ef

The class dates from 1.8.2, the CVE-2017-12620 fix release. The StAX and XPath factory methods are new in 3.0.0. Assisted-By: Claude Fable 5.1 <noreply@anthropic.com>

mawiesne merged commit a568975 into apache:main Sep 17, 2026
10 checks passed

rzo1 mentioned this pull request Sep 17, 2026

Upcoming OpenNLP releases break Android: ObjectInputFilter in 2.5.11+ and R8 missing classes in 3.0 GrapheneOS/SpeechServices#32

Open

ppkarwasz deleted the OPENNLP-1957 branch September 17, 2026 14:58

Navigation Menu

OPENNLP-1957: Use Apache Commons Secure XML for JAXP factory creation - #1306

OPENNLP-1957: Use Apache Commons Secure XML for JAXP factory creation#1306
mawiesne merged 6 commits into
apache:mainfrom
ppkarwasz:OPENNLP-1957

ppkarwasz commented Sep 16, 2026 •

edited

Loading

Uh oh!

Uh oh!

jzonthemtn left a comment

Uh oh!

garydgregory commented Sep 16, 2026 •

edited

Loading

Uh oh!

krickert left a comment

Uh oh!

mawiesne left a comment

Uh oh!

Uh oh!

Uh oh!

mawiesne commented Sep 17, 2026

Uh oh!

rzo1 left a comment

Uh oh!

rzo1 left a comment

Uh oh!

ppkarwasz commented Sep 17, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

ppkarwasz commented Sep 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Why

Behavioral changes

Uh oh!

Uh oh!

jzonthemtn left a comment

Choose a reason for hiding this comment

Uh oh!

garydgregory commented Sep 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

krickert left a comment

Choose a reason for hiding this comment

Uh oh!

mawiesne left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mawiesne commented Sep 17, 2026

Uh oh!

rzo1 left a comment

Choose a reason for hiding this comment

Uh oh!

rzo1 left a comment

Choose a reason for hiding this comment

Uh oh!

ppkarwasz commented Sep 17, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

ppkarwasz commented Sep 16, 2026 •

edited

Loading

garydgregory commented Sep 16, 2026 •

edited

Loading