Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CONFSERVER-82911 Back-port https://github.com/hunterhacker/jdom/pull/188 #1

Merged
merged 2 commits into from
Jul 18, 2023

Conversation

kennymacleod
Copy link
Collaborator

@kennymacleod kennymacleod commented Jul 18, 2023

Back-porting the above PR from JDOM 2.x upstream.

It's not a simple cherry-pick, since there have been too many intervening code changes, but the essence of it is simple.

The bug is that if client code sets the http://xml.org/sax/features/external-general-entities feature on the SAXBuilder, this can be silently over-written based on the value of the expand property of SAXBuilder. The upstream PR moves the logic which sets the features from before the expand logic, to after after.

There's also the added unit test, which verifies the fix.

String name = (String)iter.next();
Boolean value = (Boolean)features.get(name);
internalSetFeature(parser, name, value.booleanValue(), name);
}

Choose a reason for hiding this comment

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

Should also remove the redundant setting of user-specified features from above this

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done. I didn't do that initially just to minimise the code change, but that's ok.

Copy link

@rjatkins rjatkins left a comment

Choose a reason for hiding this comment

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

One fix to SAXBuilder to be done, and we're good to go with this

@kennymacleod kennymacleod merged commit 6fde4f3 into jdom-1.x Jul 18, 2023
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.

2 participants