Merge pull request #2811 from moriyoshi/moriyoshi/more-safe-chars-for-url-gen

Mark more characters as safe in escaping generated path components

6 files changed, 34 insertions, 9 deletions

diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt
index bb21337e2..98e243c1f 100644
--- a/CONTRIBUTORS.txt
+++ b/CONTRIBUTORS.txt
@@ -284,3 +284,5 @@ Contributors
 - Jon Davidson, 2016/07/18
 
 - Keith Yang, 2016/07/22
+
+- Moriyoshi Koizumi, 2016/11/20
diff --git a/pyramid/tests/test_traversal.py b/pyramid/tests/test_traversal.py
index 0decd04d6..5fc878a32 100644
--- a/pyramid/tests/test_traversal.py
+++ b/pyramid/tests/test_traversal.py
@@ -1,3 +1,4 @@
+# -*- coding: utf-8 -*-
 import unittest
 import warnings
 
@@ -839,7 +840,7 @@ class QuotePathSegmentTests(unittest.TestCase):
     def test_string(self):
         s = '/ hello!'
         result = self._callFUT(s)
-        self.assertEqual(result, '%2F%20hello%21')
+        self.assertEqual(result, '%2F%20hello!')
 
     def test_int(self):
         s = 12345
@@ -1299,6 +1300,15 @@ class Test__join_path_tuple(unittest.TestCase):
         result = self._callFUT(('x',))
         self.assertEqual(result, 'x')
 
+    def test_segments_with_unsafes(self):
+        safe_segments = tuple(u"0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz-._~!$&'()*+,;=:@")
+        result = self._callFUT(safe_segments)
+        self.assertEqual(result, u'/'.join(safe_segments))
+        unsafe_segments = tuple(chr(i) for i in range(0x20, 0x80) if not chr(i) in safe_segments) + (u'あ',)
+        result = self._callFUT(unsafe_segments)
+        self.assertEqual(result, u'/'.join(''.join('%%%02X' % (ord(c) if isinstance(c, str) else c) for c in unsafe_segment.encode('utf-8')) for unsafe_segment in unsafe_segments))
+
+
 def make_traverser(result):
     class DummyTraverser(object):
         def __init__(self, context):
diff --git a/pyramid/tests/test_urldispatch.py b/pyramid/tests/test_urldispatch.py
index 2d20b24c3..06f4ad793 100644
--- a/pyramid/tests/test_urldispatch.py
+++ b/pyramid/tests/test_urldispatch.py
@@ -485,11 +485,15 @@ class TestCompileRouteFunctional(unittest.TestCase):
     def test_generator_functional_newstyle(self):
         self.generates('/{x}', {'x':''}, '/')
         self.generates('/{x}', {'x':'a'}, '/a')
+        self.generates('/{x}', {'x':'a/b/c'}, '/a/b/c')
+        self.generates('/{x}', {'x':':@&+$,'}, '/:@&+$,')
         self.generates('zzz/{x}', {'x':'abc'}, '/zzz/abc')
         self.generates('zzz/{x}*traverse', {'x':'abc', 'traverse':''},
                        '/zzz/abc')
         self.generates('zzz/{x}*traverse', {'x':'abc', 'traverse':'/def/g'},
                        '/zzz/abc/def/g')
+        self.generates('zzz/{x}*traverse', {'x':':@&+$,', 'traverse':'/:@&+$,'},
+                       '/zzz/:@&+$,/:@&+$,')
         self.generates('/{x}', {'x':text_(b'/La Pe\xc3\xb1a', 'utf-8')},
                        '//La%20Pe%C3%B1a')
         self.generates('/{x}*y', {'x':text_(b'/La Pe\xc3\xb1a', 'utf-8'),
diff --git a/pyramid/traversal.py b/pyramid/traversal.py
index 963a76bb5..1ca52692a 100644
--- a/pyramid/traversal.py
+++ b/pyramid/traversal.py
@@ -35,6 +35,9 @@ with warnings.catch_warnings():
     warnings.filterwarnings('ignore')
     from pyramid.interfaces import IContextURL
 
+PATH_SEGMENT_SAFE = "~!$&'()*+,;=:@" # from webob
+PATH_SAFE = PATH_SEGMENT_SAFE + "/"
+
 empty = text_('')
 
 def find_root(resource):
@@ -577,7 +580,7 @@ the ``safe`` argument to this function.  This corresponds to the
 
 if PY2:
     # special-case on Python 2 for speed?  unchecked
-    def quote_path_segment(segment, safe=''):
+    def quote_path_segment(segment, safe=PATH_SEGMENT_SAFE):
         """ %s """ % quote_path_segment_doc
         # The bit of this code that deals with ``_segment_cache`` is an
         # optimization: we cache all the computation of URL path segments
@@ -596,7 +599,7 @@ if PY2:
             _segment_cache[(segment, safe)] = result
             return result
 else:
-    def quote_path_segment(segment, safe=''):
+    def quote_path_segment(segment, safe=PATH_SEGMENT_SAFE):
         """ %s """ % quote_path_segment_doc
         # The bit of this code that deals with ``_segment_cache`` is an
         # optimization: we cache all the computation of URL path segments
diff --git a/pyramid/url.py b/pyramid/url.py
index 646cc857d..d6587e783 100644
--- a/pyramid/url.py
+++ b/pyramid/url.py
@@ -25,10 +25,11 @@ from pyramid.threadlocal import get_current_registry
 from pyramid.traversal import (
     ResourceURL,
     quote_path_segment,
+    PATH_SAFE,
+    PATH_SEGMENT_SAFE,
     )
 
-PATH_SAFE = '/:@&+$,' # from webob
-QUERY_SAFE = '/?:@!$&\'()*+,;=' # RFC 3986
+QUERY_SAFE = "/?:@!$&'()*+,;=" # RFC 3986
 ANCHOR_SAFE = QUERY_SAFE
 
 def parse_url_overrides(kw):
@@ -947,4 +948,4 @@ def current_route_path(request, *elements, **kw):
 
 @lru_cache(1000)
 def _join_elements(elements):
-    return '/'.join([quote_path_segment(s, safe=':@&+$,') for s in elements])
+    return '/'.join([quote_path_segment(s, safe=PATH_SEGMENT_SAFE) for s in elements])
diff --git a/pyramid/urldispatch.py b/pyramid/urldispatch.py
index c88ad9590..a61071845 100644
--- a/pyramid/urldispatch.py
+++ b/pyramid/urldispatch.py
@@ -22,6 +22,7 @@ from pyramid.exceptions import URLDecodeError
 from pyramid.traversal import (
     quote_path_segment,
     split_path_info,
+    PATH_SAFE,
     )
 
 _marker = object()
@@ -207,6 +208,10 @@ def _compile_route(route):
         return d
 
     gen = ''.join(gen)
+
+    def q(v):
+        return quote_path_segment(v, safe=PATH_SAFE)
+
     def generator(dict):
         newdict = {}
         for k, v in dict.items():
@@ -223,17 +228,17 @@ def _compile_route(route):
                 # a stararg argument
                 if is_nonstr_iter(v):
                     v = '/'.join(
-                        [quote_path_segment(x, safe='/') for x in v]
+                        [q(x) for x in v]
                         ) # native
                 else:
                     if v.__class__ not in string_types:
                         v = str(v)
-                    v = quote_path_segment(v, safe='/')
+                    v = q(v)
             else:
                 if v.__class__ not in string_types:
                     v = str(v)
                 # v may be bytes (py2) or native string (py3)
-                v = quote_path_segment(v, safe='/')
+                v = q(v)
 
             # at this point, the value will be a native string
             newdict[k] = v