Skip to content

[Feature] Use Apache Commons Secure XML for the HTTP collector XML parsers #4401

Description

@ppkarwasz

Feature Request

Obtain the JAXP factories in HttpCollectImpl from Apache Commons Secure XML instead of configuring security features by hand.

Is your feature request related to a problem? Please describe

HttpCollectImpl parses XML returned by monitored endpoints, which is untrusted input, in two places, and each configures its own subset of security features:

Neither is wrong today, but this kind of per-call-site configuration tends to drift: every new parser has to remember the full list, and whether a feature is honored depends on the JAXP implementation on the classpath.

Describe the solution you'd like

Replace DocumentBuilderFactory.newInstance() and XPathFactory.newInstance() with SecureDocumentBuilderFactory.newInstance() and SecureXPathFactory.newInstance(), and drop the manual feature settings. Commons Secure XML installs non-removable resolvers that ignore external resources (DTDs, external entities, XInclude), and it fails loudly if the underlying implementation cannot be secured. Its guarantees are documented in the threat model and tested across the JDK, Xerces, Woodstox and Saxon.

To keep raw factories from coming back, a forbidden-apis check can reject the plain JAXP factory methods in main code. It needs no XXE unit tests in Hertzbeat, since those would only re-test the library.

The only behavior change: a response containing a DOCTYPE is no longer rejected; it is parsed, and any external references in it are ignored.

Describe alternatives you've considered

Keeping the handwritten configuration and aligning the two call sites. It works, but it leaves the protection dependent on each call site and on the parser implementation.

Additional context

The library is Java 8+, has no runtime dependencies and is licensed under Apache-2.0. I have a PR ready.

Unrelated finding: XmlUtil.fromXml(String, TypeReference) checks !StringUtils.isEmpty(xml) instead of StringUtils.isEmpty(xml), so it returns null for every non-empty input.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions