This is an automated email from the ASF dual-hosted git repository. ddekany pushed a commit to branch 2.3-gae in repository https://gitbox.apache.org/repos/asf/freemarker.git
commit 133cc44da897bde4375dc0b6b2a2844ef3400fd8 Author: ddekany <[email protected]> AuthorDate: Fri Oct 23 23:53:13 2020 +0200 Added DOMNodeSupport and JythonSupport boolean properties to DefaultObjectWrapper. This allows disabling the special wrapping of DOM nodes and Jython classes. This might be desirable for security reasons. --- .../freemarker/template/DefaultObjectWrapper.java | 82 ++++++++++++++++++---- .../DefaultObjectWrapperConfiguration.java | 30 +++++++- src/manual/en_US/book.xml | 34 +++++---- .../template/DefaultObjectWrapperTest.java | 45 +++++++++++- 4 files changed, 164 insertions(+), 27 deletions(-) diff --git a/src/main/java/freemarker/template/DefaultObjectWrapper.java b/src/main/java/freemarker/template/DefaultObjectWrapper.java index c0337db..eb44ce6 100644 --- a/src/main/java/freemarker/template/DefaultObjectWrapper.java +++ b/src/main/java/freemarker/template/DefaultObjectWrapper.java @@ -73,8 +73,10 @@ public class DefaultObjectWrapper extends freemarker.ext.beans.BeansWrapper { private boolean useAdaptersForContainers; private boolean forceLegacyNonListCollections; private boolean iterableSupport; + private boolean domNodeSupport; + private boolean jythonSupport; private final boolean useAdapterForEnumerations; - + /** * Creates a new instance with the incompatible-improvements-version specified in * {@link Configuration#DEFAULT_INCOMPATIBLE_IMPROVEMENTS}. @@ -134,6 +136,8 @@ public class DefaultObjectWrapper extends freemarker.ext.beans.BeansWrapper { && getIncompatibleImprovements().intValue() >= _TemplateAPI.VERSION_INT_2_3_26; forceLegacyNonListCollections = dowDowCfg.getForceLegacyNonListCollections(); iterableSupport = dowDowCfg.getIterableSupport(); + domNodeSupport = dowDowCfg.getDOMNodeSupport(); + jythonSupport = dowDowCfg.getJythonSupport(); finalizeConstruction(writeProtected); } @@ -255,9 +259,10 @@ public class DefaultObjectWrapper extends freemarker.ext.beans.BeansWrapper { * Called for an object that isn't considered to be of a "basic" Java type, like for an application specific type, * or for a W3C DOM node. In its default implementation, W3C {@link Node}-s will be wrapped as {@link NodeModel}-s * (allows DOM tree traversal), Jython objects will be delegated to the {@code JythonWrapper}, others will be - * wrapped using {@link BeansWrapper#wrap(Object)}. Note that if {@link #getMemberAccessPolicy()} doesn't return - * a {@link DefaultMemberAccessPolicy} or {@link LegacyDefaultMemberAccessPolicy}, then Jython wrapper will be - * skipped for security reasons. + * wrapped using {@link BeansWrapper#wrap(Object)}. However, these can be turned off with the + * {@link #setDOMNodeSupport(boolean)} and {@link #setJythonSupport(boolean)}. Note that if + * {@link #getMemberAccessPolicy()} doesn't return a {@link DefaultMemberAccessPolicy} or + * {@link LegacyDefaultMemberAccessPolicy}, then Jython wrapper will be skipped for security reasons. * * <p> * When you override this method, you should first decide if you want to wrap the object in a custom way (and if so @@ -265,16 +270,20 @@ public class DefaultObjectWrapper extends freemarker.ext.beans.BeansWrapper { * behavior is fine with you). */ protected TemplateModel handleUnknownType(Object obj) throws TemplateModelException { - if (obj instanceof Node) { + if (domNodeSupport && obj instanceof Node) { return wrapDomNode(obj); } - MemberAccessPolicy memberAccessPolicy = getMemberAccessPolicy(); - if (memberAccessPolicy instanceof DefaultMemberAccessPolicy - || memberAccessPolicy instanceof LegacyDefaultMemberAccessPolicy) { - if (JYTHON_WRAPPER != null && JYTHON_OBJ_CLASS.isInstance(obj)) { - return JYTHON_WRAPPER.wrap(obj); + + if (jythonSupport) { + MemberAccessPolicy memberAccessPolicy = getMemberAccessPolicy(); + if (memberAccessPolicy instanceof DefaultMemberAccessPolicy + || memberAccessPolicy instanceof LegacyDefaultMemberAccessPolicy) { + if (JYTHON_WRAPPER != null && JYTHON_OBJ_CLASS.isInstance(obj)) { + return JYTHON_WRAPPER.wrap(obj); + } } } + return super.wrap(obj); } @@ -389,6 +398,51 @@ public class DefaultObjectWrapper extends freemarker.ext.beans.BeansWrapper { } /** + * Getter pair of {@link #setDOMNodeSupport(boolean)}; see there. + * + * @since 2.3.31 + */ + public final boolean getDOMNodeSupport() { + return domNodeSupport; + } + + /** + * Enables wrapping {@link Node}-s on a special way (as described in the "XML Processing Guide" in the Manual); + * defaults to {@code true}.. If this is {@code true}, {@link Node}+s will be wrapped like any other generic object. + * + * @see #handleUnknownType(Object) + * + * @since 2.3.31 + */ + public void setDOMNodeSupport(boolean domNodeSupport) { + checkModifiable(); + this.domNodeSupport = domNodeSupport; + } + + /** + * Getter pair of {@link #setJythonSupport(boolean)}; see there. + * + * @since 2.3.31 + */ + public final boolean getJythonSupport() { + return jythonSupport; + } + + /** + * Enables wrapping Jython objects in a special way; defaults to {@code true}. If this is {@code false}, they will + * be wrapped like any other generic object. Note that Jython wrapping is legacy feature, and might by disabled by + * the selected {@link MemberAccessPolicy}, even if this is {@code true}; see {@link #handleUnknownType(Object)}. + * + * @see #handleUnknownType(Object) + * + * @since 2.3.31 + */ + public void setJythonSupport(boolean jythonSupport) { + checkModifiable(); + this.jythonSupport = jythonSupport; + } + + /** * Returns the lowest version number that is equivalent with the parameter version. * * @since 2.3.22 @@ -416,8 +470,12 @@ public class DefaultObjectWrapper extends freemarker.ext.beans.BeansWrapper { } } - return "useAdaptersForContainers=" + useAdaptersForContainers + ", forceLegacyNonListCollections=" - + forceLegacyNonListCollections + ", iterableSupport=" + iterableSupport + bwProps; + return "useAdaptersForContainers=" + useAdaptersForContainers + + ", forceLegacyNonListCollections=" + forceLegacyNonListCollections + + ", iterableSupport=" + iterableSupport + + ", domNodeSupport=" + domNodeSupport + + ", jythonSupport=" + jythonSupport + + bwProps; } } diff --git a/src/main/java/freemarker/template/DefaultObjectWrapperConfiguration.java b/src/main/java/freemarker/template/DefaultObjectWrapperConfiguration.java index ff474fa..a9575bc 100644 --- a/src/main/java/freemarker/template/DefaultObjectWrapperConfiguration.java +++ b/src/main/java/freemarker/template/DefaultObjectWrapperConfiguration.java @@ -35,6 +35,8 @@ public abstract class DefaultObjectWrapperConfiguration extends BeansWrapperConf private boolean useAdaptersForContainers; private boolean forceLegacyNonListCollections; private boolean iterableSupport; + private boolean domNodeSupport; + private boolean jythonSupport; protected DefaultObjectWrapperConfiguration(Version incompatibleImprovements) { super(DefaultObjectWrapper.normalizeIncompatibleImprovementsVersion(incompatibleImprovements), true); @@ -43,6 +45,8 @@ public abstract class DefaultObjectWrapperConfiguration extends BeansWrapperConf "freemarker.configuration", "DefaultObjectWrapper"); useAdaptersForContainers = getIncompatibleImprovements().intValue() >= _TemplateAPI.VERSION_INT_2_3_22; forceLegacyNonListCollections = true; // [2.4]: = IcI < _TemplateAPI.VERSION_INT_2_4_0; + domNodeSupport = true; + jythonSupport = true; } /** See {@link DefaultObjectWrapper#getUseAdaptersForContainers()}. */ @@ -65,6 +69,26 @@ public abstract class DefaultObjectWrapperConfiguration extends BeansWrapperConf this.forceLegacyNonListCollections = legacyNonListCollectionWrapping; } + /** See {@link DefaultObjectWrapper#getDOMNodeSupport()}. */ + public boolean getDOMNodeSupport() { + return domNodeSupport; + } + + /** See {@link DefaultObjectWrapper#setDOMNodeSupport(boolean)}. */ + public void setDOMNodeSupport(boolean domNodeSupport) { + this.domNodeSupport = domNodeSupport; + } + + /** See {@link DefaultObjectWrapper#getJythonSupport()}. */ + public boolean getJythonSupport() { + return jythonSupport; + } + + /** See {@link DefaultObjectWrapper#setJythonSupport(boolean)}. */ + public void setJythonSupport(boolean jythonSupport) { + this.jythonSupport = jythonSupport; + } + /** * See {@link DefaultObjectWrapper#getIterableSupport()}. * @@ -90,6 +114,8 @@ public abstract class DefaultObjectWrapperConfiguration extends BeansWrapperConf result = result * prime + (useAdaptersForContainers ? 1231 : 1237); result = result * prime + (forceLegacyNonListCollections ? 1231 : 1237); result = result * prime + (iterableSupport ? 1231 : 1237); + result = result * prime + (domNodeSupport ? 1231 : 1237); + result = result * prime + (jythonSupport ? 1231 : 1237); return result; } @@ -99,7 +125,9 @@ public abstract class DefaultObjectWrapperConfiguration extends BeansWrapperConf final DefaultObjectWrapperConfiguration thatDowCfg = (DefaultObjectWrapperConfiguration) that; return useAdaptersForContainers == thatDowCfg.getUseAdaptersForContainers() && forceLegacyNonListCollections == thatDowCfg.forceLegacyNonListCollections - && iterableSupport == thatDowCfg.iterableSupport; + && iterableSupport == thatDowCfg.iterableSupport + && domNodeSupport == thatDowCfg.domNodeSupport + && jythonSupport == thatDowCfg.jythonSupport; } } diff --git a/src/manual/en_US/book.xml b/src/manual/en_US/book.xml index a83ec5d..9d9123e 100644 --- a/src/manual/en_US/book.xml +++ b/src/manual/en_US/book.xml @@ -29179,18 +29179,18 @@ TemplateModel x = env.getVariable("x"); // get variable x</programlisting> <para>If you are using the default object wrapper class (<literal>freemarker.template.DefaultObjectWrapper</literal>), or a subclass of it, you should disable the XML (DOM) wrapping - feature of it, by overriding <literal>wrapDomNode(Object - obj)</literal> so that it does this: <literal>return - getModelFactory(obj.getClass()).create(obj, this);</literal>. - The problem with the XML wrapping feature, which wraps - <literal>org.w3c.dom.Node</literal> objects on special way to - make them easier to work with in templates, is that this - facility by design lets template authors evaluate arbitrary - XPath expressions, and XPath can do too much in certain - setups. If you really need the XML wrapping facility, review - carefully what XPath expressions are possible in your setup. - Also, be sure you don't use the long deprecated, and more - dangerous <literal>freemarker.ext.xml</literal> package, only + feature of it, by setting its + <literal>DOMNodeSupport</literal> property to + <literal>false</literal>. The problem with the XML wrapping + feature, which wraps <literal>org.w3c.dom.Node</literal> + objects on special way to make them easier to work with in + templates, is that this facility by design lets template + authors evaluate arbitrary XPath expressions, and XPath can do + too much in certain setups. If you really need the XML + wrapping facility, review carefully what XPath expressions are + possible in your setup. Also, be sure you don't use the long + deprecated, and more dangerous + <literal>freemarker.ext.xml</literal> package, only <literal>freemarker.ext.dom</literal>. Also, note that when using the XML wrapping feature, not allowing <literal>org.w3c.dom.Node</literal> methods in the @@ -29419,6 +29419,16 @@ TemplateModel x = env.getVariable("x"); // get variable x</programlisting> <itemizedlist> <listitem> + <para>Added <literal>DOMNodeSupport</literal> and + <literal>JythonSupport</literal> <literal>boolean</literal> + properties to <literal>DefaultObjectWrapper</literal>. This + allows disabling the special wrapping of DOM nodes and Jython + classes. This might be desirable <link + linkend="faq_template_uploading_security">for security + reasons</link>.</para> + </listitem> + + <listitem> <para><link xlink:href="https://issues.apache.org/jira/browse/FREEMARKER-145";>FREEMARKER-145</link>: Fixed bug where methods with "overloaded" return type may become diff --git a/src/test/java/freemarker/template/DefaultObjectWrapperTest.java b/src/test/java/freemarker/template/DefaultObjectWrapperTest.java index b973358..d9924f1 100644 --- a/src/test/java/freemarker/template/DefaultObjectWrapperTest.java +++ b/src/test/java/freemarker/template/DefaultObjectWrapperTest.java @@ -46,7 +46,9 @@ import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; +import org.hamcrest.Matchers; import org.junit.Test; +import org.python.core.PyString; import org.w3c.dom.Document; import org.xml.sax.InputSource; import org.xml.sax.SAXException; @@ -58,6 +60,7 @@ import freemarker.ext.beans.BeansWrapper; import freemarker.ext.beans.EnumerationModel; import freemarker.ext.beans.HashAdapter; import freemarker.ext.beans.WhitelistMemberAccessPolicy; +import freemarker.ext.jython.JythonSequenceModel; import freemarker.ext.util.WrapperTemplateModel; public class DefaultObjectWrapperTest { @@ -1033,11 +1036,49 @@ public class DefaultObjectWrapperTest { @Test public void testCanWrapDOM() throws SAXException, IOException, ParserConfigurationException, TemplateModelException { + assertTrue(OW22.wrap(createDocument()) instanceof TemplateNodeModel); + } + + @Test + public void testDisabledDOMNodeWrapping() throws SAXException, IOException, ParserConfigurationException, + TemplateModelException { + Document doc = createDocument(); + DefaultObjectWrapperBuilder dowB = new DefaultObjectWrapperBuilder(Configuration.VERSION_2_3_31); + dowB.setDOMNodeSupport(false); + DefaultObjectWrapper ow = dowB.build(); + + testDisabledDomWrappingInternal(doc, ow); + + ow = new DefaultObjectWrapper(Configuration.VERSION_2_3_31); + ow.setDOMNodeSupport(false); + testDisabledDomWrappingInternal(doc, ow); + } + + private void testDisabledDomWrappingInternal(Document doc, DefaultObjectWrapper ow) throws TemplateModelException { + TemplateModel model = ow.wrap(doc); + assertFalse(model instanceof TemplateNodeModel); + assertTrue(model instanceof TemplateHashModel); + assertNotNull(((TemplateHashModel) model).get("getDoctype")); + assertNotNull(((TemplateHashModel) model).get("class")); + } + + @Test + public void testDisabledJythonWrapping() throws SAXException, IOException, ParserConfigurationException, + TemplateModelException { + PyString pyString = new PyString("foo"); + + DefaultObjectWrapper ow = new DefaultObjectWrapper(Configuration.VERSION_2_3_31); + assertThat(ow.wrap(pyString), Matchers.instanceOf(JythonSequenceModel.class)); + + ow.setJythonSupport(false); + assertThat(ow.wrap(pyString), not(Matchers.instanceOf(JythonSequenceModel.class))); + } + + private Document createDocument() throws ParserConfigurationException, SAXException, IOException { DocumentBuilder db = DocumentBuilderFactory.newInstance().newDocumentBuilder(); InputSource is = new InputSource(); is.setCharacterStream(new StringReader("<doc><sub a='1' /></doc>")); - Document doc = db.parse(is); - assertTrue(OW22.wrap(doc) instanceof TemplateNodeModel); + return db.parse(is); } @Test
