Repository: incubator-freemarker Updated Branches: refs/heads/2.3-gae 28ac1abe4 -> 55b09e8de
Better error messages when someone tries to get an invalid @@... subvariable of an XML DOM node (now it's not issued by the XPath implementation, which just sees it as a syntactical error). Some optimizations and cleanups regarding the matching of special keys (@@... and some more) in freemarker.ext.dom. Project: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/commit/55b09e8d Tree: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/tree/55b09e8d Diff: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/diff/55b09e8d Branch: refs/heads/2.3-gae Commit: 55b09e8dee070b490d38f850261f1accb278928a Parents: 28ac1ab Author: ddekany <[email protected]> Authored: Wed Jan 11 00:55:21 2017 +0100 Committer: ddekany <[email protected]> Committed: Wed Jan 11 00:55:21 2017 +0100 ---------------------------------------------------------------------- .../java/freemarker/core/BuiltInsForNodes.java | 3 +- src/main/java/freemarker/ext/dom/AtAtKey.java | 58 +++++++++ .../java/freemarker/ext/dom/DocumentModel.java | 2 +- .../java/freemarker/ext/dom/DomStringUtil.java | 93 ++++++++++++++ .../java/freemarker/ext/dom/ElementModel.java | 120 +++++++++---------- .../java/freemarker/ext/dom/NodeListModel.java | 48 +++++--- src/main/java/freemarker/ext/dom/NodeModel.java | 46 +++---- .../java/freemarker/ext/dom/_ExtDomApi.java | 43 +++++++ .../freemarker/template/utility/StringUtil.java | 44 +------ src/manual/en_US/book.xml | 12 +- src/test/java/freemarker/ext/dom/DOMTest.java | 12 ++ 11 files changed, 339 insertions(+), 142 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/55b09e8d/src/main/java/freemarker/core/BuiltInsForNodes.java ---------------------------------------------------------------------- diff --git a/src/main/java/freemarker/core/BuiltInsForNodes.java b/src/main/java/freemarker/core/BuiltInsForNodes.java index fa2c6ec..33670da 100644 --- a/src/main/java/freemarker/core/BuiltInsForNodes.java +++ b/src/main/java/freemarker/core/BuiltInsForNodes.java @@ -22,6 +22,7 @@ package freemarker.core; import java.util.List; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +import freemarker.ext.dom._ExtDomApi; import freemarker.template.*; import freemarker.template.utility.StringUtil; @@ -135,7 +136,7 @@ class BuiltInsForNodes { } } else { for (int j = 0; j < names.size(); j++) { - if (StringUtil.matchesName((String) names.get(j), nodeName, nsURI, env)) { + if (_ExtDomApi.matchesName((String) names.get(j), nodeName, nsURI, env)) { result.add(tnm); break; } http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/55b09e8d/src/main/java/freemarker/ext/dom/AtAtKey.java ---------------------------------------------------------------------- diff --git a/src/main/java/freemarker/ext/dom/AtAtKey.java b/src/main/java/freemarker/ext/dom/AtAtKey.java new file mode 100644 index 0000000..a3c7166 --- /dev/null +++ b/src/main/java/freemarker/ext/dom/AtAtKey.java @@ -0,0 +1,58 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package freemarker.ext.dom; + +/** + * The special hash keys that start with "@@". + */ +enum AtAtKey { + + MARKUP("@@markup"), + NESTED_MARKUP("@@nested_markup"), + ATTRIBUTES_MARKUP("@@attributes_markup"), + TEXT("@@text"), + START_TAG("@@start_tag"), + END_TAG("@@end_tag"), + QNAME("@@qname"), + NAMESPACE("@@namespace"), + LOCAL_NAME("@@local_name"), + ATTRIBUTES("@@"), + PREVIOUS_SIGNIFICANT("@@previous_significant"), + NEXT_SIGNIFICANT("@@next_significant"); + + private final String key; + + public String getKey() { + return key; + } + + private AtAtKey(String key) { + this.key = key; + } + + public static boolean containsKey(String key) { + for (AtAtKey item : AtAtKey.values()) { + if (item.getKey().equals(key)) { + return true; + } + } + return false; + } + +} http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/55b09e8d/src/main/java/freemarker/ext/dom/DocumentModel.java ---------------------------------------------------------------------- diff --git a/src/main/java/freemarker/ext/dom/DocumentModel.java b/src/main/java/freemarker/ext/dom/DocumentModel.java index 76859eb..fb161e0 100644 --- a/src/main/java/freemarker/ext/dom/DocumentModel.java +++ b/src/main/java/freemarker/ext/dom/DocumentModel.java @@ -52,7 +52,7 @@ class DocumentModel extends NodeModel implements TemplateHashModel { } else if (key.equals("**")) { NodeList nl = ((Document) node).getElementsByTagName("*"); return new NodeListModel(nl, this); - } else if (StringUtil.isXMLID(key)) { + } else if (DomStringUtil.isXMLID(key)) { ElementModel em = (ElementModel) NodeModel.wrap(((Document) node).getDocumentElement()); if (em.matchesName(key, Environment.getCurrentEnvironment())) { return em; http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/55b09e8d/src/main/java/freemarker/ext/dom/DomStringUtil.java ---------------------------------------------------------------------- diff --git a/src/main/java/freemarker/ext/dom/DomStringUtil.java b/src/main/java/freemarker/ext/dom/DomStringUtil.java new file mode 100644 index 0000000..85df716 --- /dev/null +++ b/src/main/java/freemarker/ext/dom/DomStringUtil.java @@ -0,0 +1,93 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package freemarker.ext.dom; + +import freemarker.core.Environment; +import freemarker.template.Template; + +/** + * For internal use only; don't depend on this, there's no backward compatibility guarantee at all! + * This class is to work around the lack of module system in Java, i.e., so that other FreeMarker packages can + * access things inside this package that users shouldn't. + */ +final class DomStringUtil { + + private DomStringUtil() { + // Not meant to be instantiated + } + + static boolean isXMLID(String name) { + return isXMLID(name, 0); + } + + /** + * Check if the subvariable name is just an element name, or a more complex XPath expression. + * + * @param firstCharIdx The index of the character in the string parameter that we treat as the beginning of the + * string to check. This is to spare substringing that has become more expensive in Java 7. + * + * @return whether the name is a valid XML element name. (This routine might only be 99% accurate. REVISIT) + */ + static boolean isXMLID(String name, int firstCharIdx) { + int ln = name.length(); + for (int i = firstCharIdx; i < ln; i++) { + char c = name.charAt(i); + if (i == firstCharIdx && (c == '-' || c == '.' || Character.isDigit(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; + } + + /** + * @return whether the qname matches the combination of nodeName, nsURI, and environment prefix settings. + */ + static boolean matchesName(String qname, String nodeName, String nsURI, Environment env) { + String defaultNS = env.getDefaultNS(); + if ((defaultNS != null) && defaultNS.equals(nsURI)) { + return qname.equals(nodeName) + || qname.equals(Template.DEFAULT_NAMESPACE_PREFIX + ":" + nodeName); + } + if ("".equals(nsURI)) { + if (defaultNS != null) { + return qname.equals(Template.NO_NS_PREFIX + ":" + nodeName); + } else { + return qname.equals(nodeName) || qname.equals(Template.NO_NS_PREFIX + ":" + nodeName); + } + } + String prefix = env.getPrefixForNamespace(nsURI); + if (prefix == null) { + return false; // Is this the right thing here??? + } + return qname.equals(prefix + ":" + nodeName); + } + +} http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/55b09e8d/src/main/java/freemarker/ext/dom/ElementModel.java ---------------------------------------------------------------------- diff --git a/src/main/java/freemarker/ext/dom/ElementModel.java b/src/main/java/freemarker/ext/dom/ElementModel.java index a72e55c..21d0a35 100644 --- a/src/main/java/freemarker/ext/dom/ElementModel.java +++ b/src/main/java/freemarker/ext/dom/ElementModel.java @@ -19,6 +19,8 @@ package freemarker.ext.dom; +import java.util.Collections; + import org.w3c.dom.Attr; import org.w3c.dom.Element; import org.w3c.dom.Node; @@ -33,9 +35,6 @@ import freemarker.template.TemplateScalarModel; import freemarker.template.TemplateSequenceModel; import freemarker.template.utility.StringUtil; -import java.util.ArrayList; -import java.util.Collections; - class ElementModel extends NodeModel implements TemplateScalarModel { public ElementModel(Element element) { @@ -69,68 +68,69 @@ class ElementModel extends NodeModel implements TemplateScalarModel { } } return ns; - } - if (key.equals("**")) { - Element elem = (Element) node; - return new NodeListModel(elem.getElementsByTagName("*"), this); - } - if (key.startsWith("@")) { - if (key.equals("@@") || key.equals("@*")) { - return new NodeListModel(node.getAttributes(), this); - } - if (key.equals("@@start_tag")) { - NodeOutputter nodeOutputter = new NodeOutputter(node); - return new SimpleScalar(nodeOutputter.getOpeningTag((Element) node)); - } - if (key.equals("@@end_tag")) { - NodeOutputter nodeOutputter = new NodeOutputter(node); - return new SimpleScalar(nodeOutputter.getClosingTag((Element) node)); - } - if (key.equals("@@attributes_markup")) { - StringBuilder buf = new StringBuilder(); - NodeOutputter nu = new NodeOutputter(node); - nu.outputContent(node.getAttributes(), buf); - return new SimpleScalar(buf.toString().trim()); - } - if (key.equals("@@previous_significant")) { - Node previousSibling = node.getPreviousSibling(); - while(previousSibling != null && !this.isSignificantNode(previousSibling)) { - previousSibling = previousSibling.getPreviousSibling(); - } - if(previousSibling == null) { - return new NodeListModel(Collections.emptyList(), null); + } else if (key.equals("**")) { + return new NodeListModel(((Element) node).getElementsByTagName("*"), this); + } else if (key.startsWith("@")) { + if (key.startsWith("@@")) { + if (key.equals(AtAtKey.ATTRIBUTES.getKey())) { + return new NodeListModel(node.getAttributes(), this); + } else if (key.equals(AtAtKey.START_TAG.getKey())) { + NodeOutputter nodeOutputter = new NodeOutputter(node); + return new SimpleScalar(nodeOutputter.getOpeningTag((Element) node)); + } else if (key.equals(AtAtKey.END_TAG.getKey())) { + NodeOutputter nodeOutputter = new NodeOutputter(node); + return new SimpleScalar(nodeOutputter.getClosingTag((Element) node)); + } else if (key.equals(AtAtKey.ATTRIBUTES_MARKUP.getKey())) { + StringBuilder buf = new StringBuilder(); + NodeOutputter nu = new NodeOutputter(node); + nu.outputContent(node.getAttributes(), buf); + return new SimpleScalar(buf.toString().trim()); + } else if (key.equals(AtAtKey.PREVIOUS_SIGNIFICANT.getKey())) { + Node previousSibling = node.getPreviousSibling(); + while(previousSibling != null && !this.isSignificantNode(previousSibling)) { + previousSibling = previousSibling.getPreviousSibling(); + } + if(previousSibling == null) { + return new NodeListModel(Collections.emptyList(), null); + } else { + return wrap(previousSibling); + } + } else if (key.equals(AtAtKey.NEXT_SIGNIFICANT.getKey())) { + Node nextSibling = node.getNextSibling(); + while(nextSibling != null && !this.isSignificantNode(nextSibling)) { + nextSibling = nextSibling.getNextSibling(); + } + if(nextSibling == null) { + return new NodeListModel(Collections.emptyList(), null); + } else { + return wrap(nextSibling); + } } else { - return wrap(previousSibling); + // We don't anything like this that's element-specific; fall back + return super.get(key); } - } - if (key.equals("@@next_significant")) { - Node nextSibling = node.getNextSibling(); - while(nextSibling != null && !this.isSignificantNode(nextSibling)) { - nextSibling = nextSibling.getNextSibling(); - } - if(nextSibling == null) { - return new NodeListModel(Collections.emptyList(), null); - } - else { - return wrap(nextSibling); - } - } - if (StringUtil.isXMLID(key.substring(1))) { - Attr att = getAttribute(key.substring(1)); - if (att == null) { - return new NodeListModel(this); + } else { // Starts with "@", but not with "@@" + if (DomStringUtil.isXMLID(key, 1)) { + Attr att = getAttribute(key.substring(1)); + if (att == null) { + return new NodeListModel(this); + } + return wrap(att); + } else if (key.equals("@*")) { + return new NodeListModel(node.getAttributes(), this); + } else { + // We don't anything like this that's element-specific; fall back + return super.get(key); } - return wrap(att); } - } - if (StringUtil.isXMLID(key)) { + } else if (DomStringUtil.isXMLID(key)) { + // We interpret key as an element name NodeListModel result = ((NodeListModel) getChildNodes()).filterByName(key); - if (result.size() == 1) { - return result.get(0); - } - return result; + return result.size() != 1 ? result : result.get(0); + } else { + // We don't anything like this that's element-specific; fall back + return super.get(key); } - return super.get(key); } public boolean isSignificantNode(Node node) throws TemplateModelException { @@ -219,6 +219,6 @@ class ElementModel extends NodeModel implements TemplateScalarModel { } boolean matchesName(String name, Environment env) { - return StringUtil.matchesName(name, getNodeName(), getNodeNamespace(), env); + return DomStringUtil.matchesName(name, getNodeName(), getNodeNamespace(), env); } } \ No newline at end of file http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/55b09e8d/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 22e555a..4a37266 100644 --- a/src/main/java/freemarker/ext/dom/NodeListModel.java +++ b/src/main/java/freemarker/ext/dom/NodeListModel.java @@ -118,21 +118,34 @@ class NodeListModel extends SimpleSequence implements TemplateHashModel, _Unexpe NodeModel nm = (NodeModel) get(0); return nm.get(key); } - if (key.startsWith("@@") && - (key.equals("@@markup") - || key.equals("@@nested_markup") - || key.equals("@@text"))) { - StringBuilder result = new StringBuilder(); - for (int i = 0; i < size(); i++) { - NodeModel nm = (NodeModel) get(i); - TemplateScalarModel textModel = (TemplateScalarModel) nm.get(key); - result.append(textModel.getAsString()); + if (key.startsWith("@@")) { + if (key.equals(AtAtKey.MARKUP.getKey()) + || key.equals(AtAtKey.NESTED_MARKUP.getKey()) + || key.equals(AtAtKey.TEXT.getKey())) { + StringBuilder result = new StringBuilder(); + for (int i = 0; i < size(); i++) { + NodeModel nm = (NodeModel) get(i); + TemplateScalarModel textModel = (TemplateScalarModel) nm.get(key); + result.append(textModel.getAsString()); + } + return new SimpleScalar(result.toString()); + } else if (key.length() != 2 /* to allow "@@" to fall through */) { + // As @@... would cause exception in the XPath engine, we throw a nicer exception now. + if (AtAtKey.containsKey(key)) { + throw new TemplateModelException( + "\"" + key + "\" is only applicable to a single XML node, but it was applied on " + + (size() != 0 + ? size() + " XML nodes (multiple matches)." + : "an empty list of XML nodes (no matches).")); + } else { + throw new TemplateModelException("Unsupported @@ key: " + key); + } } - return new SimpleScalar(result.toString()); } - if (StringUtil.isXMLID(key) - || ((key.startsWith("@") && StringUtil.isXMLID(key.substring(1)))) - || key.equals("*") || key.equals("**") || key.equals("@@") || key.equals("@*")) { + if (DomStringUtil.isXMLID(key) + || ((key.startsWith("@") + && (DomStringUtil.isXMLID(key, 1) || key.equals("@@") || key.equals("@*")))) + || key.equals("*") || key.equals("**")) { NodeListModel result = new NodeListModel(contextNode); for (int i = 0; i < size(); i++) { NodeModel nm = (NodeModel) get(i); @@ -155,12 +168,11 @@ class NodeListModel extends SimpleSequence implements TemplateHashModel, _Unexpe if (xps != null) { Object context = (size() == 0) ? null : rawNodeList(); return xps.executeQuery(context, key); + } else { + throw new TemplateModelException( + "Can't try to resolve the XML query key, because no XPath support is available. " + + "This is either malformed or an XPath expression: " + key); } - throw new TemplateModelException("Key: '" + key + "' is not legal for a node sequence (" - + this.getClass().getName() + "). This node sequence contains " + size() + " node(s). " - + "Some keys are valid only for node sequences of size 1. " - + "If you use Xalan (instead of Jaxen), XPath expression keys work only with " - + "node lists of size 1."); } private List rawNodeList() throws TemplateModelException { http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/55b09e8d/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 4198e2f..5e59a89 100644 --- a/src/main/java/freemarker/ext/dom/NodeModel.java +++ b/src/main/java/freemarker/ext/dom/NodeModel.java @@ -72,8 +72,8 @@ import freemarker.template.TemplateSequenceModel; * 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. + * Note that this class can't be used to represent a result set of 0 or multiple nodes (we use {@link NodeListModel} + * for that), but should be used to represent a node set of exactly 1 node instead of {@link NodeListModel}. */ abstract public class NodeModel implements TemplateNodeModelEx, TemplateHashModel, TemplateSequenceModel, @@ -257,44 +257,48 @@ implements TemplateNodeModelEx, TemplateHashModel, TemplateSequenceModel, public TemplateModel get(String key) throws TemplateModelException { if (key.startsWith("@@")) { - if (key.equals("@@text")) { + if (key.equals(AtAtKey.TEXT.getKey())) { return new SimpleScalar(getText(node)); - } - if (key.equals("@@namespace")) { + } else if (key.equals(AtAtKey.NAMESPACE.getKey())) { String nsURI = node.getNamespaceURI(); return nsURI == null ? null : new SimpleScalar(nsURI); - } - if (key.equals("@@local_name")) { + } else if (key.equals(AtAtKey.LOCAL_NAME.getKey())) { String localName = node.getLocalName(); if (localName == null) { localName = getNodeName(); } return new SimpleScalar(localName); - } - if (key.equals("@@markup")) { + } else if (key.equals(AtAtKey.MARKUP.getKey())) { StringBuilder buf = new StringBuilder(); NodeOutputter nu = new NodeOutputter(node); nu.outputContent(node, buf); return new SimpleScalar(buf.toString()); - } - if (key.equals("@@nested_markup")) { + } else if (key.equals(AtAtKey.NESTED_MARKUP.getKey())) { StringBuilder buf = new StringBuilder(); NodeOutputter nu = new NodeOutputter(node); nu.outputContent(node.getChildNodes(), buf); return new SimpleScalar(buf.toString()); - } - if (key.equals("@@qname")) { + } else if (key.equals(AtAtKey.QNAME.getKey())) { String qname = getQualifiedName(); - return qname == null ? null : new SimpleScalar(qname); + return qname != null ? new SimpleScalar(qname) : null; + } else { + // As @@... would cause exception in the XPath engine, we throw a nicer exception now. + if (AtAtKey.containsKey(key)) { + throw new TemplateModelException( + "\"" + key + "\" is not supported for an XML node of type \"" + getNodeType() + "\"."); + } else { + throw new TemplateModelException("Unsupported @@ key: " + key); + } } - } - XPathSupport xps = getXPathSupport(); - if (xps != null) { - return xps.executeQuery(node, key); } else { - throw new TemplateModelException( - "Can't try to resolve the XML query key, because no XPath support is available. " - + "It's either malformed or an XPath expression: " + key); + XPathSupport xps = getXPathSupport(); + if (xps != null) { + return xps.executeQuery(node, key); + } else { + throw new TemplateModelException( + "Can't try to resolve the XML query key, because no XPath support is available. " + + "This is either malformed or an XPath expression: " + key); + } } } http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/55b09e8d/src/main/java/freemarker/ext/dom/_ExtDomApi.java ---------------------------------------------------------------------- diff --git a/src/main/java/freemarker/ext/dom/_ExtDomApi.java b/src/main/java/freemarker/ext/dom/_ExtDomApi.java new file mode 100644 index 0000000..70ec0e4 --- /dev/null +++ b/src/main/java/freemarker/ext/dom/_ExtDomApi.java @@ -0,0 +1,43 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package freemarker.ext.dom; + +import freemarker.core.Environment; + +/** + * For internal use only; don't depend on this, there's no backward compatibility guarantee at all! + * This class is to work around the lack of module system in Java, i.e., so that other FreeMarker packages can + * access things inside this package that users shouldn't. + */ +public final class _ExtDomApi { + + private _ExtDomApi() { + // Not meant to be called + } + + static public boolean isXMLID(String name) { + return DomStringUtil.isXMLID(name); + } + + static public boolean matchesName(String qname, String nodeName, String nsURI, Environment env) { + return DomStringUtil.matchesName(qname, nodeName, nsURI, env); + } + +} http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/55b09e8d/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 88811f4..27db53f 100644 --- a/src/main/java/freemarker/template/utility/StringUtil.java +++ b/src/main/java/freemarker/template/utility/StringUtil.java @@ -30,6 +30,7 @@ import java.util.regex.Pattern; import freemarker.core.Environment; import freemarker.core.ParseException; +import freemarker.ext.dom._ExtDomApi; import freemarker.template.Template; import freemarker.template.Version; @@ -1656,55 +1657,20 @@ public class StringUtil { * 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) + * @return whether the name is a valid XML element name. (This routine might only be 99% accurate. 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) { - int ln = name.length(); - for (int i = 0; i < ln; i++) { - char c = name.charAt(i); - if (i == 0 && (c == '-' || c == '.' || Character.isDigit(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; + return _ExtDomApi.isXMLID(name); } /** - * @return whether the qname matches the combination of nodeName, nsURI, and environment prefix settings. + * @return whether the qname matches the combination of nodeName, nsURI, and environment prefix settings. */ - static public boolean matchesName(String qname, String nodeName, String nsURI, Environment env) { - String defaultNS = env.getDefaultNS(); - if ((defaultNS != null) && defaultNS.equals(nsURI)) { - return qname.equals(nodeName) - || qname.equals(Template.DEFAULT_NAMESPACE_PREFIX + ":" + nodeName); - } - if ("".equals(nsURI)) { - if (defaultNS != null) { - return qname.equals(Template.NO_NS_PREFIX + ":" + nodeName); - } else { - return qname.equals(nodeName) || qname.equals(Template.NO_NS_PREFIX + ":" + nodeName); - } - } - String prefix = env.getPrefixForNamespace(nsURI); - if (prefix == null) { - return false; // Is this the right thing here??? - } - return qname.equals(prefix + ":" + nodeName); + return _ExtDomApi.matchesName(qname, nodeName, nsURI, env); } /** http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/55b09e8d/src/manual/en_US/book.xml ---------------------------------------------------------------------- diff --git a/src/manual/en_US/book.xml b/src/manual/en_US/book.xml index fd3c204..929801c 100644 --- a/src/manual/en_US/book.xml +++ b/src/manual/en_US/book.xml @@ -7,9 +7,9 @@ to you under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at - + http://www.apache.org/licenses/LICENSE-2.0 - + Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY @@ -26662,6 +26662,14 @@ TemplateModel x = env.getVariable("x"); // get variable x</programlisting> significantly faster (be removing the overhead caused be throwing and then catching an exception).</para> </listitem> + + <listitem> + <para>Better error messages when someone tries to get an invalid + <literal>@@<replaceable>...</replaceable></literal> subvariable + of an XML DOM node. (Now it's not issued by the XPath + implementation, which just sees it as a syntactical + error.)</para> + </listitem> </itemizedlist> </section> </section> http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/55b09e8d/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 index 8079d90..1d2ceab 100644 --- a/src/test/java/freemarker/ext/dom/DOMTest.java +++ b/src/test/java/freemarker/ext/dom/DOMTest.java @@ -107,4 +107,16 @@ public class DOMTest extends TemplateTest { addToDataModel("doc", doc); } + @Test + public void testInvalidAtAtKeyErrors() throws Exception { + addDocToDataModel("<r><multipleMatches /><multipleMatches /></r>"); + assertErrorContains("${doc.r.@@invalid_key}", "Unsupported @@ key", "@invalid_key"); + assertErrorContains("${doc.@@start_tag}", "@@start_tag", "not supported", "document"); + assertErrorContains("${doc.@@}", "\"@@\"", "not supported", "document"); + assertErrorContains("${doc.r.noMatch.@@invalid_key}", "Unsupported @@ key", "@invalid_key"); + assertErrorContains("${doc.r.multipleMatches.@@invalid_key}", "Unsupported @@ key", "@invalid_key"); + assertErrorContains("${doc.r.noMatch.@@attributes_markup}", "single XML node", "@@attributes_markup"); + assertErrorContains("${doc.r.multipleMatches.@@attributes_markup}", "single XML node", "@@attributes_markup"); + } + }
