From defd036fdb17ce7d5621d9ca7373810039282ff7 Mon Sep 17 00:00:00 2001 From: Manolo Carrasco Date: Fri, 8 Jul 2011 15:08:42 +0000 Subject: [PATCH] Fix a NPE when passing non-native selectors to xpath engine (fixes issue87). Validate xpath syntax when generating xpath selectors. --- .../client/impl/SelectorEngineCssToXPath.java | 16 ++--- .../rebind/SelectorGeneratorCssToXPath.java | 66 +++++++++++++++++-- .../query/rebind/SelectorGeneratorsTest.java | 43 ++++++++++-- 3 files changed, 104 insertions(+), 21 deletions(-) diff --git a/gwtquery-core/src/main/java/com/google/gwt/query/client/impl/SelectorEngineCssToXPath.java b/gwtquery-core/src/main/java/com/google/gwt/query/client/impl/SelectorEngineCssToXPath.java index 32e4965b..7426aa29 100644 --- a/gwtquery-core/src/main/java/com/google/gwt/query/client/impl/SelectorEngineCssToXPath.java +++ b/gwtquery-core/src/main/java/com/google/gwt/query/client/impl/SelectorEngineCssToXPath.java @@ -93,7 +93,7 @@ public class SelectorEngineCssToXPath extends SelectorEngineImpl { } }; - private static Object[] regs = new Object[]{ + public static Object[] regs = new Object[]{ // scape some dots and spaces "(['\\[])([^'\\]]+)([\\s\\.#])([^'\\]]+)(['\\]])", rc_scp, // add @ for attrib @@ -204,23 +204,21 @@ public class SelectorEngineCssToXPath extends SelectorEngineImpl { public NodeList select(String sel, Node ctx) { if (cache == null) { - JsNamedArray.create(); + cache = JsNamedArray.create(); } - JsNodeArray elm = JsNodeArray.create(); String xsel = cache.get(sel); if (xsel == null) { xsel = sel.startsWith("./") || sel.startsWith("/") ? sel : css2Xpath(sel); cache.put(sel, xsel); } + + JsNodeArray elm = JsNodeArray.create(); try { SelectorEngine.xpathEvaluate(xsel, ctx, elm); + return JsUtils.unique(elm.> cast()).cast(); } catch (Exception e) { - if (sel.startsWith("./") || sel.startsWith("/")) { - System.err.println("ERROR: xpathEvaluate: " + sel + " xpath: " + xsel + - "\nIf the syntax of your css selector is correct, report the error to gquery team.\n\n" + e.getMessage()); - } + System.err.println("ERROR: xpathEvaluate invalid xpath expression:" + xsel + " css-selector:" + sel + "\n\n" + e.getMessage()); + return elm; } - return JsUtils.unique(elm.> cast()).cast(); } - } diff --git a/gwtquery-core/src/main/java/com/google/gwt/query/rebind/SelectorGeneratorCssToXPath.java b/gwtquery-core/src/main/java/com/google/gwt/query/rebind/SelectorGeneratorCssToXPath.java index dbed7177..6164fb0f 100644 --- a/gwtquery-core/src/main/java/com/google/gwt/query/rebind/SelectorGeneratorCssToXPath.java +++ b/gwtquery-core/src/main/java/com/google/gwt/query/rebind/SelectorGeneratorCssToXPath.java @@ -20,6 +20,10 @@ import java.util.regex.MatchResult; import java.util.regex.Matcher; import java.util.regex.Pattern; +import javax.xml.xpath.XPath; +import javax.xml.xpath.XPathExpressionException; +import javax.xml.xpath.XPathFactory; + import com.google.gwt.core.ext.TreeLogger; import com.google.gwt.core.ext.UnableToCompleteException; import com.google.gwt.core.ext.typeinfo.JMethod; @@ -27,6 +31,7 @@ import com.google.gwt.query.client.Selector; import com.google.gwt.query.client.impl.SelectorEngineCssToXPath; import com.google.gwt.query.client.impl.SelectorEngineCssToXPath.ReplaceCallback; import com.google.gwt.query.client.impl.SelectorEngineCssToXPath.Replacer; +import com.google.gwt.regexp.shared.RegExp; import com.google.gwt.user.rebind.SourceWriter; /** @@ -39,7 +44,7 @@ public class SelectorGeneratorCssToXPath extends SelectorGeneratorBase { /** * The replacer implementation for the JVM. */ - public static final Replacer replacer = new Replacer() { + public static final Replacer replacerJvm = new Replacer() { public String replaceAll(String s, String r, Object o) { Pattern p = Pattern.compile(r); if (o instanceof ReplaceCallback) { @@ -50,17 +55,46 @@ public class SelectorGeneratorCssToXPath extends SelectorGeneratorBase { ArrayList argss = new ArrayList(); for (int i = 0; i < matchResult.groupCount() + 1; i++) { argss.add(matchResult.group(i)); - } + } final String replacement = callback.foundMatch(argss); - s = s.substring(0, matchResult.start()) + replacement + s.substring(matchResult.end()); + s = s.substring(0, matchResult.start()) + replacement + + s.substring(matchResult.end()); matcher.reset(s); } return s; } else { - return p.matcher(s).replaceAll(o.toString()); + return p.matcher(s).replaceAll(o.toString()); + } + } + }; + + /** + * A replacer which works in both sides. Right now gquery JsRegexp is faster + * than gwt shared RegExp and does not uses HashSet + */ + public static final Replacer replacerGwt = new Replacer() { + public String replaceAll(String s, String r, Object o) { + RegExp p = RegExp.compile(r, "g"); + if (o instanceof ReplaceCallback) { + ReplaceCallback callback = (ReplaceCallback) o; + com.google.gwt.regexp.shared.MatchResult a = null; + while ((a = p.exec(s)) != null) { + ArrayList args = new ArrayList(); + for (int i = 0; i < a.getGroupCount(); i++) { + args.add(a.getGroup(i)); + } + String f = callback.foundMatch(args); + s = s.replace(a.getGroup(0), f); + p = RegExp.compile(r, "g"); + } + return s; + } else { + return p.replace(s, o.toString()); } } }; + + public static final Replacer replacer = replacerGwt; private SelectorEngineCssToXPath engine = new SelectorEngineCssToXPath( replacer); @@ -69,18 +103,38 @@ public class SelectorGeneratorCssToXPath extends SelectorGeneratorBase { return engine.css2Xpath(s); } + private XPathFactory factory = XPathFactory.newInstance(); + private XPath xpath = factory.newXPath(); + protected void generateMethodBody(SourceWriter sw, JMethod method, TreeLogger treeLogger, boolean hasContext) throws UnableToCompleteException { String selector = method.getAnnotation(Selector.class).value(); + String xselector = css2Xpath(selector); + + // Validate the generated xpath selector. + try { + validateXpath(xselector); + } catch (XPathExpressionException e1) { + System.err.println("Invalid XPath generated selector, please revise it: " + xselector); + if (!selector.equals(xselector)) { + System.err.println("If your css2 selector syntax is correct, open an issue in the gwtquery project. cssselector:" + + selector + " xpath:" + xselector); + } + throw new UnableToCompleteException(); + } + sw.println("return " - + wrap(method, "SelectorEngine.xpathEvaluate(\"" + css2Xpath(selector) + + wrap(method, "SelectorEngine.xpathEvaluate(\"" + xselector + "\", root)") + ";"); } + + public void validateXpath(String xselector) throws XPathExpressionException { + xpath.compile(xselector); + } protected String getImplSuffix() { return "CssToXPath" + super.getImplSuffix(); } - } diff --git a/gwtquery-core/src/test/java/com/google/gwt/query/rebind/SelectorGeneratorsTest.java b/gwtquery-core/src/test/java/com/google/gwt/query/rebind/SelectorGeneratorsTest.java index 26434b58..a8955eac 100644 --- a/gwtquery-core/src/test/java/com/google/gwt/query/rebind/SelectorGeneratorsTest.java +++ b/gwtquery-core/src/test/java/com/google/gwt/query/rebind/SelectorGeneratorsTest.java @@ -17,7 +17,12 @@ package com.google.gwt.query.rebind; import java.util.ArrayList; +import javax.xml.xpath.XPath; +import javax.xml.xpath.XPathExpressionException; +import javax.xml.xpath.XPathFactory; + import com.google.gwt.junit.client.GWTTestCase; +import com.google.gwt.query.client.impl.SelectorEngineCssToXPath; import com.google.gwt.query.client.impl.SelectorEngineCssToXPath.ReplaceCallback; /** @@ -26,10 +31,27 @@ import com.google.gwt.query.client.impl.SelectorEngineCssToXPath.ReplaceCallback public class SelectorGeneratorsTest extends GWTTestCase { public String getModuleName() { - return null; + return null; + } + + public void assertReplacers(String s) { + Object[] regs = SelectorEngineCssToXPath.regs; + String r1 = s, r2 = s; + for (int i = 0; i < regs.length;) { + r1 = SelectorGeneratorCssToXPath.replacerJvm.replaceAll(r1, regs[i].toString(), regs[i+1]); + r2 = SelectorGeneratorCssToXPath.replacer.replaceAll(r2, regs[i].toString(), regs[i+1]); + i+=2; + } + assertEquals(r1, r2); + } + + public void testReplacers() { + assertReplacers("table:contains('String With | @ ~= Space Points.s and Hash#es')"); } + - public void testCss2Xpath() { + public void testCss2Xpath() throws Exception { + SelectorGeneratorCssToXPath sel = new SelectorGeneratorCssToXPath(); assertEquals(".//div[starts-with(@class,'exa') and (substring(@class,string-length(@class)-3)='mple')]", @@ -73,15 +95,24 @@ public class SelectorGeneratorsTest extends GWTTestCase { assertEquals(".//table[contains(string(.),'String With | @ ~= Space Points.s and Hash#es')]", sel.css2Xpath("table:contains('String With | @ ~= Space Points.s and Hash#es')")); - assertEquals(".//a[starts-with(@href,'http') and (contains(@href,'youtube.com/'))]", - sel.css2Xpath("a[href^=http][href*=youtube.com/]")); + String xsel = sel.css2Xpath("div[@class='comment']:contains('John')"); + sel.validateXpath(xsel); + assertEquals(".//div[@class='comment' and (contains(string(.),'John'))]", xsel); } public void testReplaceAll() { - assertEquals(" ", SelectorGeneratorCssToXPath.replacer.replaceAll("/[thumb01]/ /[thumb03]/", "/\\[thumb(\\d+)\\]/", new ReplaceCallback() { + assertEquals(" ", + SelectorGeneratorCssToXPath.replacerGwt.replaceAll("/[thumb01]/ /[thumb03]/", "/\\[thumb(\\d+)\\]/", new ReplaceCallback() { public String foundMatch(ArrayLists) { - return ""; + return ""; } })); } + + public void testValidation() throws XPathExpressionException { + SelectorGeneratorCssToXPath sel = new SelectorGeneratorCssToXPath(); + XPathFactory factory = XPathFactory.newInstance(); + XPath xpath = factory.newXPath(); + xpath.compile(sel.css2Xpath("a[href^=http][href*=youtube.com/]")); + } } -- 2.39.5