Thomas, here's a patch that adds a test case to repro the bug (also changing uses of fromTrustedString to fromSafeConstant where appropriate).
Would you be able to look into rewriting the regex? My best guess is that it's falling afoul of http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=5050507; alternation in the pattern resulting in recursion. Should I file an issue to track this? diff --git a/google3/third_party/java_src/gwt/svn/trunk/user/src/com/google/gwt/safehtml/shared/SafeUriHostedModeUtils.java b/google3/third_party/java_src/gwt/svn/trunk/user/src/com/google/gwt/safehtml/shared/SafeUriHostedModeUtils.java index 7b3dbe2..597e63c 100644 --- a/google3/third_party/java_src/gwt/svn/trunk/user/src/com/google/gwt/safehtml/shared/SafeUriHostedModeUtils.java +++ b/google3/third_party/java_src/gwt/svn/trunk/user/src/com/google/gwt/safehtml/shared/SafeUriHostedModeUtils.java @@ -104,11 +104,9 @@ public class SafeUriHostedModeUtils { } private static boolean isValidUri(String uri) { - // TODO(xtof): The regex appears to cause stack overflows in some cases. - // Investigate and re-enable. - // if (!uri.matches(HREF_UCSCHAR)) { - // return false; - // } + if (!uri.matches(HREF_UCSCHAR)) { + return false; + } /* * pre-process to turn href-ucschars into ucschars, and encode to URI. * diff --git a/google3/third_party/java_src/gwt/svn/trunk/user/test/com/google/gwt/safehtml/shared/GwtUriUtilsTest.java b/google3/third_party/java_src/gwt/svn/trunk/user/test/com/google/gwt/safehtml/shared/GwtUriUtilsTest.java index f76c66f..e8e2146 100644 --- a/google3/third_party/java_src/gwt/svn/trunk/user/test/com/google/gwt/safehtml/shared/GwtUriUtilsTest.java +++ b/google3/third_party/java_src/gwt/svn/trunk/user/test/com/google/gwt/safehtml/shared/GwtUriUtilsTest.java @@ -26,9 +26,25 @@ public class GwtUriUtilsTest extends GWTTestCase { private static final String JAVASCRIPT_URL = "javascript:alert('BOOM!');"; private static final String MAILTO_URL = "mailto:[email protected]";; private static final String CONSTANT_URL = - " http://gwt.google.com/samples/Showcase/Showcase.html?locale=fr#!CwCheckBox";; + " http://gwt.google.com/samples/Showcase/Showcase.html?locale=fr#!CwCheckBox";; private static final String EMPTY_GIF_DATA_URL = - "data:image/gif;base64,R0lGODlhAQABAPABAP///wAAACH5BAEKAAAALAAAAAABAAEAAAICRAEAOw=="; + "data:image/gif;base64,R0lGODlhAQABAPABAP///wAAACH5BAEKAAAALAAAAAABAAEAAAICRAEAOw=="; + private static final String LONG_DATA_URL = + "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAcAAAAHCAMAAADzjKfhAAAAGXRFWHRTb2Z0d2FyZQ" + + "BBZG9iZSBJbWFnZVJlYWR5ccllPAAAAwBQTFRFZmZm////AgICAwMDBAQEBQUFBgYGBwcHCAgICQkJCgoKCwsL" + + "DAwMDQ0NDg4ODw8PEBAQEREREhISExMTFBQUFRUVFhYWFxcXGBgYGRkZGhoaGxsbHBwcHR0dHh4eHx8fICAgIS" + + "EhIiIiIyMjJCQkJSUlJiYmJycnKCgoKSkpKioqKysrLCwsLS0tLi4uLy8vMDAwMTExMjIyMzMzNDQ0NTU1NjY2" + + "Nzc3ODg4OTk5Ojo6Ozs7PDw8PT09Pj4+Pz8/QEBAQUFBQkJCQ0NDRERERUVFRkZGR0dHSEhISUlJSkpKS0tLTE" + + "xMTU1NTk5OT09PUFBQUVFRUlJSU1NTVFRUVVVVVlZWV1dXWFhYWVlZWlpaW1tbXFxcXV1dXl5eX19fYGBgYWFh" + + "YmJiY2NjZGRkZWVlZmZmZ2dnaGhoaWlpampqa2trbGxsbW1tbm5ub29vcHBwcXFxcnJyc3NzdHR0dXV1dnZ2d3" + + "d3eHh4eXl5enp6e3t7fHx8fX19fn5+f39/gICAgYGBgoKCg4ODhISEhYWFhoaGh4eHiIiIiYmJioqKi4uLjIyM" + + "jY2Njo6Oj4+PkJCQkZGRkpKSk5OTlJSUlZWVlpaWl5eXmJiYmZmZmpqam5ubnJycnZ2dnp6en5+foKCgoaGhoq" + + "Kio6OjpKSkpaWlpqamp6enqKioqampqqqqq6urrKysra2trq6ur6+vsLCwsbGxsrKys7OztLS0tbW1tra2t7e3" + + "uLi4ubm5urq6u7u7vLy8vb29vr6+v7+/wMDAwcHBwsLCw8PDxMTExcXFxsbGx8fHyMjIycnJysrKy8vLzMzMzc" + + "3Nzs7Oz8/P0NDQ0dHR0tLS09PT1NTU1dXV1tbW19fX2NjY2dnZ2tra29vb3Nzc3d3d3t7e39/f4ODg4eHh4uLi" + + "4+Pj5OTk5eXl5ubm5+fn6Ojo6enp6urq6+vr7Ozs7e3t7u7u7+/v8PDw8fHx8vLy8/Pz9PT09fX19vb29/f3+P" + + "j4+fn5+vr6+/v7/Pz8/f39/v7+////AADF2QAAAAJ0Uk5T/wDltzBKAAAAH0lEQVR42mJghAAGGJ0GAQyMYAok" + + "DqLA8mlI6gACDAC8pAaCn/ezogAAAABJRU5ErkJggg=="; public void testEncode_noEscape() { StringBuilder sb = new StringBuilder(UriUtils.DONT_NEED_ENCODING); @@ -77,10 +93,11 @@ public class GwtUriUtilsTest extends GWTTestCase { } public void testFromTrustedString() { - assertEquals(CONSTANT_URL, UriUtils.fromTrustedString(CONSTANT_URL).asString()); - assertEquals(MAILTO_URL, UriUtils.fromTrustedString(MAILTO_URL).asString()); - assertEquals(EMPTY_GIF_DATA_URL, UriUtils.fromTrustedString(EMPTY_GIF_DATA_URL).asString()); - assertEquals(JAVASCRIPT_URL, UriUtils.fromTrustedString(JAVASCRIPT_URL).asString()); + assertEquals(CONSTANT_URL, UriUtils.fromSafeConstant(CONSTANT_URL).asString()); + assertEquals(MAILTO_URL, UriUtils.fromSafeConstant(MAILTO_URL).asString()); + assertEquals(EMPTY_GIF_DATA_URL, UriUtils.fromSafeConstant(EMPTY_GIF_DATA_URL).asString()); + assertEquals(LONG_DATA_URL, UriUtils.fromSafeConstant(LONG_DATA_URL).asString()); + assertEquals(JAVASCRIPT_URL, UriUtils.fromSafeConstant(JAVASCRIPT_URL).asString()); if (GWT.isClient()) { assertEquals(GWT.getModuleBaseURL(), UriUtils.fromTrustedString(GWT.getModuleBaseURL()).asString()); @@ -96,7 +113,7 @@ public class GwtUriUtilsTest extends GWTTestCase { return; } try { - SafeUri u = UriUtils.fromTrustedString("a\uD800b"); + SafeUri u = UriUtils.fromSafeConstant("a\uD800b"); fail("Should have thrown IllegalArgumentException"); } catch (IllegalArgumentException e) { // expected On Mon, Jun 6, 2011 at 14:47, <[email protected]> wrote: > Reviewers: tbroyer, jlabanca, jat, pdr, > > Description: > Disable regex checking of URLs in SafeUriHostedModeUtils; this regex > appears to > cause stack overflows in some cases. > > > Please review this at http://gwt-code-reviews.appspot.com/1443813/ > > Affected files: > M user/src/com/google/gwt/safehtml/shared/SafeUriHostedModeUtils.java > > > Index: user/src/com/google/gwt/safehtml/shared/SafeUriHostedModeUtils.java > =================================================================== > --- user/src/com/google/gwt/safehtml/shared/SafeUriHostedModeUtils.java > (revision 10284) > +++ user/src/com/google/gwt/safehtml/shared/SafeUriHostedModeUtils.java > (working copy) > @@ -104,9 +104,11 @@ > } > > private static boolean isValidUri(String uri) { > - if (!uri.matches(HREF_UCSCHAR)) { > - return false; > - } > + // TODO(xtof): The regex appears to cause stack overflows in some > cases. > + // Investigate and re-enable) > + // if (!uri.matches(HREF_UCSCHAR)) { > + // return false; > + // } > /* > * pre-process to turn href-ucschars into ucschars, and encode to URI. > * > > > -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
