Skip to content

Recognize Android parsers in Builder's internal parser list#518

Draft
Copilot wants to merge 3 commits into
masterfrom
copilot/recognize-android-parsers
Draft

Recognize Android parsers in Builder's internal parser list#518
Copilot wants to merge 3 commits into
masterfrom
copilot/recognize-android-parsers

Conversation

Copilot AI commented May 27, 2026

Copy link
Copy Markdown
Contributor

On Android, XMLReaderFactory.createXMLReader() returns parsers like org.kxml2.io.KXmlParser or org.xml.sax.helpers.XMLReaderFactory$Driver that aren't in XOM's known-parser list, and these parsers don't support the external-general-entities/external-parameter-entities SAX features — causing setupParser to throw and Builder construction to fail entirely on Android.

Changes

  • Builder.parsers array — adds org.kxml2.io.KXmlParser and org.xml.sax.helpers.XMLReaderFactory$Driver so they are recognized by findParser and knownGoodParser

  • setupParser — adds a named special-case branch for the three known Android parsers (org.kxml2.io.KXmlParser, org.xml.sax.helpers.XMLReaderFactory$Driver, org.xmlpull.v1.sax2.Driver) that skips setting the external entity features for those parsers only, consistent with the existing Crimson workaround pattern; all other parsers still require external entity support

// Android parsers do not support external entity features; skip setting them.
else if (parserName.equals("org.kxml2.io.KXmlParser")
  || parserName.equals("org.xml.sax.helpers.XMLReaderFactory$Driver")
  || parserName.equals("org.xmlpull.v1.sax2.Driver")) {
    // skip
}
else {
    parser.setFeature("http://xml.org/sax/features/external-general-entities", true);
    parser.setFeature("http://xml.org/sax/features/external-parameter-entities", true);
}
  • BuilderTesttestNonEntityResolvingParserException is unchanged: parsers that explicitly reject external entity features are still rejected with XMLException

Copilot AI changed the title [WIP] Recognize Android parsers in internal parser list Recognize Android parsers in Builder's internal parser list May 27, 2026
Copilot finished work on behalf of elharo May 27, 2026 10:52
Copilot AI requested a review from elharo May 27, 2026 10:52
Comment thread src/nu/xom/tests/BuilderTest.java Outdated

public void testNonEntityResolvingParserException() throws SAXException {
// A parser that does not support external entity features is now
// accepted, since platforms such as Android may not support them.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot this is a problem. are we sure about that?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right — the blanket try-catch was too permissive. Revised in the latest commit to use named special cases for the three known Android parsers (org.kxml2.io.KXmlParser, org.xml.sax.helpers.XMLReaderFactory$Driver, org.xmlpull.v1.sax2.Driver), consistent with how Crimson is handled. The original testNonEntityResolvingParserException is restored — parsers that explicitly reject external entity features are still rejected.

Copilot AI requested a review from elharo May 27, 2026 11:06
Copilot finished work on behalf of elharo May 27, 2026 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Recognize Android parsers in Builder's internal parser list

2 participants