web: URLs that cannot be unescaped should be allowed

Fixes a regression in version 4.4

Fixes #1770

diff --git a/tornado/test/web_test.py b/tornado/test/web_test.py
index 745d0fa84e0575150524208bdeee355216da7acd..fdd1797cca0d373b8384be317b3803f17de9fb49 100644 (file)
--- a/tornado/test/web_test.py
+++ b/tornado/test/web_test.py
@@ -2811,3 +2811,26 @@ class ApplicationTest(AsyncTestCase):
 class URLSpecReverseTest(unittest.TestCase):
     def test_reverse(self):
         self.assertEqual('/favicon.ico', url(r'/favicon\.ico', None).reverse())
+        self.assertEqual('/favicon.ico', url(r'^/favicon\.ico$', None).reverse())
+
+    def test_non_reversible(self):
+        # URLSpecs are non-reversible if they include non-constant
+        # regex features outside capturing groups. Currently, this is
+        # only strictly enforced for backslash-escaped character
+        # classes.
+        paths = [
+            r'^/api/v\d+/foo/(\w+)$',
+        ]
+        for path in paths:
+            # A URLSpec can still be created even if it cannot be reversed.
+            url_spec = url(path, None)
+            try:
+                result = url_spec.reverse()
+                self.fail("did not get expected exception when reversing %s. "
+                          "result: %s" % (path, result))
+            except ValueError:
+                pass
+
+    def test_reverse_arguments(self):
+        self.assertEqual('/api/v1/foo/bar',
+                         url(r'^/api/v1/foo/(\w+)$', None).reverse('bar'))

diff --git a/tornado/web.py b/tornado/web.py
index 479dd2457cfa4c1cfed3713ed1b97278ed9e8dff..f54c4d039d98560cbdae1d3cc7654534126b17b8 100644 (file)
--- a/tornado/web.py
+++ b/tornado/web.py
@@ -3059,14 +3059,16 @@ class URLSpec(object):
                 try:
                     unescaped_fragment = re_unescape(fragment)
                 except ValueError as exc:
-                    raise ValueError(exc.args[0] + '; invalid url: %r' % pattern)
+                    # If we can't unescape part of it, we can't
+                    # reverse this url.
+                    return (None, None)
                 pieces.append(unescaped_fragment)
 
         return (''.join(pieces), self.regex.groups)
 
     def reverse(self, *args):
-        assert self._path is not None, \
-            "Cannot reverse url regex " + self.regex.pattern
+        if self._path is None:
+            raise ValueError("Cannot reverse url regex " + self.regex.pattern)
         assert len(args) == self._group_count, "required number of arguments "\
             "not found"
         if not len(args):