On 2011/06/07 16:34:24, xtof wrote:
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.

That's possible. Given that the regex only checks each char to see if
it's in the set of valid chars, with the exception of surrogate pairs, I
think our best bet is to rewrite it without using a regexp (scanning the
string instead). We could also change it to a two-pass regexp check
(first check each char, allowing "surrogate chars", then in a second
pass check invalid surrogate pairs).
I'll see if I can work on it in the coming days.

    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

These should all remain fromTrustedString: they are unit tests for
fromTrustedString!

http://gwt-code-reviews.appspot.com/1443813/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Reply via email to