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.)
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.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;
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.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,
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 @@
}
/**
- * @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;
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 @@
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 @@
</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 @@
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>
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);
+ }
+
+}
