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.

Sign up for GitHub

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

Jump to bottom

Unnecessary XML validation #1622

Merged
merged 1 commit into from
Sep 9, 2013
Merged

Unnecessary XML validation #1622

merged 1 commit into from
Sep 9, 2013

Conversation

huntc
Copy link
Contributor

@huntc huntc commented Sep 9, 2013

XML validation against a schema is not required within Play and could present an unnecessary performance overhead. Places where XML validation is performed are:

  • L422 of ContentTypes.scala
  • L662 of WS.scala

Recommendation in terms of establishing a loader:

val f = javax.xml.parsers.SAXParserFactory.newInstance()
f.setValidating(false)
val p = f.newSAXParser()
val nonValidatingXml = xml.XML.withSAXParser(p)

...and when required

val doc = nonValidatingXml.load(url)

@ghost ghost assigned huntc Sep 9, 2013
@cloudbees-pull-request-builder
Copy link

play2-PRs #683 SUCCESS
This pull request looks good

richdougherty
richdougherty reviewed Sep 9, 2013
View reviewed changes
framework/src/play/src/main/scala/play/api/Play.scala
*/
private[play] val xercesSaxParserFactory =
SAXParserFactory.newInstance("org.apache.xerces.jaxp.SAXParserFactoryImpl", Play.getClass.getClassLoader)
xercesSaxParserFactory.setFeature(Constants.SAX_FEATURE_PREFIX + Constants.EXTERNAL_GENERAL_ENTITIES_FEATURE, false)
Copy link
Member

Choose a reason for hiding this comment

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

Nit: indent

@richdougherty
Copy link
Member

LGTM 👍

jroper added a commit that referenced this pull request Sep 9, 2013
@jroper
Merge pull request #1622 from huntc/disallow-doctype-decls
c508103 
Unnecessary XML validation
@jroper jroper merged commit c508103 into playframework:master Sep 9, 2013
richdougherty added a commit to richdougherty/playframework that referenced this pull request Sep 10, 2013
@richdougherty
Ensures that DTDs do not cause resources to be downloaded
6e6a3cc 
(Backport of playframework#1622/dc94b94 from master)
@richdougherty richdougherty mentioned this pull request Sep 10, 2013
Merged
richdougherty added a commit that referenced this pull request Sep 10, 2013
@richdougherty @jroper
Ensures that DTDs do not cause resources to be downloaded
fef5f01 
(Backport of #1622/dc94b94 from master)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@huntc huntc

Labels
None yet
Projects
None yet
Milestone
2.2.0
Development

Successfully merging this pull request may close these issues.
4 participants
@huntc @cloudbees-pull-request-builder @richdougherty @jroper