Merge branch 'feature.custom-query-strings'

7 files changed, 177 insertions, 87 deletions

diff --git a/CHANGES.txt b/CHANGES.txt
index ad51ed174..d6f5ea792 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -48,6 +48,17 @@ Features
   timeouts, and conformance with the ``ISession`` API.
   See https://github.com/Pylons/pyramid/pull/1142
 
+- Allow ``pyramid.request.Request.route_url`` and
+  ``pyramid.request.Request.resource_url`` to accept strings for their
+  query string to enable alternative encodings. Also the anchor argument
+  will now be escaped to ensure minimal conformance.
+  See https://github.com/Pylons/pyramid/pull/1183
+
+- Allow sending of ``_query`` and ``_anchor`` options to
+  ``pyramid.request.Request.static_url`` when an external URL is being
+  generated.
+  See https://github.com/Pylons/pyramid/pull/1183
+
 Bug Fixes
 ---------
 
diff --git a/pyramid/config/views.py b/pyramid/config/views.py
index a3f885504..72dc3f414 100644
--- a/pyramid/config/views.py
+++ b/pyramid/config/views.py
@@ -44,6 +44,11 @@ from pyramid.compat import (
     is_nonstr_iter
     )
 
+from pyramid.encode import (
+    quote_plus,
+    urlencode,
+)
+
 from pyramid.exceptions import (
     ConfigurationError,
     PredicateMismatch,
@@ -65,6 +70,8 @@ from pyramid.security import NO_PERMISSION_REQUIRED
 from pyramid.static import static_view
 from pyramid.threadlocal import get_current_registry
 
+from pyramid.url import parse_url_overrides
+
 from pyramid.view import (
     render_view_to_response,
     AppendSlashNotFoundViewFactory,
@@ -1895,14 +1902,15 @@ class StaticURLInfo(object):
                     kw['subpath'] = subpath
                     return request.route_url(route_name, **kw)
                 else:
+                    app_url, scheme, host, port, qs, anchor = \
+                        parse_url_overrides(kw)
                     parsed = url_parse(url)
                     if not parsed.scheme:
-                        # parsed.scheme is readonly, so we have to parse again
-                        # to change the scheme, sigh.
-                        url = urlparse.urlunparse(url_parse(
-                            url, scheme=request.environ['wsgi.url_scheme']))
+                        url = urlparse.urlunparse(parsed._replace(
+                            scheme=request.environ['wsgi.url_scheme']))
                     subpath = url_quote(subpath)
-                    return urljoin(url, subpath)
+                    result = urljoin(url, subpath)
+                    return result + qs + anchor
 
         raise ValueError('No static URL definition matching %s' % path)
 
diff --git a/pyramid/encode.py b/pyramid/encode.py
index 9e190bc21..0be0107b3 100644
--- a/pyramid/encode.py
+++ b/pyramid/encode.py
@@ -3,11 +3,16 @@ from pyramid.compat import (
     binary_type,
     is_nonstr_iter,
     url_quote as _url_quote,
-    url_quote_plus as quote_plus, # bw compat api (dnr)
+    url_quote_plus as _quote_plus,
     )
 
-def url_quote(s, safe=''): # bw compat api
-    return _url_quote(s, safe=safe)
+def url_quote(val, safe=''): # bw compat api
+    cls = val.__class__
+    if cls is text_type:
+        val = val.encode('utf-8')
+    elif cls is not binary_type:
+        val = str(val).encode('utf-8')
+    return _url_quote(val, safe=safe)
 
 def urlencode(query, doseq=True):
     """
@@ -47,28 +52,28 @@ def urlencode(query, doseq=True):
     prefix = ''
 
     for (k, v) in query:
-        k = _enc(k)
+        k = quote_plus(k)
 
         if is_nonstr_iter(v):
             for x in v:
-                x = _enc(x)
+                x = quote_plus(x)
                 result += '%s%s=%s' % (prefix, k, x)
                 prefix = '&'
         elif v is None:
             result += '%s%s=' % (prefix, k)
         else:
-            v = _enc(v)
+            v = quote_plus(v)
             result += '%s%s=%s' % (prefix, k, v)
 
         prefix = '&'
 
     return result
 
-def _enc(val):
+# bw compat api (dnr)
+def quote_plus(val, safe=''):
     cls = val.__class__
     if cls is text_type:
         val = val.encode('utf-8')
     elif cls is not binary_type:
         val = str(val).encode('utf-8')
-    return quote_plus(val)
-
+    return _quote_plus(val, safe=safe)
diff --git a/pyramid/tests/test_config/test_views.py b/pyramid/tests/test_config/test_views.py
index 051961d25..57bb5e9d0 100644
--- a/pyramid/tests/test_config/test_views.py
+++ b/pyramid/tests/test_config/test_views.py
@@ -3820,6 +3820,27 @@ class TestStaticURLInfo(unittest.TestCase):
         result = inst.generate('package:path/abc def', request, a=1)
         self.assertEqual(result, 'http://example.com/abc%20def')
 
+    def test_generate_url_with_custom_query(self):
+        inst = self._makeOne()
+        registrations = [('http://example.com/', 'package:path/', None)]
+        inst._get_registrations = lambda *x: registrations
+        request = self._makeRequest()
+        result = inst.generate('package:path/abc def', request, a=1,
+                               _query='(openlayers)')
+        self.assertEqual(result,
+                         'http://example.com/abc%20def?(openlayers)')
+
+    def test_generate_url_with_custom_anchor(self):
+        inst = self._makeOne()
+        registrations = [('http://example.com/', 'package:path/', None)]
+        inst._get_registrations = lambda *x: registrations
+        request = self._makeRequest()
+        uc = text_(b'La Pe\xc3\xb1a', 'utf-8')
+        result = inst.generate('package:path/abc def', request, a=1,
+                               _anchor=uc)
+        self.assertEqual(result,
+                         'http://example.com/abc%20def#La%20Pe%C3%B1a')
+
     def test_add_already_exists(self):
         inst = self._makeOne()
         config = self._makeConfig(
diff --git a/pyramid/tests/test_encode.py b/pyramid/tests/test_encode.py
index 908249877..8fb766d88 100644
--- a/pyramid/tests/test_encode.py
+++ b/pyramid/tests/test_encode.py
@@ -72,3 +72,8 @@ class URLQuoteTests(unittest.TestCase):
         la = b'La/Pe\xc3\xb1a'
         result = self._callFUT(la, '/')
         self.assertEqual(result, 'La/Pe%C3%B1a')
+
+    def test_it_with_nonstr_nonbinary(self):
+        la = None
+        result = self._callFUT(la, '/')
+        self.assertEqual(result, 'None')
diff --git a/pyramid/tests/test_url.py b/pyramid/tests/test_url.py
index f6117777f..22ccd1d0e 100644
--- a/pyramid/tests/test_url.py
+++ b/pyramid/tests/test_url.py
@@ -93,6 +93,14 @@ class TestURLMethodsMixin(unittest.TestCase):
         result = request.resource_url(context, 'a b c')
         self.assertEqual(result, 'http://example.com:5432/context/a%20b%20c')
 
+    def test_resource_url_with_query_str(self):
+        request = self._makeOne()
+        self._registerResourceURL(request.registry)
+        context = DummyContext()
+        result = request.resource_url(context, 'a', query='(openlayers)')
+        self.assertEqual(result,
+            'http://example.com:5432/context/a?(openlayers)')
+
     def test_resource_url_with_query_dict(self):
         request = self._makeOne()
         self._registerResourceURL(request.registry)
@@ -149,23 +157,18 @@ class TestURLMethodsMixin(unittest.TestCase):
         request = self._makeOne()
         self._registerResourceURL(request.registry)
         context = DummyContext()
-        uc = text_(b'La Pe\xc3\xb1a', 'utf-8') 
+        uc = text_(b'La Pe\xc3\xb1a', 'utf-8')
         result = request.resource_url(context, anchor=uc)
-        self.assertEqual(
-            result,
-            native_(
-                text_(b'http://example.com:5432/context/#La Pe\xc3\xb1a',
-                      'utf-8'),
-                'utf-8')
-            )
+        self.assertEqual(result,
+                         'http://example.com:5432/context/#La%20Pe%C3%B1a')
 
-    def test_resource_url_anchor_is_not_urlencoded(self):
+    def test_resource_url_anchor_is_urlencoded_safe(self):
         request = self._makeOne()
         self._registerResourceURL(request.registry)
         context = DummyContext()
-        result = request.resource_url(context, anchor=' /#')
+        result = request.resource_url(context, anchor=' /#?&+')
         self.assertEqual(result,
-                         'http://example.com:5432/context/# /#')
+                         'http://example.com:5432/context/#%20/%23?&+')
 
     def test_resource_url_no_IResourceURL_registered(self):
         # falls back to ResourceURL
@@ -448,14 +451,8 @@ class TestURLMethodsMixin(unittest.TestCase):
         request.registry.registerUtility(mapper, IRoutesMapper)
         result = request.route_url('flub', _anchor=b"La Pe\xc3\xb1a")
 
-        self.assertEqual(
-            result,
-            native_(
-                text_(
-                    b'http://example.com:5432/1/2/3#La Pe\xc3\xb1a',
-                    'utf-8'),
-                'utf-8')
-            )
+        self.assertEqual(result,
+                         'http://example.com:5432/1/2/3#La%20Pe%C3%B1a')
 
     def test_route_url_with_anchor_unicode(self):
         from pyramid.interfaces import IRoutesMapper
@@ -465,14 +462,8 @@ class TestURLMethodsMixin(unittest.TestCase):
         anchor = text_(b'La Pe\xc3\xb1a', 'utf-8')
         result = request.route_url('flub', _anchor=anchor)
 
-        self.assertEqual(
-            result,
-            native_(
-                text_(
-                    b'http://example.com:5432/1/2/3#La Pe\xc3\xb1a',
-                    'utf-8'),
-                'utf-8')
-            )
+        self.assertEqual(result,
+                         'http://example.com:5432/1/2/3#La%20Pe%C3%B1a')
 
     def test_route_url_with_query(self):
         from pyramid.interfaces import IRoutesMapper
@@ -483,6 +474,15 @@ class TestURLMethodsMixin(unittest.TestCase):
         self.assertEqual(result,
                          'http://example.com:5432/1/2/3?q=1')
 
+    def test_route_url_with_query_str(self):
+        from pyramid.interfaces import IRoutesMapper
+        request = self._makeOne()
+        mapper = DummyRoutesMapper(route=DummyRoute('/1/2/3'))
+        request.registry.registerUtility(mapper, IRoutesMapper)
+        result = request.route_url('flub', _query='(openlayers)')
+        self.assertEqual(result,
+                         'http://example.com:5432/1/2/3?(openlayers)')
+
     def test_route_url_with_empty_query(self):
         from pyramid.interfaces import IRoutesMapper
         request = self._makeOne()
diff --git a/pyramid/url.py b/pyramid/url.py
index fda2c72c7..14f4add35 100644
--- a/pyramid/url.py
+++ b/pyramid/url.py
@@ -12,12 +12,13 @@ from pyramid.interfaces import (
     )
 
 from pyramid.compat import (
-    native_,
     bytes_,
-    text_type,
-    url_quote,
+    string_types,
     )
-from pyramid.encode import urlencode
+from pyramid.encode import (
+    url_quote,
+    urlencode,
+)
 from pyramid.path import caller_package
 from pyramid.threadlocal import get_current_registry
 
@@ -27,6 +28,48 @@ from pyramid.traversal import (
     )
 
 PATH_SAFE = '/:@&+$,' # from webob
+QUERY_SAFE = '/?:@!$&\'()*+,;=' # RFC 3986
+ANCHOR_SAFE = QUERY_SAFE
+
+def parse_url_overrides(kw):
+    """Parse special arguments passed when generating urls.
+
+    The supplied dictionary is mutated, popping arguments as necessary.
+    Returns a 6-tuple of the format ``(app_url, scheme, host, port,
+    qs, anchor)``.
+    """
+    anchor = ''
+    qs = ''
+    app_url = None
+    host = None
+    scheme = None
+    port = None
+
+    if '_query' in kw:
+        query = kw.pop('_query')
+        if isinstance(query, string_types):
+            qs = '?' + url_quote(query, QUERY_SAFE)
+        elif query:
+            qs = '?' + urlencode(query, doseq=True)
+
+    if '_anchor' in kw:
+        anchor = kw.pop('_anchor')
+        anchor = url_quote(anchor, ANCHOR_SAFE)
+        anchor = '#' + anchor
+
+    if '_app_url' in kw:
+        app_url = kw.pop('_app_url')
+
+    if '_host' in kw:
+        host = kw.pop('_host')
+
+    if '_scheme' in kw:
+        scheme = kw.pop('_scheme')
+
+    if '_port' in kw:
+        port = kw.pop('_port')
+
+    return app_url, scheme, host, port, qs, anchor
 
 class URLMethodsMixin(object):
     """ Request methods mixin for BaseRequest having to do with URL
@@ -128,11 +171,15 @@ class URLMethodsMixin(object):
         query string will be returned in the URL. If it is present, it
         will be used to compose a query string that will be tacked on
         to the end of the URL, replacing any request query string.
-        The value of ``_query`` must be a sequence of two-tuples *or*
+        The value of ``_query`` may be a sequence of two-tuples *or*
         a data structure with an ``.items()`` method that returns a
         sequence of two-tuples (presumably a dictionary).  This data
         structure will be turned into a query string per the
-        documentation of :func:`pyramid.encode.urlencode` function.
+        documentation of :func:`pyramid.url.urlencode` function.
+        Alternative encodings may be used by passing a string for ``_query``
+        in which case it will be quoted as per :rfc:`3986#section-3.4` but
+        no other assumptions will be made about the data format. For example,
+        spaces will be escaped as ``%20`` instead of ``+``.
         After the query data is turned into a query string, a leading
         ``?`` is prepended, and the resulting string is appended to
         the generated URL.
@@ -146,8 +193,13 @@ class URLMethodsMixin(object):
            as values, and a k=v pair will be placed into the query string for
            each value.
 
+        .. versionchanged:: 1.5
+           Allow the ``_query`` option to be a string to enable alternative
+           encodings.
+
         If a keyword argument ``_anchor`` is present, its string
-        representation will be used as a named anchor in the generated URL
+        representation will be quoted per :rfc:`3986#section-3.5` and used as
+        a named anchor in the generated URL
         (e.g. if ``_anchor`` is passed as ``foo`` and the route URL is
         ``http://example.com/route/url``, the resulting generated URL will
         be ``http://example.com/route/url#foo``).
@@ -156,8 +208,11 @@ class URLMethodsMixin(object):
 
            If ``_anchor`` is passed as a string, it should be UTF-8 encoded. If
            ``_anchor`` is passed as a Unicode object, it will be converted to
-           UTF-8 before being appended to the URL.  The anchor value is not
-           quoted in any way before being appended to the generated URL.
+           UTF-8 before being appended to the URL.
+
+        .. versionchanged:: 1.5
+           The ``_anchor`` option will be escaped instead of using
+           its raw string representation.
 
         If both ``_anchor`` and ``_query`` are specified, the anchor
         element will always follow the query element,
@@ -213,34 +268,7 @@ class URLMethodsMixin(object):
         if route.pregenerator is not None:
             elements, kw = route.pregenerator(self, elements, kw)
 
-        anchor = ''
-        qs = ''
-        app_url = None
-        host = None
-        scheme = None
-        port = None
-
-        if '_query' in kw:
-            query = kw.pop('_query')
-            if query:
-                qs = '?' + urlencode(query, doseq=True)
-
-        if '_anchor' in kw:
-            anchor = kw.pop('_anchor')
-            anchor = native_(anchor, 'utf-8')
-            anchor = '#' + anchor
-
-        if '_app_url' in kw:
-            app_url = kw.pop('_app_url')
-
-        if '_host' in kw:
-            host = kw.pop('_host')
-
-        if '_scheme' in kw:
-            scheme = kw.pop('_scheme')
-
-        if '_port' in kw:
-            port = kw.pop('_port')
+        app_url, scheme, host, port, qs, anchor = parse_url_overrides(kw)
 
         if app_url is None:
             if (scheme is not None or host is not None or port is not None):
@@ -335,13 +363,17 @@ class URLMethodsMixin(object):
 
         If a keyword argument ``query`` is present, it will be used to
         compose a query string that will be tacked on to the end of the URL.
-        The value of ``query`` must be a sequence of two-tuples *or* a data
+        The value of ``query`` may be a sequence of two-tuples *or* a data
         structure with an ``.items()`` method that returns a sequence of
         two-tuples (presumably a dictionary).  This data structure will be
         turned into a query string per the documentation of
-        ``pyramid.url.urlencode`` function.  After the query data is turned
-        into a query string, a leading ``?`` is prepended, and the resulting
-        string is appended to the generated URL.
+        :func:``pyramid.url.urlencode`` function.
+        Alternative encodings may be used by passing a string for ``query``
+        in which case it will be quoted as per :rfc:`3986#section-3.4` but
+        no other assumptions will be made about the data format. For example,
+        spaces will be escaped as ``%20`` instead of ``+``.
+        After the query data is turned into a query string, a leading ``?`` is
+        prepended, and the resulting string is appended to the generated URL.
 
         .. note::
 
@@ -352,6 +384,10 @@ class URLMethodsMixin(object):
            as values, and a k=v pair will be placed into the query string for
            each value.
 
+        .. versionchanged:: 1.5
+           Allow the ``query`` option to be a string to enable alternative
+           encodings.
+
         If a keyword argument ``anchor`` is present, its string
         representation will be used as a named anchor in the generated URL
         (e.g. if ``anchor`` is passed as ``foo`` and the resource URL is
@@ -362,8 +398,11 @@ class URLMethodsMixin(object):
 
            If ``anchor`` is passed as a string, it should be UTF-8 encoded. If
            ``anchor`` is passed as a Unicode object, it will be converted to
-           UTF-8 before being appended to the URL.  The anchor value is not
-           quoted in any way before being appended to the generated URL.
+           UTF-8 before being appended to the URL.
+
+        .. versionchanged:: 1.5
+           The ``anchor`` option will be escaped instead of using
+           its raw string representation.
 
         If both ``anchor`` and ``query`` are specified, the anchor element
         will always follow the query element,
@@ -580,13 +619,14 @@ class URLMethodsMixin(object):
 
         if 'query' in kw:
             query = kw['query']
-            if query:
+            if isinstance(query, string_types):
+                qs = '?' + url_quote(query, QUERY_SAFE)
+            elif query:
                 qs = '?' + urlencode(query, doseq=True)
 
         if 'anchor' in kw:
             anchor = kw['anchor']
-            if isinstance(anchor, text_type):
-                anchor = native_(anchor, 'utf-8')
+            anchor = url_quote(anchor, ANCHOR_SAFE)
             anchor = '#' + anchor
 
         if elements: