Repository: oozie Updated Branches: refs/heads/master ef6d0af5e -> ba665da34
Fix parsing issue (satishsaley, asasvari) Project: http://git-wip-us.apache.org/repos/asf/oozie/repo Commit: http://git-wip-us.apache.org/repos/asf/oozie/commit/ba665da3 Tree: http://git-wip-us.apache.org/repos/asf/oozie/tree/ba665da3 Diff: http://git-wip-us.apache.org/repos/asf/oozie/diff/ba665da3 Branch: refs/heads/master Commit: ba665da34c23b1fa86bf1570724147e6f8c85b1d Parents: ef6d0af Author: Attila Sasvari <asasv...@cloudera.com> Authored: Sun Feb 4 11:51:41 2018 +0100 Committer: Attila Sasvari <asasv...@cloudera.com> Committed: Sun Feb 4 21:39:34 2018 +0100 ---------------------------------------------------------------------- .../command/bundle/BundleSubmitXCommand.java | 3 +- .../command/coord/CoordSubmitXCommand.java | 3 +- .../oozie/service/ConfigurationService.java | 24 +++-- .../oozie/service/HadoopAccessorService.java | 2 +- .../org/apache/oozie/service/SchemaService.java | 64 ++++++++++- .../org/apache/oozie/service/XLogService.java | 4 +- .../apache/oozie/servlet/V2ValidateServlet.java | 6 +- .../org/apache/oozie/util/XConfiguration.java | 106 ++++++++++++++----- .../java/org/apache/oozie/util/XmlUtils.java | 65 +++++------- .../workflow/lite/LiteWorkflowAppParser.java | 44 ++++---- .../apache/oozie/service/TestSchemaService.java | 8 +- .../apache/oozie/util/TestXConfiguration.java | 41 +++---- .../org/apache/oozie/util/TestXmlUtils.java | 16 ++- 13 files changed, 246 insertions(+), 140 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/oozie/blob/ba665da3/core/src/main/java/org/apache/oozie/command/bundle/BundleSubmitXCommand.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/oozie/command/bundle/BundleSubmitXCommand.java b/core/src/main/java/org/apache/oozie/command/bundle/BundleSubmitXCommand.java index 3a17234..dff2223 100644 --- a/core/src/main/java/org/apache/oozie/command/bundle/BundleSubmitXCommand.java +++ b/core/src/main/java/org/apache/oozie/command/bundle/BundleSubmitXCommand.java @@ -348,9 +348,8 @@ public class BundleSubmitXCommand extends SubmitTransitionXCommand { * @throws BundleJobException thrown if failed to validate xml */ private void validateXml(String xmlContent) throws BundleJobException { - javax.xml.validation.Schema schema = Services.get().get(SchemaService.class).getSchema(SchemaName.BUNDLE); - Validator validator = schema.newValidator(); try { + Validator validator = Services.get().get(SchemaService.class).getValidator(SchemaName.BUNDLE); validator.validate(new StreamSource(new StringReader(xmlContent))); } catch (SAXException ex) { http://git-wip-us.apache.org/repos/asf/oozie/blob/ba665da3/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java b/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java index fecf006..42bdc99 100644 --- a/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java +++ b/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java @@ -507,9 +507,8 @@ public class CoordSubmitXCommand extends SubmitTransitionXCommand { * @throws CoordinatorJobException thrown if unable to validate coordinator xml */ private void validateXml(String xmlContent) throws CoordinatorJobException { - javax.xml.validation.Schema schema = Services.get().get(SchemaService.class).getSchema(SchemaName.COORDINATOR); - Validator validator = schema.newValidator(); try { + Validator validator = Services.get().get(SchemaService.class).getValidator(SchemaName.COORDINATOR); validator.validate(new StreamSource(new StringReader(xmlContent))); } catch (SAXException ex) { http://git-wip-us.apache.org/repos/asf/oozie/blob/ba665da3/core/src/main/java/org/apache/oozie/service/ConfigurationService.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/oozie/service/ConfigurationService.java b/core/src/main/java/org/apache/oozie/service/ConfigurationService.java index 15076de..618d5e6 100644 --- a/core/src/main/java/org/apache/oozie/service/ConfigurationService.java +++ b/core/src/main/java/org/apache/oozie/service/ConfigurationService.java @@ -19,13 +19,15 @@ package org.apache.oozie.service; import com.google.common.base.Strings; +import com.google.common.annotations.VisibleForTesting; import org.apache.hadoop.conf.Configuration; +import org.apache.oozie.ErrorCode; import org.apache.oozie.util.ConfigUtils; import org.apache.oozie.util.Instrumentable; import org.apache.oozie.util.Instrumentation; -import org.apache.oozie.util.XLog; import org.apache.oozie.util.XConfiguration; -import org.apache.oozie.ErrorCode; +import org.apache.oozie.util.XLog; +import org.apache.oozie.util.ZKUtils; import java.io.File; import java.io.FileInputStream; @@ -34,15 +36,11 @@ import java.io.InputStream; import java.io.StringWriter; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.util.Arrays; import java.util.HashMap; import java.util.HashSet; import java.util.Map; import java.util.Set; -import java.util.Arrays; - -import org.apache.oozie.util.ZKUtils; - -import com.google.common.annotations.VisibleForTesting; import javax.xml.parsers.DocumentBuilderFactory; @@ -122,6 +120,13 @@ public class ConfigurationService implements Service, Instrumentable { // These properties should be masked when displayed because they contain sensitive info (e.g. password) MASK_PROPS.add(JPAService.CONF_PASSWORD); MASK_PROPS.add("oozie.authentication.signature.secret"); + + try { + Method method = Configuration.class.getDeclaredMethod("setRestrictSystemPropertiesDefault", boolean + .class); + method.invoke(null, true); + } catch( NoSuchMethodException | InvocationTargetException | IllegalAccessException ignore) { + } } public static final String DEFAULT_CONFIG_FILE = "oozie-default.xml"; @@ -303,6 +308,7 @@ public class ConfigurationService implements Service, Instrumentable { private XConfiguration loadConfig(InputStream inputStream, boolean defaultConfig) throws IOException, ServiceException { XConfiguration configuration; configuration = new XConfiguration(inputStream); + configuration.setRestrictSystemProperties(false); for(Map.Entry<String,String> entry: configuration) { if (defaultConfig) { defaultConfigs.put(entry.getKey(), entry.getValue()); @@ -322,6 +328,10 @@ public class ConfigurationService implements Service, Instrumentable { setValue(entry.getKey(), entry.getValue()); } } + if(conf instanceof XConfiguration) { + this.setRestrictParser(((XConfiguration)conf).getRestrictParser()); + this.setRestrictSystemProperties(((XConfiguration)conf).getRestrictSystemProperties()); + } } @Override http://git-wip-us.apache.org/repos/asf/oozie/blob/ba665da3/core/src/main/java/org/apache/oozie/service/HadoopAccessorService.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/oozie/service/HadoopAccessorService.java b/core/src/main/java/org/apache/oozie/service/HadoopAccessorService.java index 73300a6..05cdc55 100644 --- a/core/src/main/java/org/apache/oozie/service/HadoopAccessorService.java +++ b/core/src/main/java/org/apache/oozie/service/HadoopAccessorService.java @@ -266,7 +266,7 @@ public class HadoopAccessorService implements Service { File f = new File(dir, file); if (f.exists()) { InputStream is = new FileInputStream(f); - Configuration conf = new XConfiguration(is); + Configuration conf = new XConfiguration(is, false); is.close(); XConfiguration.copy(conf, hadoopConf); } http://git-wip-us.apache.org/repos/asf/oozie/blob/ba665da3/core/src/main/java/org/apache/oozie/service/SchemaService.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/oozie/service/SchemaService.java b/core/src/main/java/org/apache/oozie/service/SchemaService.java index 9d2a521..14d1eeb 100644 --- a/core/src/main/java/org/apache/oozie/service/SchemaService.java +++ b/core/src/main/java/org/apache/oozie/service/SchemaService.java @@ -28,11 +28,18 @@ import javax.xml.XMLConstants; import javax.xml.transform.stream.StreamSource; import javax.xml.validation.Schema; import javax.xml.validation.SchemaFactory; +import javax.xml.validation.Validator; import org.apache.oozie.ErrorCode; import org.apache.oozie.util.IOUtils; import org.apache.oozie.util.schema.ResourceResolver; +import org.apache.xerces.xni.XMLResourceIdentifier; +import org.apache.xerces.xni.XNIException; +import org.apache.xerces.xni.parser.XMLEntityResolver; +import org.apache.xerces.xni.parser.XMLInputSource; import org.xml.sax.SAXException; +import org.xml.sax.SAXNotRecognizedException; +import org.xml.sax.SAXNotSupportedException; /** @@ -75,6 +82,10 @@ public class SchemaService implements Service { private Schema slaSchema; + private SchemaFactory schemaFactory; + + private static NoXMLEntityResolver xmlEntityResolver; + private Schema loadSchema(String baseSchemas, String extSchema) throws SAXException, IOException { Set<String> schemaNames = new HashSet<String>(); String[] schemas = ConfigurationService.getStrings(baseSchemas); @@ -101,9 +112,8 @@ public class SchemaService implements Service { s.setSystemId(schemaName); sources.add(s); } - SchemaFactory factory = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI); - factory.setResourceResolver(new ResourceResolver()); - return factory.newSchema(sources.toArray(new StreamSource[sources.size()])); + schemaFactory.setResourceResolver(new ResourceResolver()); + return schemaFactory.newSchema(sources.toArray(new StreamSource[sources.size()])); } /** @@ -115,10 +125,12 @@ public class SchemaService implements Service { @Override public void init(Services services) throws ServiceException { try { + schemaFactory = createSchemaFactory(); wfSchema = loadSchema(WF_CONF_SCHEMAS, WF_CONF_EXT_SCHEMAS); coordSchema = loadSchema(COORD_CONF_SCHEMAS, COORD_CONF_EXT_SCHEMAS); bundleSchema = loadSchema(BUNDLE_CONF_SCHEMAS, BUNDLE_CONF_EXT_SCHEMAS); slaSchema = loadSchema(SLA_CONF_SCHEMAS, SLA_CONF_EXT_SCHEMAS); + xmlEntityResolver = new NoXMLEntityResolver(); } catch (SAXException ex) { throw new ServiceException(ErrorCode.E0130, ex.getMessage(), ex); @@ -129,6 +141,19 @@ public class SchemaService implements Service { } /** + * Creates schema factory + * @return + * @throws SAXNotRecognizedException + * @throws SAXNotSupportedException + */ + private SchemaFactory createSchemaFactory() throws SAXNotRecognizedException, SAXNotSupportedException { + SchemaFactory factory = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI); + factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl";, true); + factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING,true); + return factory; + } + + /** * Return the public interface of the service. * * @return {@link SchemaService}. @@ -188,4 +213,37 @@ public class SchemaService implements Service { return id; } } + + /** + * Returns validator for schema + * @param schemaName + * @return + * @throws SAXException + */ + public Validator getValidator(SchemaName schemaName) throws SAXException { + return getValidator(getSchema(schemaName)); + } + + /** + * Returns validator for schema + * @param schema + * @return + * @throws SAXException + */ + public static Validator getValidator(Schema schema) throws SAXException { + Validator validator = schema.newValidator(); + validator.setFeature("http://apache.org/xml/features/disallow-doctype-decl";, true); + validator.setFeature("http://xml.org/sax/features/external-general-entities";, false); + validator.setFeature("http://xml.org/sax/features/external-parameter-entities";, false); + validator.setProperty("http://apache.org/xml/properties/internal/entity-resolver";, xmlEntityResolver); + return validator; + } + + private static class NoXMLEntityResolver implements XMLEntityResolver { + @Override + public XMLInputSource resolveEntity(XMLResourceIdentifier xmlResourceIdentifier) throws XNIException, IOException { + throw new IOException("DOCTYPE is disallowed when the feature http://apache.org/xml/features/disallow-doctype-decl " + + "set to true."); + } + } } http://git-wip-us.apache.org/repos/asf/oozie/blob/ba665da3/core/src/main/java/org/apache/oozie/service/XLogService.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/oozie/service/XLogService.java b/core/src/main/java/org/apache/oozie/service/XLogService.java index 0b43722..1ac82f5 100644 --- a/core/src/main/java/org/apache/oozie/service/XLogService.java +++ b/core/src/main/java/org/apache/oozie/service/XLogService.java @@ -144,6 +144,7 @@ public class XLogService implements Service, Instrumentable { String oozieHome = Services.getOozieHome(); String oozieLogs = System.getProperty(OOZIE_LOG_DIR, oozieHome + "/logs"); System.setProperty(OOZIE_LOG_DIR, oozieLogs); + try { LogManager.resetConfiguration(); log4jFileName = System.getProperty(LOG4J_FILE, DEFAULT_LOG4J_PROPERTIES); @@ -211,7 +212,8 @@ public class XLogService implements Service, Instrumentable { Properties props = new Properties(); props.load(is); - Configuration conf = new XConfiguration(); + XConfiguration conf = new XConfiguration(); + conf.setRestrictSystemProperties(false); for (Map.Entry entry : props.entrySet()) { conf.set((String) entry.getKey(), (String) entry.getValue()); } http://git-wip-us.apache.org/repos/asf/oozie/blob/ba665da3/core/src/main/java/org/apache/oozie/servlet/V2ValidateServlet.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/oozie/servlet/V2ValidateServlet.java b/core/src/main/java/org/apache/oozie/servlet/V2ValidateServlet.java index 7bf8fb3..b86fa6a 100644 --- a/core/src/main/java/org/apache/oozie/servlet/V2ValidateServlet.java +++ b/core/src/main/java/org/apache/oozie/servlet/V2ValidateServlet.java @@ -137,8 +137,8 @@ public class V2ValidateServlet extends JsonRestServlet { } private void validateSchema(Schema schema, Reader src) throws SAXException, IOException, OozieCLIException{ - Validator validator = schema.newValidator(); - validator.validate(new StreamSource(src)); + Validator validator = SchemaService.getValidator(schema); + validator.validate(new StreamSource(src)); } private JSONObject createJSON(String content) { @@ -147,4 +147,4 @@ public class V2ValidateServlet extends JsonRestServlet { return jsonObject; } -} +} \ No newline at end of file http://git-wip-us.apache.org/repos/asf/oozie/blob/ba665da3/core/src/main/java/org/apache/oozie/util/XConfiguration.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/oozie/util/XConfiguration.java b/core/src/main/java/org/apache/oozie/util/XConfiguration.java index bd62cb1..e3591db 100644 --- a/core/src/main/java/org/apache/oozie/util/XConfiguration.java +++ b/core/src/main/java/org/apache/oozie/util/XConfiguration.java @@ -50,6 +50,8 @@ public class XConfiguration extends Configuration { public static final String CONFIGURATION_SUBSTITUTE_DEPTH = "oozie.configuration.substitute.depth"; + private boolean restrictSystemProperties = true; + private boolean restrictParser = true; /** * Create an empty configuration. <p> Default values are not loaded. */ @@ -62,25 +64,49 @@ public class XConfiguration extends Configuration { * Create a configuration from an InputStream. <p> Code canibalized from <code>Configuration.loadResource()</code>. * * @param is inputstream to read the configuration from. + * @param restrictParser whether to restrict the parser * @throws IOException thrown if the configuration could not be read. */ - public XConfiguration(InputStream is) throws IOException { + public XConfiguration(InputStream is, boolean restrictParser) throws IOException { this(); + this.restrictParser = restrictParser; parse(is); } /** + * Create a configuration from an InputStream. <p> Code canibalized from <code>Configuration.loadResource()</code>. + * + * @param is inputstream to read the configuration from. + * @throws IOException thrown if the configuration could not be read. + */ + public XConfiguration(InputStream is) throws IOException { + this(is, true); + } + + /** * Create a configuration from an Reader. <p> Code canibalized from <code>Configuration.loadResource()</code>. * * @param reader reader to read the configuration from. + * @param restrictParser whether to restrict the parser * @throws IOException thrown if the configuration could not be read. */ - public XConfiguration(Reader reader) throws IOException { + public XConfiguration(Reader reader, boolean restrictParser) throws IOException { this(); + this.restrictParser = restrictParser; parse(reader); } /** + * Create a configuration from an Reader. <p> Code canibalized from <code>Configuration.loadResource()</code>. + * + * @param reader reader to read the configuration from. + * @throws IOException thrown if the configuration could not be read. + */ + public XConfiguration(Reader reader) throws IOException { + this(reader, true); + } + + /** * Create an configuration from a Properties instance. * * @param props Properties instance to get all properties from. @@ -90,7 +116,6 @@ public class XConfiguration extends Configuration { for (Map.Entry entry : props.entrySet()) { set((String) entry.getKey(), (String) entry.getValue()); } - } /** @@ -123,7 +148,7 @@ public class XConfiguration extends Configuration { */ @Override public String get(String name) { - return substituteVars(getRaw(name)); + return substituteVars(getRaw(name)); } /** @@ -173,8 +198,9 @@ public class XConfiguration extends Configuration { var = var.substring(2, var.length() - 1); // remove ${ .. } String val = getRaw(var); - if (val == null) { - val = System.getProperty(var); + + if (val == null && !this.restrictSystemProperties) { + val = System.getProperty(var); } if (val == null) { @@ -253,16 +279,7 @@ public class XConfiguration extends Configuration { // Canibalized from Hadoop <code>Configuration.loadResource()</code>. private void parse(InputStream is) throws IOException { try { - DocumentBuilderFactory docBuilderFactory = DocumentBuilderFactory.newInstance(); - // support for includes in the xml file - docBuilderFactory.setNamespaceAware(true); - docBuilderFactory.setXIncludeAware(true); - // ignore all comments inside the xml file - docBuilderFactory.setIgnoringComments(true); - docBuilderFactory.setExpandEntityReferences(false); - docBuilderFactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl";, true); - DocumentBuilder builder = docBuilderFactory.newDocumentBuilder(); - Document doc = builder.parse(is); + Document doc = getDocumentBuilder().parse(is); parseDocument(doc); } @@ -277,16 +294,7 @@ public class XConfiguration extends Configuration { // Canibalized from Hadoop <code>Configuration.loadResource()</code>. private void parse(Reader reader) throws IOException { try { - DocumentBuilderFactory docBuilderFactory = DocumentBuilderFactory.newInstance(); - // support for includes in the xml file - docBuilderFactory.setNamespaceAware(true); - docBuilderFactory.setXIncludeAware(true); - // ignore all comments inside the xml file - docBuilderFactory.setIgnoringComments(true); - docBuilderFactory.setExpandEntityReferences(false); - docBuilderFactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl";, true); - DocumentBuilder builder = docBuilderFactory.newDocumentBuilder(); - Document doc = builder.parse(new InputSource(reader)); + Document doc = getDocumentBuilder().parse(new InputSource(reader)); parseDocument(doc); } catch (SAXException e) { @@ -306,7 +314,7 @@ public class XConfiguration extends Configuration { processNodes(root); } - // Canibalized from Hadoop <code>Configuration.loadResource()</code>. + // Cannibalized from Hadoop <code>Configuration.loadResource()</code>. private void processNodes(Element root) throws IOException { try { NodeList props = root.getChildNodes(); @@ -392,4 +400,48 @@ public class XConfiguration extends Configuration { } return values; } + + private DocumentBuilder getDocumentBuilder() throws ParserConfigurationException { + DocumentBuilderFactory docBuilderFactory = DocumentBuilderFactory.newInstance(); + docBuilderFactory.setNamespaceAware(true); + docBuilderFactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl",true); + docBuilderFactory.setXIncludeAware(true); + if(this.restrictParser) { + docBuilderFactory.setXIncludeAware(false); + //Redundant with disallow-doctype, but just in case + docBuilderFactory.setFeature("http://xml.org/sax/features/external-general-entities";, false); + docBuilderFactory.setFeature("http://xml.org/sax/features/external-parameter-entities";, false); + docBuilderFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd";, false); + } + docBuilderFactory.setExpandEntityReferences(false); + // ignore all comments inside the xml file + docBuilderFactory.setIgnoringComments(true); + return docBuilderFactory.newDocumentBuilder(); + } + + /** + * Restrict the parser + * @param restrictParser + */ + public void setRestrictParser(boolean restrictParser) { + this.restrictParser = restrictParser; + } + + public boolean getRestrictParser() { + return restrictParser; + } + /** + * Restrict reading property from System.getProperty() + * @param restrictSystemProperties + */ + public void setRestrictSystemProperties(boolean restrictSystemProperties) { + this.restrictSystemProperties = restrictSystemProperties; + } + + public boolean getRestrictSystemProperties() { + return restrictSystemProperties; + } + + + } http://git-wip-us.apache.org/repos/asf/oozie/blob/ba665da3/core/src/main/java/org/apache/oozie/util/XmlUtils.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/oozie/util/XmlUtils.java b/core/src/main/java/org/apache/oozie/util/XmlUtils.java index 6875eec..9db46b3 100644 --- a/core/src/main/java/org/apache/oozie/util/XmlUtils.java +++ b/core/src/main/java/org/apache/oozie/util/XmlUtils.java @@ -32,7 +32,9 @@ import java.util.Map; import java.util.Properties; import javax.xml.XMLConstants; +import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; +import javax.xml.parsers.ParserConfigurationException; import javax.xml.transform.Result; import javax.xml.transform.Source; import javax.xml.transform.Transformer; @@ -56,8 +58,6 @@ import org.jdom.Namespace; import org.jdom.input.SAXBuilder; import org.jdom.output.Format; import org.jdom.output.XMLOutputter; -import org.xml.sax.EntityResolver; -import org.xml.sax.InputSource; import org.xml.sax.SAXException; /** @@ -65,23 +65,12 @@ import org.xml.sax.SAXException; */ public class XmlUtils { - private static class NoExternalEntityEntityResolver implements EntityResolver { - - @Override - public InputSource resolveEntity(String publicId, String systemId) throws SAXException, IOException { - return new InputSource(new ByteArrayInputStream(new byte[0])); - } - - } - private static SAXBuilder createSAXBuilder() { SAXBuilder saxBuilder = new SAXBuilder(); - - //THIS IS NOT WORKING - //saxBuilder.setFeature("http://xml.org/sax/features/external-general-entities";, false); - - //INSTEAD WE ARE JUST SETTING AN EntityResolver that does not resolve entities - saxBuilder.setEntityResolver(new NoExternalEntityEntityResolver()); + saxBuilder.setFeature("http://apache.org/xml/features/disallow-doctype-decl",true); + saxBuilder.setFeature("http://xml.org/sax/features/external-general-entities";, false); + saxBuilder.setFeature("http://xml.org/sax/features/external-parameter-entities";, false); + saxBuilder.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd";, false); return saxBuilder; } @@ -281,28 +270,10 @@ public class XmlUtils { * @throws IOException in case of IO error */ public static void validateXml(Schema schema, String xml) throws SAXException, IOException { - - Validator validator = schema.newValidator(); + Validator validator = SchemaService.getValidator(schema); validator.validate(new StreamSource(new ByteArrayInputStream(xml.getBytes()))); } - /** - * Create schema object for the given xsd - * - * @param is inputstream to schema. - * @return the schema object. - */ - public static Schema createSchema(InputStream is) { - SchemaFactory factory = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI); - StreamSource src = new StreamSource(is); - try { - return factory.newSchema(src); - } - catch (SAXException e) { - throw new RuntimeException(e.getMessage(), e); - } - } - public static void validateData(String xmlData, SchemaName xsdFile) throws SAXException, IOException { if (xmlData == null || xmlData.length() == 0) { return; @@ -320,7 +291,7 @@ public class XmlUtils { */ public static String writePropToString(Properties props) throws IOException { try { - org.w3c.dom.Document doc = DocumentBuilderFactory.newInstance().newDocumentBuilder().newDocument(); + org.w3c.dom.Document doc = getDocumentBuilder().newDocument(); org.w3c.dom.Element conf = doc.createElement("configuration"); doc.appendChild(conf); conf.appendChild(doc.createTextNode("\n")); @@ -364,6 +335,26 @@ public class XmlUtils { } /** + * Returns a DocumentBuilder + * @return DocumentBuilder + * @throws ParserConfigurationException + */ + private static DocumentBuilder getDocumentBuilder() throws ParserConfigurationException { + DocumentBuilderFactory docBuilderFactory = DocumentBuilderFactory.newInstance(); + docBuilderFactory.setNamespaceAware(true); + docBuilderFactory.setXIncludeAware(false); + docBuilderFactory.setExpandEntityReferences(false); + docBuilderFactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl",true); + //Redundant with disallow-doctype, but just in case + docBuilderFactory.setFeature("http://xml.org/sax/features/external-general-entities";, false); + docBuilderFactory.setFeature("http://xml.org/sax/features/external-parameter-entities";, false); + docBuilderFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd";, false); + // ignore all comments inside the xml file + docBuilderFactory.setIgnoringComments(true); + return docBuilderFactory.newDocumentBuilder(); + } + + /** * Escape characters for text appearing as XML data, between tags. * <p> * The following characters are replaced with corresponding character entities : http://git-wip-us.apache.org/repos/asf/oozie/blob/ba665da3/core/src/main/java/org/apache/oozie/workflow/lite/LiteWorkflowAppParser.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/oozie/workflow/lite/LiteWorkflowAppParser.java b/core/src/main/java/org/apache/oozie/workflow/lite/LiteWorkflowAppParser.java index aa0e06b..c236daf 100644 --- a/core/src/main/java/org/apache/oozie/workflow/lite/LiteWorkflowAppParser.java +++ b/core/src/main/java/org/apache/oozie/workflow/lite/LiteWorkflowAppParser.java @@ -18,27 +18,6 @@ package org.apache.oozie.workflow.lite; -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; -import java.io.DataInput; -import java.io.DataInputStream; -import java.io.DataOutput; -import java.io.DataOutputStream; -import java.io.IOException; -import java.io.Reader; -import java.io.StringReader; -import java.io.StringWriter; -import java.util.ArrayList; -import java.util.List; -import java.util.zip.Deflater; -import java.util.zip.DeflaterOutputStream; -import java.util.zip.Inflater; -import java.util.zip.InflaterInputStream; - -import javax.xml.transform.stream.StreamSource; -import javax.xml.validation.Schema; -import javax.xml.validation.Validator; - import org.apache.commons.codec.binary.Base64; import org.apache.commons.lang.StringUtils; import org.apache.hadoop.conf.Configuration; @@ -49,6 +28,7 @@ import org.apache.oozie.action.hadoop.FsActionExecutor; import org.apache.oozie.action.oozie.SubWorkflowActionExecutor; import org.apache.oozie.service.ActionService; import org.apache.oozie.service.ConfigurationService; +import org.apache.oozie.service.SchemaService; import org.apache.oozie.service.Services; import org.apache.oozie.util.ELUtils; import org.apache.oozie.util.IOUtils; @@ -63,6 +43,26 @@ import org.jdom.JDOMException; import org.jdom.Namespace; import org.xml.sax.SAXException; +import javax.xml.transform.stream.StreamSource; +import javax.xml.validation.Schema; +import javax.xml.validation.Validator; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.DataInput; +import java.io.DataInputStream; +import java.io.DataOutput; +import java.io.DataOutputStream; +import java.io.IOException; +import java.io.Reader; +import java.io.StringReader; +import java.io.StringWriter; +import java.util.ArrayList; +import java.util.List; +import java.util.zip.Deflater; +import java.util.zip.DeflaterOutputStream; +import java.util.zip.Inflater; +import java.util.zip.InflaterInputStream; + /** * Class to parse and validate workflow xml */ @@ -166,7 +166,7 @@ public class LiteWorkflowAppParser { String strDef = writer.toString(); if (schema != null) { - Validator validator = schema.newValidator(); + Validator validator = SchemaService.getValidator(schema); validator.validate(new StreamSource(new StringReader(strDef))); } http://git-wip-us.apache.org/repos/asf/oozie/blob/ba665da3/core/src/test/java/org/apache/oozie/service/TestSchemaService.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/oozie/service/TestSchemaService.java b/core/src/test/java/org/apache/oozie/service/TestSchemaService.java index d0a3fdf..3ae6f6f 100644 --- a/core/src/test/java/org/apache/oozie/service/TestSchemaService.java +++ b/core/src/test/java/org/apache/oozie/service/TestSchemaService.java @@ -319,9 +319,9 @@ public class TestSchemaService extends XTestCase { super.setUp(); new Services().init(); wss = Services.get().get(SchemaService.class); - workflowValidator = wss.getSchema(SchemaName.WORKFLOW).newValidator(); - coordinatorValidator = wss.getSchema(SchemaName.COORDINATOR).newValidator(); - bundleValidator = wss.getSchema(SchemaName.BUNDLE).newValidator(); + workflowValidator = wss.getValidator(SchemaName.WORKFLOW); + coordinatorValidator = wss.getValidator(SchemaName.COORDINATOR); + bundleValidator = wss.getValidator(SchemaName.BUNDLE); } @Override @@ -355,7 +355,7 @@ public class TestSchemaService extends XTestCase { setSystemProperty(SchemaService.WF_CONF_EXT_SCHEMAS, "wf-ext-schema.xsd"); new Services().init(); SchemaService wss = Services.get().get(SchemaService.class); - Validator validator = wss.getSchema(SchemaName.WORKFLOW).newValidator(); + Validator validator = wss.getValidator(SchemaName.WORKFLOW); validator.validate(new StreamSource(new StringReader(APP2))); } http://git-wip-us.apache.org/repos/asf/oozie/blob/ba665da3/core/src/test/java/org/apache/oozie/util/TestXConfiguration.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/oozie/util/TestXConfiguration.java b/core/src/test/java/org/apache/oozie/util/TestXConfiguration.java index bd8e077..7a9d780 100644 --- a/core/src/test/java/org/apache/oozie/util/TestXConfiguration.java +++ b/core/src/test/java/org/apache/oozie/util/TestXConfiguration.java @@ -56,32 +56,24 @@ public class TestXConfiguration extends XTestCase { String parentXml = "parentXml"; prepareXmlWithInclude(parentXml); try { - XConfiguration conf = new XConfiguration(new FileInputStream(new File(getTestCaseDir(), parentXml))); - assertEquals("DEFAULT", conf.get("oozie.dummy")); - // verify the properties from include file - assertEquals("bar", conf.get("foo")); - assertEquals("def", conf.get("abc")); - } catch (IOException e) { - e.printStackTrace(); - fail("XInclude failed"); + new XConfiguration(new FileInputStream(new File(getTestCaseDir(), parentXml))); + fail("XInclude should not be allowed"); + } + catch (IOException e) { + assertEquals("bad conf file: element not <property>", e.getMessage()); } - } public void testAddXIncludeFromReader() throws IOException { String parentXml = "parentXml"; prepareXmlWithInclude(parentXml); try { - XConfiguration conf = new XConfiguration(new FileReader(new File(getTestCaseDir(), parentXml))); - assertEquals("DEFAULT", conf.get("oozie.dummy")); - // verify the properties from include file - assertEquals("bar", conf.get("foo")); - assertEquals("def", conf.get("abc")); - } catch (IOException e) { - e.printStackTrace(); - fail("XInclude failed"); + new XConfiguration(new FileReader(new File(getTestCaseDir(), parentXml))); + fail("XInclude should not be allowed"); + } + catch (IOException e) { + assertEquals("bad conf file: element not <property>", e.getMessage()); } - } // Copy the parent xml to testCaseDir and add the include element @@ -184,6 +176,7 @@ public class TestXConfiguration extends XTestCase { public void testResolve() { XConfiguration conf = new XConfiguration(); + conf.setRestrictSystemProperties(false); conf.set("a", "A"); conf.set("b", "${a}"); assertEquals("A", conf.getRaw("a")); @@ -205,14 +198,9 @@ public class TestXConfiguration extends XTestCase { assertEquals("${aa}", conf.getRaw("c")); assertEquals("A", conf.get("a")); assertEquals("A", conf.get("b")); - assertEquals("foo", conf.get("c")); assertEquals("${aaa}", conf.get("d")); - conf.set("un","${user.name}"); - assertEquals(System.getProperty("user.name"), conf.get("un")); - setSystemProperty("user.name", "foo"); - assertEquals("foo", conf.get("un")); - + assertEquals("${user.name}", conf.get("un")); } public void testSubstituteVar() throws ServiceException { @@ -235,7 +223,8 @@ public class TestXConfiguration extends XTestCase { try { conf.get(String.valueOf("key" + (depth))); fail("Fail to apply substitution depth"); - } catch (IllegalStateException e) { + } + catch (IllegalStateException e) { assertTrue(e.getMessage().contains("Variable substitution depth too large: " + depth)); } @@ -264,4 +253,4 @@ public class TestXConfiguration extends XTestCase { services.destroy(); } -} +} \ No newline at end of file http://git-wip-us.apache.org/repos/asf/oozie/blob/ba665da3/core/src/test/java/org/apache/oozie/util/TestXmlUtils.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/oozie/util/TestXmlUtils.java b/core/src/test/java/org/apache/oozie/util/TestXmlUtils.java index ac0b5ed..133789c 100644 --- a/core/src/test/java/org/apache/oozie/util/TestXmlUtils.java +++ b/core/src/test/java/org/apache/oozie/util/TestXmlUtils.java @@ -18,10 +18,10 @@ package org.apache.oozie.util; -import org.jdom.Element; import org.junit.Test; - -import static junit.framework.Assert.assertEquals; +import org.jdom.input.JDOMParseException; +import static junit.framework.Assert.assertTrue; +import static junit.framework.Assert.fail; public class TestXmlUtils { @@ -30,8 +30,14 @@ public class TestXmlUtils { @Test public void testExternalEntity() throws Exception { - Element e = XmlUtils.parseXml(EXTERNAL_ENTITY_XML); - assertEquals(0, e.getText().length()); + try { + XmlUtils.parseXml(EXTERNAL_ENTITY_XML); + fail("DOCTYPE should not be allowed"); + } catch (JDOMParseException e) { + assertTrue("Exception has different message.", e.getMessage(). + contains("DOCTYPE is disallowed when the feature \"http://apache."; + + "org/xml/features/disallow-doctype-decl\" set to true")); + } } @Test
