freemarker-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ddek...@apache.org
Subject [23/35] incubator-freemarker git commit: Bug fixed: XPath queries that has only contained characters that are valid in XML element names and has also contained :: (which is valid in names in namespace-unware documents), like e['following-sibling::foo'],
Date Wed, 30 Dec 2015 18:12:18 GMT
Bug fixed: XPath queries that has only contained characters that are valid in XML element names
and has also contained :: (which is valid in names in namespace-unware documents), like e['following-sibling::foo'],
were interpreted as literal element names (giving 0 hits) rather than as XPath expressions.
Note that there were no such problem with e['following-sibling::*'] for example, as it's not
a valid XML element name according the XML specification. This fix can actually break applications
that has processed namespace unaware XML that use :: as part of element or attribute names,
but such an application is highly unlikely, unlike running into the fixed problem. (Unfortunately,
using incompatible_improvements wasn't technically possible here.)


Project: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/commit/33ba278f
Tree: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/tree/33ba278f
Diff: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/diff/33ba278f

Branch: refs/heads/master
Commit: 33ba278fdd7a26653d2f96f945c3e5e701a9acf5
Parents: 55cb12e
Author: ddekany <ddekany@apache.org>
Authored: Mon Dec 28 20:17:40 2015 +0100
Committer: ddekany <ddekany@apache.org>
Committed: Mon Dec 28 20:17:40 2015 +0100

----------------------------------------------------------------------
 .../java/freemarker/ext/dom/NodeListModel.java  |  5 ++
 src/main/java/freemarker/ext/dom/NodeModel.java |  6 ++-
 .../freemarker/template/utility/StringUtil.java | 27 +++++++---
 src/manual/en_US/book.xml                       | 40 +++++++++++++-
 src/test/java/freemarker/ext/dom/DOMTest.java   | 55 ++++++++++++++++++++
 5 files changed, 124 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/33ba278f/src/main/java/freemarker/ext/dom/NodeListModel.java
----------------------------------------------------------------------
diff --git a/src/main/java/freemarker/ext/dom/NodeListModel.java b/src/main/java/freemarker/ext/dom/NodeListModel.java
index c0e85b9..22e555a 100644
--- a/src/main/java/freemarker/ext/dom/NodeListModel.java
+++ b/src/main/java/freemarker/ext/dom/NodeListModel.java
@@ -42,6 +42,11 @@ import freemarker.template.TemplateScalarModel;
 import freemarker.template.TemplateSequenceModel;
 import freemarker.template.utility.StringUtil;
 
+/**
+ * Used when the result set contains 0 or multiple nodes; shouldn't be used when you have
exactly 1 node. For exactly 1
+ * node, use {@link NodeModel#wrap(Node)}, because {@link NodeModel} subclasses can have
extra features building on that
+ * restriction, like single elements with text content can be used as FTL string-s.
+ */
 class NodeListModel extends SimpleSequence implements TemplateHashModel, _UnexpectedTypeErrorExplainerTemplateModel
{
     
     NodeModel contextNode;

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/33ba278f/src/main/java/freemarker/ext/dom/NodeModel.java
----------------------------------------------------------------------
diff --git a/src/main/java/freemarker/ext/dom/NodeModel.java b/src/main/java/freemarker/ext/dom/NodeModel.java
index c0ade05..c338664 100644
--- a/src/main/java/freemarker/ext/dom/NodeModel.java
+++ b/src/main/java/freemarker/ext/dom/NodeModel.java
@@ -63,12 +63,16 @@ import freemarker.template.TemplateNumberModel;
 import freemarker.template.TemplateSequenceModel;
 
 /**
- * A base class for wrapping a W3C DOM Node as a FreeMarker template model.
+ * A base class for wrapping a single W3C DOM Node as a FreeMarker template model.
  * 
  * <p>
  * Note that {@link DefaultObjectWrapper} automatically wraps W3C DOM {@link Node}-s into
this, so you may not need to
  * do that with this class manually. Though, before dropping the {@link Node}-s into the
data-model, you may want to
  * apply {@link NodeModel#simplify(Node)} on them.
+ * 
+ * <p>
+ * Note that this class shouldn't be used to represent a result set of 0 or multiple nodes
(we use {@link NodeListModel}
+ * then), but should be used to represent a node set of exactly 1 node.
  */
 abstract public class NodeModel
 implements TemplateNodeModel, TemplateHashModel, TemplateSequenceModel,

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/33ba278f/src/main/java/freemarker/template/utility/StringUtil.java
----------------------------------------------------------------------
diff --git a/src/main/java/freemarker/template/utility/StringUtil.java b/src/main/java/freemarker/template/utility/StringUtil.java
index 881bb3b..973c1f4 100644
--- a/src/main/java/freemarker/template/utility/StringUtil.java
+++ b/src/main/java/freemarker/template/utility/StringUtil.java
@@ -1653,18 +1653,31 @@ public class StringUtil {
     }
     
     /**
-     * @return whether the name is a valid XML tagname.
-     * (This routine might only be 99% accurate. Should maybe REVISIT) 
+     * Used internally by the XML DOM wrapper to check if the subvariable name is just an
element name, or a more
+     * complex XPath expression.
+     * 
+     * @return whether the name is a valid XML tagname. (This routine might only be 99% accurate.
Should maybe REVISIT)
+     * 
+     * @deprecated Don't use this outside FreeMarker; it's name if misleading, and it doesn't
follow the XML specs.
      */
+    @Deprecated
     static public boolean isXMLID(String name) {
-        for (int i = 0; i < name.length(); i++) {
+        int ln = name.length();
+        for (int i = 0; i < ln; i++) {
             char c = name.charAt(i);
-            if (i == 0) {
-                if (c == '-' || c == '.' || Character.isDigit(c))
+            if (i == 0 && (c == '-' || c == '.' || Character.isDigit(c))) {
                     return false;
             }
-            if (!Character.isLetterOrDigit(c) && c != ':' && c != '_' &&
c != '-' && c != '.') {
-                return false;
+            if (!Character.isLetterOrDigit(c) && c != '_' && c != '-' &&
c != '.') {
+                if (c == ':') {
+                    if (i + 1 < ln && name.charAt(i + 1) == ':') {
+                        // "::" is used in XPath
+                        return false;
+                    }
+                    // We don't return here, as a lonely ":" is allowed.
+                } else {
+                    return false;
+                }
             }
         }
         return true;

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/33ba278f/src/manual/en_US/book.xml
----------------------------------------------------------------------
diff --git a/src/manual/en_US/book.xml b/src/manual/en_US/book.xml
index a08d339..ba8c1b1 100644
--- a/src/manual/en_US/book.xml
+++ b/src/manual/en_US/book.xml
@@ -7250,7 +7250,7 @@ public class RepeatDirective implements TemplateDirectiveModel {
           nested calls of the same directive, or directive objects used as
           shared variables accessed by multiple threads concurrently.</para>
 
-          <para>Unfortunatelly, <literal>TemplateDirectiveModel</literal>-s
+          <para>Unfortunately, <literal>TemplateDirectiveModel</literal>-s
           don't support passing parameters by position (rather than by name).
           This is fixed starting from FreeMarker 2.4.</para>
         </section>
@@ -26928,6 +26928,25 @@ TemplateModel x = env.getVariable("x");  // get variable x</programlisting>
             </listitem>
 
             <listitem>
+              <para>Bug fixed: XPath queries that has only contained
+              characters that are valid in XML element names and has also
+              contained <literal>::</literal> (which is valid in names in
+              namespace-unware documents), like
+              <literal>e['following-sibling::foo']</literal>, were interpreted
+              as literal element names (giving 0 hits) rather than as XPath
+              expressions. Note that there were no such problem with
+              <literal>e['following-sibling::*']</literal> for example, as
+              it's not a valid XML element name according the XML
+              specification. This fix can actually break applications that has
+              processed namespace unaware XML that use <literal>::</literal>
+              as part of element or attribute names, but such an application
+              is highly unlikely, unlike running into the fixed problem.
+              (Unfortunately, using
+              <literal>incompatible_improvements</literal> wasn't technically
+              possible here.)</para>
+            </listitem>
+
+            <listitem>
               <para>Internal code cleanup: Mostly source code formatting, also
               many parser construction/setup cleanup</para>
             </listitem>
@@ -27264,6 +27283,25 @@ TemplateModel x = env.getVariable("x");  // get variable x</programlisting>
               the imported template was already imported earlier in another
               namespace.</para>
             </listitem>
+
+            <listitem>
+              <para>Bug fixed: XPath queries that has only contained
+              characters that are valid in XML element names and has also
+              contained <literal>::</literal> (which is valid in names in
+              namespace-unware documents), like
+              <literal>e['following-sibling::foo']</literal>, were interpreted
+              as literal element names (giving 0 hits) rather than as XPath
+              expressions. Note that there were no such problem with
+              <literal>e['following-sibling::*']</literal> for example, as
+              it's not a valid XML element name according the XML
+              specification. This fix can actually break applications that has
+              processed namespace unaware XML that use <literal>::</literal>
+              as part of element or attribute names, but such an application
+              is highly unlikely, unlike running into the fixed problem.
+              (Unfortunately, using
+              <literal>incompatible_improvements</literal> wasn't technically
+              possible here.)</para>
+            </listitem>
           </itemizedlist>
         </section>
       </section>

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/33ba278f/src/test/java/freemarker/ext/dom/DOMTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/freemarker/ext/dom/DOMTest.java b/src/test/java/freemarker/ext/dom/DOMTest.java
new file mode 100644
index 0000000..182ac31
--- /dev/null
+++ b/src/test/java/freemarker/ext/dom/DOMTest.java
@@ -0,0 +1,55 @@
+package freemarker.ext.dom;
+
+import static org.hamcrest.Matchers.*;
+import static org.junit.Assert.*;
+
+import java.io.IOException;
+import java.io.StringReader;
+
+import javax.xml.parsers.DocumentBuilder;
+import javax.xml.parsers.DocumentBuilderFactory;
+import javax.xml.parsers.ParserConfigurationException;
+
+import org.junit.Test;
+import org.w3c.dom.Document;
+import org.xml.sax.InputSource;
+import org.xml.sax.SAXException;
+
+import freemarker.template.TemplateException;
+import freemarker.test.TemplateTest;
+
+public class DOMTest extends TemplateTest {
+
+    @Test
+    public void xpathDetectionBugfix() throws Exception {
+        addDocToDataModel("<root><a>A</a><b>B</b><c>C</c></root>");
+        assertOutput("${doc.root.b['following-sibling::c']}", "C");
+        assertOutput("${doc.root.b['following-sibling::*']}", "C");
+    }
+
+    @Test
+    public void namespaceUnaware() throws Exception {
+        addNSUnawareDocToDataModel("<root><x:a>A</x:a><:>B</:><xyz::c>C</xyz::c></root>");
+        assertOutput("${doc.root['x:a']}", "A");
+        assertOutput("${doc.root[':']}", "B");
+        try {
+            assertOutput("${doc.root['xyz::c']}", "C");
+            fail();
+        } catch (TemplateException e) {
+            assertThat(e.getMessage(), containsString("xyz"));
+        }
+    }
+    
+    private void addDocToDataModel(String xml) throws SAXException, IOException, ParserConfigurationException
{
+        addToDataModel("doc", NodeModel.parse(new InputSource(new StringReader(xml))));
+    }
+
+    private void addNSUnawareDocToDataModel(String xml) throws ParserConfigurationException,
SAXException, IOException {
+        DocumentBuilderFactory newFactory = DocumentBuilderFactory.newInstance();
+        newFactory.setNamespaceAware(false);
+        DocumentBuilder builder = newFactory.newDocumentBuilder();
+        Document doc = builder.parse(new InputSource(new StringReader(xml)));
+        addToDataModel("doc", doc);
+    }
+
+}


Mime
View raw message