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

Fixing Xerces dtd/schema backward compatibility #307

Merged
merged 1 commit into from
Sep 26, 2020

Conversation

the-thing
Copy link
Contributor

Fixes #306

Changes

  • data dictionary blocks external dtd/schema properties only when using JDK Xerces implementation
  • removed dtd/schema properties from quickfix.Message#toXML(quickfix.DataDictionary) - it is not required as we are neither loading, nor validation against schemas
  • unit tests

Comments

  • dtd/schema properties probably are not required in org.quickfixj.codegenerator.MessageCodeGenerator as it uses files that belong to the codebase, so changes pointing to external dtd/schema would be visible in PRs (the only reason I can think to keep it there is if somebody is dependent on org.quickfixj:quickfixj-codegenerator directly in their project to customise message generation)
  • this change fixes Xerces compatiblity problem, but if external Xerces dependency is being used, there is not easy way to supply javax.xml.parsers.DocumentBuilderFactory with custom properties or features (https://xerces.apache.org/xerces2-j/features.html, https://xerces.apache.org/xerces2-j/properties.html). Additional changes have to be made to propagate javax.xml.parsers.DocumentBuilderFactory across the board.

@chrjohn chrjohn added this to the QFJ 2.2.1 milestone Aug 31, 2020
@chrjohn chrjohn merged commit 1618a09 into quickfix-j:master Sep 26, 2020
@the-thing the-thing deleted the xml_security branch October 20, 2020 11:35
@chrjohn chrjohn linked an issue Dec 22, 2020 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants