diff --git a/source/java/org/alfresco/repo/rendition/executer/XSLTRenderingEngineTest.java b/source/java/org/alfresco/repo/rendition/executer/XSLTRenderingEngineTest.java index f7c04b3e83..13d7043555 100644 --- a/source/java/org/alfresco/repo/rendition/executer/XSLTRenderingEngineTest.java +++ b/source/java/org/alfresco/repo/rendition/executer/XSLTRenderingEngineTest.java @@ -35,7 +35,6 @@ import org.alfresco.service.cmr.repository.ContentWriter; import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.cmr.repository.NodeService; import org.alfresco.service.cmr.repository.StoreRef; -import org.alfresco.service.cmr.repository.TemplateException; import org.alfresco.service.cmr.repository.TemplateProcessor; import org.alfresco.service.cmr.repository.TemplateService; import org.alfresco.service.cmr.search.ResultSet; @@ -46,7 +45,6 @@ import org.alfresco.util.GUID; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; - /** * @author Brian * @@ -84,62 +82,6 @@ public class XSLTRenderingEngineTest extends BaseAlfrescoSpringTest "/app:company_home"); this.companyHome = rs.getNodeRef(0); } - - public void testSecurityFilter() throws Exception - { - try - { - FileInfo file = createXmlFile(companyHome); - FileInfo xslFile = createXmlFile(companyHome, insecureVerySimpleXSLT); - - RenditionDefinition def = renditionService.createRenditionDefinition(QName.createQName("Test"), XSLTRenderingEngine.NAME); - def.setParameterValue(XSLTRenderingEngine.PARAM_TEMPLATE_NODE, xslFile.getNodeRef()); - - ChildAssociationRef rendition = renditionService.render(file.getNodeRef(), def); - log.error("This insecure template should not process!"); - fail(); - - } - catch (TemplateException e) - { - //pass! - } - catch (Exception ex) - { - - log.error("Error!", ex); - fail(); - } - } - - public void testIncludeSecurityFilter() throws Exception - { - try - { - FileInfo file = createXmlFile(companyHome); - FileInfo insecureXSLFile = createXmlFile(companyHome, insecureVerySimpleXSLT); - - String includeInsecureXSLFile = String.format(insecureIncludeVerySimpleXSLT, insecureXSLFile.getName()); - FileInfo xslFile = createXmlFile(companyHome, includeInsecureXSLFile); - - RenditionDefinition def = renditionService.createRenditionDefinition(QName.createQName("Test"), XSLTRenderingEngine.NAME); - def.setParameterValue(XSLTRenderingEngine.PARAM_TEMPLATE_NODE, xslFile.getNodeRef()); - - ChildAssociationRef rendition = renditionService.render(file.getNodeRef(), def); - log.error("This insecure include template should not process!"); - fail(); - } - catch (TemplateException e) - { - //pass! - } - catch (Exception ex) - { - - log.error("Error!", ex); - fail(); - } - } public void testSimplestStringTemplate() throws Exception { @@ -376,23 +318,6 @@ public class XSLTRenderingEngineTest extends BaseAlfrescoSpringTest "" + "" + "" + "" + "" + ""; - private String insecureVerySimpleXSLT = "" + " " - + "xmlns:fn=\"http://www.w3.org/2005/02/xpath-functions\"> " + "" + - - "" + - - "" + "" - + "" + "" + "" + ""; - - private String insecureIncludeVerySimpleXSLT = "" + "" - + "" - + "" - + "" - + ""; - private String callParseXmlDocument = "" + " " + "" + diff --git a/source/java/org/alfresco/repo/template/XSLTProcessor.java b/source/java/org/alfresco/repo/template/XSLTProcessor.java index 3aa91a90a3..e988073d36 100644 --- a/source/java/org/alfresco/repo/template/XSLTProcessor.java +++ b/source/java/org/alfresco/repo/template/XSLTProcessor.java @@ -22,7 +22,6 @@ import java.io.IOException; import java.io.InputStream; import java.io.Reader; import java.io.StringReader; -import java.io.StringWriter; import java.io.Writer; import java.util.Arrays; import java.util.HashMap; @@ -49,7 +48,6 @@ import org.alfresco.service.namespace.QName; import org.alfresco.util.XMLUtil; import org.apache.bsf.BSFManager; import org.apache.commons.lang.StringUtils; -import org.apache.commons.lang.exception.ExceptionUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.xml.utils.Constants; @@ -136,7 +134,7 @@ public class XSLTProcessor extends BaseProcessor implements TemplateProcessor Document xslTemplate; try { - xslTemplate = XMLUtil.secureParseXSL(templateSource.getReader(defaultEncoding)); + xslTemplate = XMLUtil.parse(templateSource.getReader(defaultEncoding)); } catch (IOException ex) { @@ -202,7 +200,7 @@ public class XSLTProcessor extends BaseProcessor implements TemplateProcessor throw new TransformerException("unable to resolve href " + href); } - Document d = XMLUtil.secureParseXSL(in); + Document d = XMLUtil.parse(in); if (log.isDebugEnabled()) { log.debug("loaded " + XMLUtil.toString(d)); @@ -241,12 +239,7 @@ public class XSLTProcessor extends BaseProcessor implements TemplateProcessor final StringBuilder msg = new StringBuilder("errors encountered creating tranformer ... \n"); for (TransformerException te : errors) { - msg.append("message: " + te.getMessageAndLocation()).append("\n"); - String cause = ExceptionUtils.getRootCauseMessage(te); - if (cause != null) - { - msg.append(" caused by: " + cause); - } + msg.append(te.getMessageAndLocation()).append("\n"); } throw new TemplateException(msg.toString()); } @@ -290,11 +283,6 @@ public class XSLTProcessor extends BaseProcessor implements TemplateProcessor for (TransformerException te : errors) { msg.append(te.getMessageAndLocation()).append("\n"); - String cause = ExceptionUtils.getRootCauseMessage(te); - if (cause != null) - { - msg.append(" caused by: " + cause); - } } throw new TemplateException(msg.toString()); } @@ -326,17 +314,18 @@ public class XSLTProcessor extends BaseProcessor implements TemplateProcessor final Element docEl = xslTemplate.getDocumentElement(); final String XALAN_NS = Constants.S_BUILTIN_EXTENSIONS_URL; final String XALAN_NS_PREFIX = "xalan"; + docEl.setAttribute("xmlns:" + XALAN_NS_PREFIX, XALAN_NS); final Set excludePrefixes = new HashSet(); if (docEl.hasAttribute("exclude-result-prefixes")) { excludePrefixes.addAll(Arrays.asList(docEl.getAttribute("exclude-result-prefixes").split(" "))); } + excludePrefixes.add(XALAN_NS_PREFIX); final List result = new LinkedList(); for (QName ns : methods.keySet()) { - final String prefix = ns.getLocalName(); docEl.setAttribute("xmlns:" + prefix, ns.getNamespaceURI()); excludePrefixes.add(prefix); @@ -376,11 +365,6 @@ public class XSLTProcessor extends BaseProcessor implements TemplateProcessor } docEl.setAttribute("exclude-result-prefixes", StringUtils.join(excludePrefixes .toArray(new String[excludePrefixes.size()]), " ")); - if (log.isDebugEnabled()) { - StringWriter writer = new StringWriter(); - XMLUtil.print(xslTemplate, writer); - log.debug(writer); - } return result; } diff --git a/source/java/org/alfresco/repo/template/XSLTProcessorTest.java b/source/java/org/alfresco/repo/template/XSLTProcessorTest.java index 3cef815dba..21c9413a66 100644 --- a/source/java/org/alfresco/repo/template/XSLTProcessorTest.java +++ b/source/java/org/alfresco/repo/template/XSLTProcessorTest.java @@ -30,7 +30,6 @@ import org.alfresco.service.cmr.repository.ContentWriter; import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.cmr.repository.NodeService; import org.alfresco.service.cmr.repository.StoreRef; -import org.alfresco.service.cmr.repository.TemplateException; import org.alfresco.service.cmr.repository.TemplateProcessor; import org.alfresco.service.cmr.repository.TemplateService; import org.alfresco.service.cmr.search.ResultSet; @@ -76,60 +75,6 @@ public class XSLTProcessorTest extends BaseAlfrescoSpringTest "/app:company_home"); this.companyHome = rs.getNodeRef(0); } - - public void testSecurityFilter() throws Exception - { - try - { - FileInfo file = createXmlFile(companyHome); - XSLTemplateModel model = new XSLTemplateModel(); - model.put(XSLTProcessor.ROOT_NAMESPACE, XMLUtil.parse(file.getNodeRef(), contentService)); - - StringWriter writer = new StringWriter(); - xsltProcessor.processString(insecureVerySimpleXSLT, model, writer); - log.error("This insecure template should not process!"); - fail(); - } - catch (TemplateException e) - { - //pass! - } - catch (Exception ex) - { - log.error("Error!", ex); - fail(); - } - } - - public void testIncludeSecurityFilter() throws Exception - { - try - { - FileInfo file = createXmlFile(companyHome); - FileInfo insecureXSLFile = createXmlFile(companyHome, insecureVerySimpleXSLT); - - String includeInsecureXSLTemplate = String.format(insecureIncludeVerySimpleXSLT, insecureXSLFile.getName()); - FileInfo includeInsecureXSLFile = createXmlFile(companyHome, includeInsecureXSLTemplate); - - XSLTemplateModel model = new XSLTemplateModel(); - model.put(XSLTProcessor.ROOT_NAMESPACE, XMLUtil.parse(file.getNodeRef(), contentService)); - - StringWriter writer = new StringWriter(); - xsltProcessor.process(includeInsecureXSLFile.getNodeRef().toString(), model, writer); - log.error("This insecure include template should not process!"); - fail(); - } - catch (TemplateException e) - { - //pass! - } - catch (Exception ex) - { - - log.error("Error!", ex); - fail(); - } - } public void testSimplestStringTemplate() throws Exception { @@ -322,21 +267,4 @@ public class XSLTProcessorTest extends BaseAlfrescoSpringTest "" + "" + "" + "" + "" + ""; - - private String insecureIncludeVerySimpleXSLT = "" + "" - + "" - + "" - + "" - + ""; - - private String insecureVerySimpleXSLT = "" + " " - + "xmlns:fn=\"http://www.w3.org/2005/02/xpath-functions\"> " + "" + - - "" + - - "" + "" - + "" + "" + "" + ""; } diff --git a/source/java/org/alfresco/util/XMLUtil.java b/source/java/org/alfresco/util/XMLUtil.java index 664d1ae3bc..683e356805 100644 --- a/source/java/org/alfresco/util/XMLUtil.java +++ b/source/java/org/alfresco/util/XMLUtil.java @@ -19,17 +19,7 @@ package org.alfresco.util; -import java.io.CharArrayReader; -import java.io.File; -import java.io.FileInputStream; -import java.io.FileWriter; -import java.io.IOException; -import java.io.InputStream; -import java.io.Reader; -import java.io.StringWriter; -import java.io.Writer; -import java.util.LinkedList; -import java.util.List; +import java.io.*; import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; @@ -38,33 +28,18 @@ import javax.xml.transform.OutputKeys; import javax.xml.transform.Transformer; import javax.xml.transform.TransformerException; import javax.xml.transform.TransformerFactory; -import javax.xml.transform.dom.DOMResult; import javax.xml.transform.dom.DOMSource; -import javax.xml.transform.sax.SAXTransformerFactory; -import javax.xml.transform.sax.TransformerHandler; import javax.xml.transform.stream.StreamResult; - import org.alfresco.model.ContentModel; import org.alfresco.service.cmr.avm.AVMService; import org.alfresco.service.cmr.repository.ContentReader; import org.alfresco.service.cmr.repository.ContentService; import org.alfresco.service.cmr.repository.NodeRef; -import org.apache.commons.lang.StringUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.w3c.dom.Attr; -import org.w3c.dom.Document; -import org.w3c.dom.Element; -import org.w3c.dom.Node; -import org.w3c.dom.NodeList; -import org.w3c.dom.Text; -import org.xml.sax.Attributes; +import org.w3c.dom.*; import org.xml.sax.InputSource; import org.xml.sax.SAXException; -import org.xml.sax.XMLFilter; -import org.xml.sax.XMLReader; -import org.xml.sax.helpers.XMLFilterImpl; -import org.xml.sax.helpers.XMLReaderFactory; /** * XML utility functions. @@ -139,229 +114,75 @@ public class XMLUtil } /** utility function for parsing xml */ - public static Document parse(final String source, final XMLFilter... filters) + public static Document parse(final String source) throws SAXException, IOException { - return XMLUtil.parse(new CharArrayReader(source.toCharArray()), filters); - } - - public static Document secureParseXSL (final String source, final XMLFilter... filters) - throws SAXException, - IOException - { - return parse(new CharArrayReader(source.toCharArray()), addSecurityFilter(filters)); + return XMLUtil.parse(new ByteArrayInputStream(source.getBytes("UTF-8"))); } /** utility function for parsing xml */ public static Document parse(final NodeRef nodeRef, - final ContentService contentService, - final XMLFilter... filters) + final ContentService contentService) throws SAXException, IOException { final ContentReader contentReader = contentService.getReader(nodeRef, ContentModel.TYPE_CONTENT); final InputStream in = contentReader.getContentInputStream(); - return XMLUtil.parse(in, filters); + return XMLUtil.parse(in); } - - public static Document secureParseXSL(final NodeRef nodeRef, - final ContentService contentService, - final XMLFilter... filters) - throws SAXException, - IOException - { - final ContentReader contentReader = - contentService.getReader(nodeRef, ContentModel.TYPE_CONTENT); - final InputStream in = contentReader.getContentInputStream(); - return parse(in, addSecurityFilter(filters)); - } /** utility function for parsing xml */ public static Document parse(final int version, final String path, - final AVMService avmService, - final XMLFilter...filters) + final AVMService avmService) throws SAXException, IOException { - return XMLUtil.parse(avmService.getFileInputStream(version, path), filters); - } - - public static Document secureParseXSL(final int version, - final String path, - final AVMService avmService, - final XMLFilter... filters) - throws SAXException, - IOException - { - return parse(avmService.getFileInputStream(version, path), addSecurityFilter(filters)); + return XMLUtil.parse(avmService.getFileInputStream(version, path)); } /** utility function for parsing xml */ - public static Document parse(final File source, - final XMLFilter... filters) + public static Document parse(final File source) throws SAXException, IOException { - return XMLUtil.parse(new FileInputStream(source), filters); - } - - public static Document secureParseXSL(final File source, - final XMLFilter... filters) - throws SAXException, - IOException - { - return parse(new FileInputStream(source), addSecurityFilter(filters)); - } - - private static Document parseWithXMLFilters(final InputSource source, - final XMLFilter... filters) - throws SAXException, - IOException - { - return parseWithXMLFilters(source, false, filters); - } - - private static Document parseWithXMLFilters(final InputSource source, - final boolean validating, - final XMLFilter... filters) - throws SAXException, - IOException - { - TransformerFactory tf = TransformerFactory.newInstance(); - // Check to make sure this is a SAX TransformerFactory - if (!tf.getFeature(SAXTransformerFactory.FEATURE)) - { - throw new SAXException("SAX Transformation factory not found."); - } - // Cast to appropriate factory class - SAXTransformerFactory stf = (SAXTransformerFactory) tf; - final DocumentBuilder db = XMLUtil.getDocumentBuilder(true, validating); - - if (filters == null || filters.length == 0) - { - // No filters. Process this as normal. - return db.parse(source); - } - else - { - // Process with filters - try - { - final Document doc = db.newDocument(); - final TransformerHandler th = stf.newTransformerHandler(); - // Specify transformation to DOMResult with empty Node container (Document) - th.setResult(new DOMResult(doc)); - XMLReader reader = XMLReaderFactory.createXMLReader(); - - //emulate what the document builder parser supports - //all readers are required to support namespaces and namespace-prefixes - reader.setFeature("http://xml.org/sax/features/namespaces", db.isNamespaceAware()); - reader.setFeature("http://xml.org/sax/features/namespace-prefixes", db.isNamespaceAware() ? true : false); - - // Chain multiple filters together - int i = 0; - XMLFilter filter = null; - for (XMLFilter f : filters) - { - // there can be no null in the filter list - if (f == null) - throw new SAXException("Nulls are not allowed in XML filter list."); - // if first item then set new reader - if (i == 0) - f.setParent(reader); - else - // set parent filter to previous element in the array - f.setParent(filters[i - 1]); - - filter = f; - i++; - } - //not sure how filter could be null - if (filter != null) - { - filter.setContentHandler(th); - filter.parse(source); - try - { - //try to activate/deactivate validation - filter.setFeature("http://xml.org/sax/features/validation", db.isValidating()); - } - catch (SAXException se) - { - LOGGER.warn("XML reader does not support validation feature.", se); - } - } - else - { - //not sure how we could get here - throw new SAXException("No XML filters available to process this request."); - } - if (LOGGER.isDebugEnabled()) { - StringWriter writer = new StringWriter(); - XMLUtil.print(doc, writer); - LOGGER.debug(writer); - } - return doc; - } - catch (TransformerException tce) - { - throw new SAXException(tce); - } - } + return XMLUtil.parse(new FileInputStream(source)); } /** utility function for parsing xml */ - public static Document parse(final InputStream source, final XMLFilter... filters) + public static Document parse(final InputStream source) throws SAXException, IOException { try { - return parseWithXMLFilters(new InputSource(source), filters); + final DocumentBuilder db = XMLUtil.getDocumentBuilder(); + return db.parse(source); } finally { source.close(); } } - - /** secure parse for InputStream source */ - public static Document secureParseXSL(final InputStream source, - final XMLFilter... filters) - throws SAXException, - IOException - { - return parse(source, addSecurityFilter(filters)); - } /** utility function for parsing xml */ - public static Document parse(final Reader source, - final XMLFilter... filters) + public static Document parse(final Reader source) throws SAXException, IOException { try { - return parseWithXMLFilters(new InputSource(source), filters); + final DocumentBuilder db = XMLUtil.getDocumentBuilder(); + return db.parse(new InputSource(source)); } finally { source.close(); } } - - /** secure parse for Reader source **/ - public static Document secureParseXSL(final Reader source, - final XMLFilter... filters) - throws SAXException, - IOException - { - return parse(source, addSecurityFilter(filters)); - } - + /** provides a document builder that is namespace aware but not validating by default */ public static DocumentBuilder getDocumentBuilder() { @@ -472,81 +293,4 @@ public class XMLUtil } }; } - - /** - * returns a new array of filters with the security filter at the head of the array - */ - private static XMLFilter[] addSecurityFilter(XMLFilter...filters) { - if (filters == null || filters.length == 0) { - return new XMLFilter[] {new FastFailSecureXMLFilter()}; - } else { - XMLFilter[] xmlfilters = new XMLFilter[filters.length + 1]; - xmlfilters[0] = new FastFailSecureXMLFilter(); - System.arraycopy(filters, 0, xmlfilters, 1, filters.length); - return xmlfilters; - } - } - - /** - * XMLFilter that throws an exception when it comes across any insecure namespaces - */ - private static class FastFailSecureXMLFilter extends XMLFilterImpl - { - - private static final List insecureURIs = new LinkedList() - { - private static final long serialVersionUID = 1L; - - { - add("xalan://"); - add("http://exslt.org/"); - add("http://xml.apache.org/xalan/PipeDocument"); - add("http://xml.apache.org/xalan/sql"); - add("http://xml.apache.org/xalan/redirect"); - add("http://xml.apache.org/xalan/xsltc/java"); - add("http://xml.apache.org/xalan/java"); - add("http://xml.apache.org/xslt"); - add("http://xml.apache.org/java"); - } - }; - - public FastFailSecureXMLFilter() - { - }; - - public void startPrefixMapping(String prefix, String uri) - throws SAXException - { - if (isInsecureURI(uri)) - { - throw new SAXException("Insecure namespace: " + uri); - } - super.startPrefixMapping(prefix, uri); - } - - - public void startElement (String uri, - String localName, - String qName, - final Attributes atts) - throws SAXException - { - - if (isInsecureURI(uri)) - { - throw new SAXException("Insecure namespace: " + uri); - } - super.startElement(uri, localName, qName, atts); - } - - private boolean isInsecureURI(String uri) - { - for (String insecureURI : insecureURIs) - { - if (StringUtils.startsWithIgnoreCase(uri, insecureURI)) return true; - } - return false; - } - - } }