- Cause ``:segment`` matches in route paths to put a Unicode-decoded

  and URL-dequoted value in the matchdict for the value matched.
  Previously a non-decoded non-URL-dequoted string was placed in the
  matchdict as the value.

- Cause ``*remainder`` matches in route paths to put a *tuple* in the
  matchdict dictionary in order to be able to present Unicode-decoded
  and URL-dequoted values for the traversal path.  Previously a
  non-decoded non-URL-dequoted string was placed in the matchdict as
  the value.

7 files changed, 115 insertions, 21 deletions

diff --git a/CHANGES.txt b/CHANGES.txt
index 783349c81..4c477ac71 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -4,6 +4,17 @@ Next release
 Features
 --------
 
+- Cause ``:segment`` matches in route paths to put a Unicode-decoded
+  and URL-dequoted value in the matchdict for the value matched.
+  Previously a non-decoded non-URL-dequoted string was placed in the
+  matchdict as the value.
+
+- Cause ``*remainder`` matches in route paths to put a *tuple* in the
+  matchdict dictionary in order to be able to present Unicode-decoded
+  and URL-dequoted values for the traversal path.  Previously a
+  non-decoded non-URL-dequoted string was placed in the matchdict as
+  the value.
+
 - Add optional ``max_age`` keyword value to the ``remember`` method of
   ``repoze.bfg.authentication.AuthTktAuthenticationPolicy``; if this
   value is passed to ``remember``, the generated cookie will have a
diff --git a/docs/narr/urldispatch.rst b/docs/narr/urldispatch.rst
index 6c1d7fe2b..57910839f 100644
--- a/docs/narr/urldispatch.rst
+++ b/docs/narr/urldispatch.rst
@@ -142,16 +142,32 @@ segment ("foo") and two dynamic segments ("baz", and "bar")::
 The above pattern will match these URLs, generating the followng
 matchdicts::
 
-   foo/1/2        -> {'baz':1, 'bar':2}
-   foo/abc/def    -> {'baz':abc, 'bar':2}
+   foo/1/2        -> {'baz':u'1', 'bar':u'2'}
+   foo/abc/def    -> {'baz':u'abc', 'bar':u'2'}
 
 It will not match the following patterns however:
 
    foo/1/2/        -> No match (trailing slash)
    bar/abc/def     -> First segment literal mismatch
 
+Note that values representing path segments matched with a
+``:segment`` match will be url-unquoted and decoded from UTF-8 into
+Unicode within the matchdict.  So for instance, the following
+pattern::
+
+    foo/:bar
+
+When matching the following URL::
+
+    foo/La%20Pe%C3%B1a
+
+The matchdict will look like so (the value is URL-decoded / UTF-8
+decoded)::
+
+  {'bar':u'La Pe\xf1a'}
+
 If the pattern has a ``*`` in it, the name which follows it is
-considered a "remainder match".  A remainder match must come at the
+considered a "remainder match".  A remainder match *must* come at the
 end of the path pattern.  Unlike segment replacement markers, it does
 not need to be preceded by a slash.  For example::
 
@@ -160,8 +176,24 @@ not need to be preceded by a slash.  For example::
 The above pattern will match these URLs, generating the followng
 matchdicts::
 
-   foo/1/2/               -> {'baz':1, 'bar':2, 'traverse':'/'}
-   foo/abc/def/a/b/c/d    -> {'baz':abc, 'bar':2, 'traverse':'/a/b/c/d'}
+   foo/1/2/               -> {'baz':1, 'bar':2, 'traverse':()}
+   foo/abc/def/a/b/c      -> {'baz':abc, 'bar':2, 'traverse':('a', 'b', 'c')}
+
+Note that when a ``*stararg`` remainder match is matched, the value
+put into the matchdict is turned into a tuple of path segments
+representing the remainder of the path.  These path segments are
+url-unquoted and decoded from UTF-8 into Unicode.  For example, for
+the following pattern::
+
+    foo/*traverse
+
+When matching the following path::
+
+    /foo/La%20Pe%C3%B1a/a/b/c
+
+Will generate the following matchdict::
+
+  {'traverse':(u'foo', u'La Pe\xf1a', u'a', u'b', u'c')}
 
 Example 1
 ---------
diff --git a/repoze/bfg/tests/test_traversal.py b/repoze/bfg/tests/test_traversal.py
index 68b920fa4..4a4080aab 100644
--- a/repoze/bfg/tests/test_traversal.py
+++ b/repoze/bfg/tests/test_traversal.py
@@ -270,7 +270,7 @@ class ModelGraphTraverserTests(unittest.TestCase):
         self.assertEqual(result['virtual_root'], model)
         self.assertEqual(result['virtual_root_path'], ())
 
-    def test_withroute_with_subpath(self):
+    def test_withroute_with_subpath_string(self):
         model = DummyContext()
         traverser = self._makeOne(model)
         environ = {'bfg.routes.matchdict': {'subpath':'/a/b/c'}}
@@ -283,7 +283,20 @@ class ModelGraphTraverserTests(unittest.TestCase):
         self.assertEqual(result['virtual_root'], model)
         self.assertEqual(result['virtual_root_path'], ())
 
-    def test_withroute_and_traverse(self):
+    def test_withroute_with_subpath_tuple(self):
+        model = DummyContext()
+        traverser = self._makeOne(model)
+        environ = {'bfg.routes.matchdict': {'subpath':('a', 'b', 'c')}}
+        result = traverser(environ)
+        self.assertEqual(result['context'], model)
+        self.assertEqual(result['view_name'], '')
+        self.assertEqual(result['subpath'], ('a', 'b','c'))
+        self.assertEqual(result['traversed'], ())
+        self.assertEqual(result['root'], model)
+        self.assertEqual(result['virtual_root'], model)
+        self.assertEqual(result['virtual_root_path'], ())
+
+    def test_withroute_and_traverse_string(self):
         model = DummyContext()
         traverser = self._makeOne(model)
         environ = {'bfg.routes.matchdict': {'traverse':'foo/bar'}}
@@ -296,6 +309,19 @@ class ModelGraphTraverserTests(unittest.TestCase):
         self.assertEqual(result['virtual_root'], model)
         self.assertEqual(result['virtual_root_path'], ())
 
+    def test_withroute_and_traverse_tuple(self):
+        model = DummyContext()
+        traverser = self._makeOne(model)
+        environ = {'bfg.routes.matchdict': {'traverse':('foo', 'bar')}}
+        result = traverser(environ)
+        self.assertEqual(result['context'], model)
+        self.assertEqual(result['view_name'], 'foo')
+        self.assertEqual(result['subpath'], ('bar',))
+        self.assertEqual(result['traversed'], ())
+        self.assertEqual(result['root'], model)
+        self.assertEqual(result['virtual_root'], model)
+        self.assertEqual(result['virtual_root_path'], ())
+
 class FindInterfaceTests(unittest.TestCase):
     def _callFUT(self, context, iface):
         from repoze.bfg.traversal import find_interface
diff --git a/repoze/bfg/tests/test_urldispatch.py b/repoze/bfg/tests/test_urldispatch.py
index 014401008..0bee548c5 100644
--- a/repoze/bfg/tests/test_urldispatch.py
+++ b/repoze/bfg/tests/test_urldispatch.py
@@ -132,10 +132,10 @@ class TestCompileRoute(unittest.TestCase):
     def test_with_star(self):
         matcher, generator = self._callFUT('/foo/:baz/biz/:buz/bar*traverse')
         self.assertEqual(matcher('/foo/baz/biz/buz/bar'),
-                         {'baz':'baz', 'buz':'buz', 'traverse':''})
+                         {'baz':'baz', 'buz':'buz', 'traverse':()})
         self.assertEqual(matcher('/foo/baz/biz/buz/bar/everything/else/here'),
                          {'baz':'baz', 'buz':'buz',
-                          'traverse':'/everything/else/here'})
+                          'traverse':('everything', 'else', 'here')})
         self.assertEqual(matcher('foo/baz/biz/buz/bar'), None)
         self.assertEqual(generator(
             {'baz':1, 'buz':2, 'traverse':u'/a/b'}), '/foo/1/biz/2/bar/a/b')
@@ -167,9 +167,14 @@ class TestCompileRouteMatchFunctional(unittest.TestCase):
         self.matches('/:x', '/', {'x':''})
         self.matches('/:x', '/a', {'x':'a'})
         self.matches('zzz/:x', '/zzz/abc', {'x':'abc'})
-        self.matches('zzz/:x*traverse', '/zzz/abc', {'x':'abc', 'traverse':''})
-        self.matches('zzz/:x*traverse', '/zzz/abc/def/g', {'x':'abc',
-                                                         'traverse':'/def/g'})
+        self.matches('zzz/:x*traverse', '/zzz/abc', {'x':'abc', 'traverse':()})
+        self.matches('zzz/:x*traverse', '/zzz/abc/def/g',
+                     {'x':'abc', 'traverse':('def', 'g')})
+        self.matches('*traverse', '/zzz/abc', {'traverse':('zzz', 'abc')})
+        self.matches('*traverse', '/zzz/%20abc', {'traverse':('zzz', ' abc')})
+        self.matches(':x', '/La%20Pe%C3%B1a', {'x':u'La Pe\xf1a'})
+        self.matches('*traverse', '/La%20Pe%C3%B1a/x',
+                     {'traverse':(u'La Pe\xf1a', 'x')})
         
     def test_generator_functional(self):
         self.generates('', {}, '/')
@@ -186,8 +191,8 @@ class TestCompileRouteMatchFunctional(unittest.TestCase):
         self.generates('/:x*y', {'x':unicode('/La Pe\xc3\xb1a', 'utf-8'),
                                  'y':'/rest/of/path'},
                        '/%2FLa%20Pe%C3%B1a/rest/of/path')
-                       
-
+        self.generates('*traverse', {'traverse':('a', u'La Pe\xf1a')},
+                       '/a/La%20Pe%C3%B1a')
 
 class DummyRootFactory(object):
     def __init__(self, result):
diff --git a/repoze/bfg/traversal.py b/repoze/bfg/traversal.py
index b79f87d4c..374804df0 100644
--- a/repoze/bfg/traversal.py
+++ b/repoze/bfg/traversal.py
@@ -368,7 +368,7 @@ def virtual_root(model, request):
     urlgenerator = getMultiAdapter((model, request), IContextURL)
     return urlgenerator.virtual_root()
 
-@lru_cache(500)
+@lru_cache(1000)
 def traversal_path(path):
     """ Given a ``PATH_INFO`` string (slash-separated path segments),
     return a tuple representing that path which can be used to
@@ -493,9 +493,17 @@ class ModelGraphTraverser(object):
     def __call__(self, environ):
         if 'bfg.routes.matchdict' in environ:
             matchdict = environ['bfg.routes.matchdict']
+
             path = matchdict.get('traverse', '/')
-            subpath = matchdict.get('subpath', '')
-            subpath = tuple(filter(None, subpath.split('/')))
+            if hasattr(path, '__iter__'):
+                # this is a *traverse stararg (not a :traverse)
+                path = '/'.join([quote_path_segment(x) for x in path]) or '/'
+
+            subpath = matchdict.get('subpath', ())
+            if not hasattr(subpath, '__iter__'):
+                # this is not a *subpath stararg (just a :subpath)
+                subpath = traversal_path(subpath)
+            
         else:
             # this request did not match a Routes route
             subpath = ()
diff --git a/repoze/bfg/url.py b/repoze/bfg/url.py
index 7f505e659..12b86c786 100644
--- a/repoze/bfg/url.py
+++ b/repoze/bfg/url.py
@@ -29,7 +29,7 @@ def route_url(route_name, request, *elements, **kw):
         route_url('foobar', request, foo='1')          => <KeyError exception>
         route_url('foobar', request, foo='1', bar='2') => <KeyError exception>
         route_url('foobar', request, foo='1', bar='2',
-                   'traverse='a/b)                     =>  http://e.com/1/2/a/b
+                   'traverse='a/b')                    =>  http://e.com/1/2/a/b
 
     If a keyword argument ``_query`` is present, it will used to
     compose a query string that will be tacked on to the end of the
diff --git a/repoze/bfg/urldispatch.py b/repoze/bfg/urldispatch.py
index 885f0eb98..7a35f76f1 100644
--- a/repoze/bfg/urldispatch.py
+++ b/repoze/bfg/urldispatch.py
@@ -1,6 +1,9 @@
 import re
+from urllib import unquote
 
 from repoze.bfg.traversal import _url_quote
+from repoze.bfg.traversal import traversal_path
+from repoze.bfg.traversal import quote_path_segment
 
 _marker = object()
 
@@ -89,8 +92,15 @@ def _compile_route(route):
         m = match(path)
         if m is None:
             return m
-        return dict(item for item in m.groupdict().iteritems()
-                    if item[1] is not None)
+        d = {}
+        for k,v in m.groupdict().iteritems():
+            if k is not None:
+                if k == star:
+                    d[k] = traversal_path(v)
+                else:
+                    d[k] = unquote(v).decode('utf-8')
+        return d
+                    
 
     gen = ''.join(gen)
     def generator(dict):
@@ -98,7 +108,9 @@ def _compile_route(route):
         for k, v in dict.items():
             if isinstance(v, unicode):
                 v = v.encode('utf-8')
-            if (k!=star):
+            if k == star and hasattr(v, '__iter__'):
+                v = '/'.join([quote_path_segment(x) for x in v])
+            elif k != star:
                 try:
                     v = _url_quote(v)
                 except TypeError: