Invalid url exception (#2258)

* Fix #2241: Raise  instead of  on fetches with invalid URL

* Add missing changes

* Fix test

* Fix spelling

* Improve test coverage

Author: asvetlov

aiohttp/client.py

@@ -15,8 +15,8 @@
 from . import connector as connector_mod
 from . import client_exceptions, client_reqrep, hdrs, http, payload
 from .client_exceptions import *  # noqa
-from .client_exceptions import (ClientError, ClientOSError, ServerTimeoutError,
-                                WSServerHandshakeError)
+from .client_exceptions import (ClientError, ClientOSError, InvalidURL,
+                                ServerTimeoutError, WSServerHandshakeError)
 from .client_reqrep import *  # noqa
 from .client_reqrep import ClientRequest, ClientResponse
 from .client_ws import ClientWebSocketResponse
@@ -204,7 +204,15 @@ def _request(self, method, url, *,
                 skip_headers.add(istr(i))
 
         if proxy is not None:
-            proxy = URL(proxy)
+            try:
+                proxy = URL(proxy)
+            except ValueError:
+                raise InvalidURL(url)
+
+        try:
+            url = URL(url)
+        except ValueError:
+            raise InvalidURL(url)
 
         # timeout is cumulative for all request operations
         # (request, redirects, responses, data consuming)
@@ -217,7 +225,7 @@ def _request(self, method, url, *,
         try:
             with timer:
                 while True:
-                    url = URL(url).with_fragment(None)
+                    url = url.with_fragment(None)
                     cookies = self._cookie_jar.filter_cookies(url)
 
                     req = self._request_class(
@@ -285,8 +293,12 @@ def _request(self, method, url, *,
                             # see github.com/aio-libs/aiohttp/issues/2022
                             break
 
-                        r_url = URL(
-                            r_url, encoded=not self.requote_redirect_url)
+                        try:
+                            r_url = URL(
+                                r_url, encoded=not self.requote_redirect_url)
+
+                        except ValueError:
+                            raise InvalidURL(r_url)
 
                         scheme = r_url.scheme
                         if scheme not in ('http', 'https', ''):

aiohttp/client_exceptions.py

@@ -15,7 +15,7 @@
     'ClientResponseError', 'ClientHttpProxyError',
     'WSServerHandshakeError', 'ContentTypeError',
 
-    'ClientPayloadError')
+    'ClientPayloadError', 'InvalidURL')
 
 
 class ClientError(Exception):
@@ -131,3 +131,22 @@ def __repr__(self):
 
 class ClientPayloadError(ClientError):
     """Response payload error."""
+
+
+class InvalidURL(ClientError, ValueError):
+    """Invalid URL.
+
+    URL used for fetching is malformed, e.g. it doesn't contains host
+    part."""
+
+    # Derive from ValueError for backward compatibility
+
+    def __init__(self, url):
+        super().__init__(url)
+
+    @property
+    def url(self):
+        return self.args[0]
+
+    def __repr__(self):
+        return '<{} {}>'.format(self.__class__.__name__, self.url)

aiohttp/client_reqrep.py

@@ -15,7 +15,8 @@
 
 from . import hdrs, helpers, http, payload
 from .client_exceptions import (ClientConnectionError, ClientOSError,
-                                ClientResponseError, ContentTypeError)
+                                ClientResponseError, ContentTypeError,
+                                InvalidURL)
 from .formdata import FormData
 from .helpers import PY_35, HeadersMixin, SimpleCookie, TimerNoop, noop
 from .http import SERVER_SOFTWARE, HttpVersion10, HttpVersion11, PayloadWriter
@@ -143,8 +144,7 @@ def update_host(self, url):
         """Update destination host, port and connection type (ssl)."""
         # get host/port
         if not url.host:
-            raise ValueError(
-                "Could not parse hostname from URL '{}'".format(url))
+            raise InvalidURL(url)
 
         # basic auth info
         username, password = url.user, url.password

changes/2241.feature

@@ -0,0 +1 @@
+Raise `InvalidURL` instead of `ValueError` on fetches with invalid URL.

docs/client_reference.rst

@@ -1476,6 +1476,29 @@ All exceptions are available as members of *aiohttp* module.
    Derived from :exc:`Exception`
 
 
+.. class:: ClientPayloadError
+
+   This exception can only be raised while reading the response
+   payload if one of these errors occurs:
+
+   1. invalid compression
+   2. malformed chunked encoding
+   3. not enough data that satisfy ``Content-Length`` HTTP header.
+
+   Derived from :exc:`ClientError`
+
+.. exception:: InvalidURL
+
+   URL used for fetching is malformed, e.g. it does not contain host
+   part.
+
+   Derived from :exc:`ClientError` and :exc:`ValueError`
+
+   .. attribute:: url
+
+      Invalid URL, :class:`yarl.URL` instance.
+
+
 Response errors
 ^^^^^^^^^^^^^^^
 
@@ -1576,17 +1599,6 @@ Connection errors
    Derived from :exc:`ServerConnectonError`
 
 
-.. class:: ClientPayloadError
-
-   This exception can only be raised while reading the response
-   payload if one of these errors occurs:
-
-   1. invalid compression
-   2. malformed chunked encoding
-   3. not enough data that satisfy ``Content-Length`` HTTP header.
-
-   Derived from :exc:`ClientError`
-
 Hierarchy of exceptions
 ^^^^^^^^^^^^^^^^^^^^^^^
 
@@ -1614,3 +1626,5 @@ Hierarchy of exceptions
       * :exc:`ServerFingerprintMismatch`
 
   * :exc:`ClientPayloadError`
+
+  * :exc:`InvalidURL`

tests/test_client_exceptions.py

@@ -1,5 +1,7 @@
 """Tests for http_exceptions.py"""
 
+from yarl import URL
+
 from aiohttp import client
 
 
@@ -8,3 +10,11 @@ def test_fingerprint_mismatch():
     expected = ('<ServerFingerprintMismatch expected=exp'
                 ' got=got host=host port=8888>')
     assert expected == repr(err)
+
+
+def test_invalid_url():
+    url = URL('http://example.com')
+    err = client.InvalidURL(url)
+    assert err.args[0] is url
+    assert err.url is url
+    assert repr(err) == "<InvalidURL http://example.com>"

tests/test_client_functional.py

@@ -2216,3 +2216,13 @@ def handler_redirect(request):
 
     with pytest.raises(aiohttp.ClientResponseError):
         yield from client.get('/')
+
+
+@asyncio.coroutine
+def test_invalid_idna(loop):
+    session = aiohttp.ClientSession(loop=loop)
+    try:
+        with pytest.raises(aiohttp.InvalidURL):
+            yield from session.get('http://\u2061owhefopw.com')
+    finally:
+        yield from session.close()

tests/test_client_request.py

@@ -286,15 +286,10 @@ def test_headers_default(make_request):
 
 
 def test_invalid_url(make_request):
-    with pytest.raises(ValueError):
+    with pytest.raises(aiohttp.InvalidURL):
         make_request('get', 'hiwpefhipowhefopw')
 
 
-def test_invalid_idna(make_request):
-    with pytest.raises(ValueError):
-        make_request('get', 'http://\u2061owhefopw.com')
-
-
 def test_no_path(make_request):
     req = make_request('get', 'http://python.org')
     assert '/' == req.url.path