mbeckerle commented on a change in pull request #519:
URL: https://github.com/apache/daffodil/pull/519#discussion_r628389004
##########
File path: daffodil-cli/src/main/scala/org/apache/daffodil/Main.scala
##########
@@ -807,7 +807,18 @@ object Main extends Logging {
case Left(bytes) => new ByteArrayInputStream(bytes)
case Right(is) => is
}
- scala.xml.XML.load(is)
+ val parser: SAXParser = {
+ val f = SAXParserFactory.newInstance()
+ f.setNamespaceAware(false)
+ val p = f.newSAXParser()
+ p
+ }
+ //
+ // FIXME: This needs to use a Daffodil-created loader, so that we can
+ // insure that the XMLReader used from it has the setSecureDefaults
treatment.
+ //
+ System.err.println("FIXME: Needs to use a Daffodil-created loader.")
Review comment:
Still to fix.
##########
File path:
daffodil-lib/src/main/scala/org/apache/daffodil/externalvars/ExternalVariablesXMLValidator.scala
##########
@@ -17,22 +17,31 @@
package org.apache.daffodil.externalvars
+import org.apache.daffodil.util.Misc
+import org.apache.daffodil.xml.DFDLCatalogResolver
+import org.apache.daffodil.xml.XMLUtils
+import org.apache.xerces.jaxp.validation.XMLSchemaFactory
+
import javax.xml.transform.stream.StreamSource
import org.xml.sax.SAXException
+
import java.io.File
object ExternalVariablesValidator {
final val extVarXsd = {
- val stream = this.getClass().getResourceAsStream("/xsd/dafext.xsd")
+ val uri = Misc.getRequiredResource("org/apache/daffodil/xsd/dafext.xsd")
+ val stream = uri.toURL.openStream()
stream
}
def validate(xmlFile: File): Either[java.lang.Throwable, _] = {
try {
- val factory = new org.apache.xerces.jaxp.validation.XMLSchemaFactory()
+ val factory = new XMLSchemaFactory()
+ factory.setResourceResolver(DFDLCatalogResolver.get)
val schema = factory.newSchema(new StreamSource(extVarXsd))
val validator = schema.newValidator()
+ validator.setFeature(XMLUtils.XML_DISALLOW_DOCTYPE_FEATURE, true)
Review comment:
move to XMLUtils.setSecureDefaults(v: Validator) ??
##########
File path:
daffodil-lib/src/main/scala/org/apache/daffodil/api/DaffodilSchemaSource.scala
##########
@@ -109,6 +113,27 @@ class URISchemaSource protected (val fileOrResource: URI)
extends DaffodilSchema
}
}
+/**
+ * Convenient for testing. Allows creating XML strings for loading that
contain things
+ * not supported by scala XML syntax like DOCTYPE decls.
+ *
+ * @param str - string that is loaded as XML.
+ */
+case class StringSchemaSource(str: String)
Review comment:
If for testing only, can live in src/test/scala
##########
File path:
daffodil-runtime1/src/main/scala/org/apache/daffodil/externalvars/ExternalVariablesLoader.scala
##########
@@ -66,13 +68,15 @@ object ExternalVariablesLoader {
def fileToBindings(file: File): Queue[Binding] = {
Assert.usage(file ne null)
- ExternalVariablesValidator.validate(file) match {
+ val validatorResult = ExternalVariablesValidator.validate(file)
+ validatorResult match {
case Left(ex) => Assert.abort(ex)
case Right(_) => // Success
}
val enc = determineEncoding(file) // The encoding is needed for
ConstructingParser
val input = scala.io.Source.fromURI(file.toURI)(enc)
- val node = ConstructingParser.fromSource(input, true).document.docElem
Review comment:
Add comment that this was changed from the Constructing loader because
of a unexpected error, and reusing the DaffodlXMLLoader works, no need for a
special case loader for external variable bindings.
##########
File path: daffodil-lib/src/main/scala/org/apache/daffodil/xml/XMLUtils.scala
##########
@@ -450,6 +450,64 @@ object XMLUtils {
val SAX_NAMESPACES_FEATURE = "http://xml.org/sax/features/namespaces";
val SAX_NAMESPACE_PREFIXES_FEATURE =
"http://xml.org/sax/features/namespace-prefixes";
+ /**
+ * Always enable this feature (which disables doctypes).
+ */
+ val XML_DISALLOW_DOCTYPE_FEATURE =
"http://apache.org/xml/features/disallow-doctype-decl";
+
+ /**
+ * Always disable this. Might not be necessary if doctypes are disallowed.
+ */
+ val XML_EXTERNAL_PARAMETER_ENTITIES_FEATURE =
"http://xml.org/sax/features/external-parameter-entities";
+
+ /**
+ * Always disable this. Might not be necessary if doctypes are disallowed.
+ */
+ val XML_EXTERNAL_GENERAL_ENTITIES_FEATURE =
"http://xml.org/sax/features/external-general-entities";
+
+ /**
+ * Sets properties that disable insecure XML reader behaviors.
+ * @param xmlReader - the reader to change feature settings on.
+ */
+ def setSecureDefaults(xmlReader: XMLReader) : Unit = {
+ try {
+ xmlReader.setFeature(XMLUtils.XML_DISALLOW_DOCTYPE_FEATURE, true)
+ xmlReader.setFeature(XMLUtils.XML_EXTERNAL_PARAMETER_ENTITIES_FEATURE,
false)
Review comment:
Add comment that these next 2 restrictions are not strictly speaking
necessary, as they are implied by disallowing doctypes.
##########
File path: daffodil-lib/src/main/scala/org/apache/daffodil/xml/XMLUtils.scala
##########
@@ -450,6 +450,64 @@ object XMLUtils {
val SAX_NAMESPACES_FEATURE = "http://xml.org/sax/features/namespaces";
val SAX_NAMESPACE_PREFIXES_FEATURE =
"http://xml.org/sax/features/namespace-prefixes";
+ /**
+ * Always enable this feature (which disables doctypes).
+ */
+ val XML_DISALLOW_DOCTYPE_FEATURE =
"http://apache.org/xml/features/disallow-doctype-decl";
+
+ /**
+ * Always disable this. Might not be necessary if doctypes are disallowed.
+ */
+ val XML_EXTERNAL_PARAMETER_ENTITIES_FEATURE =
"http://xml.org/sax/features/external-parameter-entities";
+
+ /**
+ * Always disable this. Might not be necessary if doctypes are disallowed.
+ */
+ val XML_EXTERNAL_GENERAL_ENTITIES_FEATURE =
"http://xml.org/sax/features/external-general-entities";
+
+ /**
+ * Sets properties that disable insecure XML reader behaviors.
+ * @param xmlReader - the reader to change feature settings on.
+ */
+ def setSecureDefaults(xmlReader: XMLReader) : Unit = {
+ try {
+ xmlReader.setFeature(XMLUtils.XML_DISALLOW_DOCTYPE_FEATURE, true)
+ xmlReader.setFeature(XMLUtils.XML_EXTERNAL_PARAMETER_ENTITIES_FEATURE,
false)
+ xmlReader.setFeature(XMLUtils.XML_EXTERNAL_GENERAL_ENTITIES_FEATURE,
false)
+ // System.err.println("SAX XMLReader supports disallow properties: " +
xmlReader)
+ } catch {
+ case e: SAXNotRecognizedException => {
+ // System.err.println("xmlReader: " + e.getMessage()) // good place
for a breakpoint
+ throw e
+ }
+ }
+ }
+
+// def setSecureDefaults(schemaFactory:
org.apache.xerces.jaxp.validation.XMLSchemaFactory) : Unit = {
Review comment:
Remove commented code. Add comment above that the disallowing of
doctypes works for XMLReader and Validator, but not XMLSchemaFactory.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
