raise ValueError instead of generating just path when _app_url is provided to request.route_url and the route has an external pattern

4 files changed, 29 insertions, 21 deletions

diff --git a/CHANGES.txt b/CHANGES.txt
index b4fe60085..1eeb0ce7b 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -142,9 +142,9 @@ Features
 - The ``alchemy`` scaffold tests now provide better coverage.  See
   https://github.com/Pylons/pyramid/pull/1029
 
-- The ``pyramid.config.Configurator.add_route`` method now supports being called
-  with an external URL as pattern. See https://github.com/Pylons/pyramid/issues/611
-  for more information.
+- The ``pyramid.config.Configurator.add_route`` method now supports being
+  called with an external URL as pattern. See
+  https://github.com/Pylons/pyramid/issues/611 for more information.
 
 Bug Fixes
 ---------
diff --git a/docs/narr/urldispatch.rst b/docs/narr/urldispatch.rst
index f3513624e..62eb89348 100644
--- a/docs/narr/urldispatch.rst
+++ b/docs/narr/urldispatch.rst
@@ -785,11 +785,12 @@ purposes only and are never considered for matching at request time.
    >>> request.route_url('youtube', video_id='oHg5SJYRHA0')
    >>> "https://youtube.com/watch/oHg5SJYRHA0"
 
-All pattern replacements and calls to
-:meth:`pyramid.request.Request.route_url` will work as expected. Note that
-:meth:`pyramid.request.Request.route_path` will also just return the external
-URLs path part.
-
+Most pattern replacements and calls to
+:meth:`pyramid.request.Request.route_url` will work as expected. However, calls
+to :meth:`pyramid.request.Request.route_path` against external patterns will
+raise an exception, and passing ``_app_url`` to
+:meth:`~pyramid.request.Request.route_url` to generate a URL against a route
+that has an external pattern will also raise an exception.
 
 .. index::
    single: redirecting to slash-appended routes
diff --git a/pyramid/config/routes.py b/pyramid/config/routes.py
index af7ddd6f3..0ed370c94 100644
--- a/pyramid/config/routes.py
+++ b/pyramid/config/routes.py
@@ -385,21 +385,29 @@ class RoutesConfiguratorMixin(object):
         if pattern is None:
             raise ConfigurationError('"pattern" argument may not be None')
 
-        # check for an external route
+        # check for an external route; an external route is one which is
+        # is a full url (e.g. 'http://example.com/{id}')
         parsed = urlparse.urlparse(pattern)
         if parsed.hostname:
             pattern = parsed.path
 
             original_pregenerator = pregenerator
             def external_url_pregenerator(request, elements, kw):
-                if '_app_url' not in kw:
-                    if '_scheme' in kw:
-                        scheme = kw['_scheme']
-                    elif parsed.scheme:
-                        scheme = parsed.scheme
-                    else:
-                        scheme = request.scheme
-                    kw['_app_url'] = '{0}://{1}'.format(scheme, parsed.netloc)
+                if '_app_url' in kw:
+                    raise ValueError(
+                        'You cannot generate a path to an external route '
+                        'pattern via request.route_path nor pass an _app_url '
+                        'to request.route_url when generating a URL for an '
+                        'external route pattern (pattern was "%s") ' %
+                        (pattern,)
+                        )
+                if '_scheme' in kw:
+                    scheme = kw['_scheme']
+                elif parsed.scheme:
+                    scheme = parsed.scheme
+                else:
+                    scheme = request.scheme
+                kw['_app_url'] = '{0}://{1}'.format(scheme, parsed.netloc)
 
                 if original_pregenerator:
                     elements, kw = original_pregenerator(
diff --git a/pyramid/tests/test_url.py b/pyramid/tests/test_url.py
index 9e4f72c41..6f1ee3bf0 100644
--- a/pyramid/tests/test_url.py
+++ b/pyramid/tests/test_url.py
@@ -1067,15 +1067,14 @@ class Test_external_static_url_integration(unittest.TestCase):
         self.config.add_route('acme', 'http://acme.org/path/{foo}')
         request = self._makeRequest()
         request.registry = self.config.registry
-        self.assertEqual(
-            request.route_url('acme', foo='bar', _app_url='http://fakeme.com'),
-            'http://fakeme.com/path/bar')
+        self.assertRaises(ValueError,
+            request.route_url, 'acme', foo='bar', _app_url='http://fakeme.com')
 
     def test_generate_external_url_route_path(self):
         self.config.add_route('acme', 'https://acme.org/path/{foo}')
         request = self._makeRequest()
         request.registry = self.config.registry
-        self.assertEqual(request.route_path('acme', foo='bar'), '/path/bar')
+        self.assertRaises(ValueError, request.route_path, 'acme', foo='bar')
 
     def test_generate_external_url_with_pregenerator(self):
         def pregenerator(request, elements, kw):