Killing fetchers.HTTPResponse

- fetchers.fetch now returns unwrapped response from urlopen
- HTTPResponse still lives in test.support as a mock object
- fixed a lot of code and tests to expect bytes from
  response.read() instead of strings in response.body
- temporarily dropped support for limiting reading from
  a response by size

openid/consumer/consumer.py

@@ -225,7 +225,7 @@ def makeKVPost(request_message, server_url):
     """
     try:
         response = fetchers.fetch(server_url, body=request_message.toURLEncoded())
-        return Message.fromKVForm(response.body)
+        return Message.fromKVForm(response.read())
     except urllib.error.HTTPError as e:
         if e.code == 400:
             raise ServerError.fromMessage(Message.fromKVForm(e.read()))

openid/consumer/discover.py

@@ -435,9 +435,9 @@ def discoverXRI(iname):
 
 def discoverNoYadis(uri):
     http_resp = fetchers.fetch(uri)
-    claimed_id = http_resp.final_url
+    claimed_id = http_resp.url
     openid_services = OpenIDServiceEndpoint.fromHTML(
-        claimed_id, http_resp.body)
+        claimed_id, http_resp.read()) # MAX_RESPONSE
     return claimed_id, openid_services
 
 def discoverURI(uri):

openid/fetchers.py

@@ -3,14 +3,13 @@
 This module contains the HTTP fetcher interface and several implementations.
 """
 
-__all__ = ['fetch', 'HTTPResponse']
+__all__ = ['fetch']
 
 import urllib.request
 import urllib.error
 import urllib.parse
 
 import sys
-import contextlib
 
 import openid
 
@@ -20,20 +19,6 @@
     sys.platform,
     urllib.request.__version__,
 )
-MAX_RESPONSE_KB = 1024
-
-
-class HTTPResponse(object):
-    def __init__(self, final_url, status, headers=None, body=None):
-        self.final_url = final_url
-        self.status = status
-        self.headers = headers
-        self.body = body
-
-    def __repr__(self):
-        return "<%s status %s for %s>" % (self.__class__.__name__,
-                                          self.status,
-                                          self.final_url)
 
 
 def fetch(url, body=None, headers=None):
@@ -45,12 +30,4 @@ def fetch(url, body=None, headers=None):
     headers.setdefault('User-Agent', USER_AGENT)
 
     request = urllib.request.Request(url, data=body, headers=headers)
-    f = urllib.request.urlopen(request)
-    with contextlib.closing(f):
-        return HTTPResponse(
-            final_url=f.geturl(),
-            status=f.status,
-            headers={k.lower(): v for k, v in f.info().items()},
-            body=f.read(MAX_RESPONSE_KB * 1024),
-        )
-
+    return urllib.request.urlopen(request)

openid/test/discoverdata.py

@@ -114,6 +114,7 @@ def generateResult(base_url, input_name, id_name, result_name, success):
 
     result = generateSample(result_name, base_url)
     headers, content = result.split('\n\n', 1)
+    content = content.encode('utf-8')
     header_lines = headers.split('\n')
     for header_line in header_lines:
         if header_line.startswith('Content-Type:'):

openid/test/support.py

@@ -1,4 +1,5 @@
 from openid import message
+import io
 from logging.handlers import BufferingHandler
 import logging
 
@@ -67,3 +68,19 @@ def failUnlessLogMatches(self, *prefixes):
 
     def failUnlessLogEmpty(self):
         self.failUnlessLogMatches()
+
+class HTTPResponse:
+    def __init__(self, url, status, headers=None, body=None):
+        self.url = url
+        self.status = status
+        self.headers = headers or {}
+        self._body = io.BytesIO(body)
+
+    def info():
+        return self._headers
+
+    def read(self, *args):
+        return self._body.read(*args)
+
+    def getheader(self, name):
+        return {k.lower(): v for k, v in self.headers.items()}.get(name.lower())

openid/test/test_consumer.py

@@ -25,11 +25,10 @@
 from openid.yadis.discover import DiscoveryFailure
 from openid.dh import DiffieHellman
 
-from openid.fetchers import HTTPResponse
 from openid import fetchers
 from openid.store import memstore
 
-from .support import CatchLogs
+from .support import CatchLogs, HTTPResponse
 
 
 assocs = [
@@ -98,7 +97,7 @@ class TestFetcher(object):
     def __init__(self, user_url, user_page, xxx_todo_changeme):
         (assoc_secret, assoc_handle) = xxx_todo_changeme
         self.get_responses = {
-            user_url: HTTPResponse(user_url, 200, body=user_page)
+            user_url: user_page
         }
         self.assoc_secret = assoc_secret
         self.assoc_handle = assoc_handle
@@ -107,7 +106,7 @@ def __init__(self, user_url, user_page, xxx_todo_changeme):
     def fetch(self, url, body=None, headers=None):
         if body is None:
             if url in self.get_responses:
-                return self.get_responses[url]
+                return HTTPResponse(url, 200, body=self.get_responses[url])
         else:
             try:
                 body.index('openid.mode=associate')
@@ -1254,7 +1253,7 @@ def tearDown(self):
 
     def test_error(self):
         self.fetcher.response = HTTPResponse(
-            "http://some_url", 404, {'Hea': 'der'}, 'blah:blah\n')
+            'http://some_url', 404, {'Hea': 'der'}, b'blah:blah\n')
         query = {'openid.signed': 'stuff',
                  'openid.stuff': 'a value'}
         r = self.consumer._checkAuth(Message.fromPostArgs(query),
@@ -2088,7 +2087,7 @@ def test_SillyExtension(self):
 def kvpost_fetch(url, body=None, headers=None):
     status = int(url.rsplit('/', 1)[1])
     if 200 <= status < 300:
-        return HTTPResponse(final_url=url, status=status, body='foo:bar\nbaz:quux\n')
+        return HTTPResponse(url, status, body=b'foo:bar\nbaz:quux\n')
     if 400 <= status < 500:
         raise urllib.error.HTTPError(url, status, 'Test request failed', {}, io.BytesIO(b'error:bonk\nerror_code:7\n'))
     if 500 <= status:

openid/test/test_discover.py

@@ -5,9 +5,9 @@
 from urllib.parse import urlsplit
 
 from . import datadriven
+from .support import HTTPResponse
 
 from openid import fetchers
-from openid.fetchers import HTTPResponse
 from openid.yadis.discover import DiscoveryFailure
 from openid.consumer import discover, consumer as openid_consumer
 from openid.yadis import xrires
@@ -24,7 +24,7 @@ def __init__(self, responses):
     def fetch(self, url, body=None, headers=None):
         response = self.responses.pop(0)
         assert body is None
-        assert response.final_url == url
+        assert response.url == url
         return response
 
 
@@ -40,7 +40,7 @@ class TestDiscoveryFailure(datadriven.DataDrivenTestCase):
         ]
 
     def __init__(self, responses):
-        self.url = responses[0].final_url
+        self.url = responses[0].url
         datadriven.DataDrivenTestCase.__init__(self, self.url)
         self.responses = responses
 
@@ -167,7 +167,7 @@ def fetch(self, url, body=None, headers=None):
         except KeyError:
             status = 404
             ctype = 'text/plain'
-            body = ''
+            body = b''
         else:
             status = 200
 
@@ -291,7 +291,7 @@ def test_unicode_undecodable_html2(self):
 
     def test_noOpenID(self):
         services = self._discover(content_type='text/plain',
-                                  data="junk",
+                                  data=b'junk',
                                   expected_service_count=0)
 
         services = self._discover(

openid/test/test_fetchers.py

@@ -7,6 +7,8 @@
 
 from openid import fetchers
 
+from .support import HTTPResponse
+
 # XXX: make these separate test cases
 
 
@@ -18,16 +20,14 @@ def _assertEqual(v1, v2, extra):
 
 
 def failUnlessResponseExpected(expected, actual, extra):
-    _assertEqual(expected.final_url, actual.final_url, extra)
+    _assertEqual(expected.url, actual.url, extra)
     _assertEqual(expected.status, actual.status, extra)
-    _assertEqual(expected.body, actual.body, extra)
-    got_headers = dict(actual.headers)
-
-    del got_headers['date']
-    del got_headers['server']
-
-    for k, v in expected.headers.items():
-        assert got_headers[k] == v, (k, v, got_headers[k], extra)
+    _assertEqual(expected.read(), actual.read(), extra)
+    actual_headers = {k.lower(): v for k, v in actual.headers.items()}
+    expected_headers = {k.lower(): v for k, v in expected.headers.items()}
+    del actual_headers['date']
+    del actual_headers['server']
+    assert actual_headers == expected_headers
 
 
 def test_fetcher(server):
@@ -36,19 +36,9 @@ def geturl(path):
         host, port = server.server_address
         return 'http://%s:%s%s' % (host, port, path)
 
-    expected_headers = {'content-type': 'text/plain'}
-
-    expect_success = fetchers.HTTPResponse(
-        geturl('/success'), 200, expected_headers, b'/success')
-    cases = [
-        ('/success', expect_success),
-        ('/301redirect', expect_success),
-        ('/302redirect', expect_success),
-        ('/303redirect', expect_success),
-        ('/307redirect', expect_success),
-        ]
-
-    for path, expected in cases:
+    paths = ['/success', '/301redirect', '/302redirect', '/303redirect', '/307redirect']
+    for path in paths:
+        expected = HTTPResponse(geturl('/success'), 200, {'content-type': 'text/plain'}, b'/success')
         fetch_url = geturl(path)
         try:
             actual = fetchers.fetch(fetch_url)

openid/test/test_yadis_discover.py

@@ -18,6 +18,7 @@
 from openid import fetchers
 
 from . import discoverdata
+from .support import HTTPResponse
 
 status_header_re = re.compile(r'Status: (\d+) .*?$', re.MULTILINE)
 
@@ -38,11 +39,11 @@ def mkResponse(data):
     headers = {}
     for line in headers_str.split('\n'):
         k, v = line.split(':', 1)
-        k = k.strip().lower()
+        k = k.strip()
         v = v.strip()
         headers[k] = v
     status = int(status_mo.group(1))
-    return fetchers.HTTPResponse('<test>', status, headers=headers, body=body)
+    return HTTPResponse('<test>', status, headers=headers, body=body.encode('utf-8'))
 
 
 class TestFetcher(object):
@@ -63,9 +64,9 @@ def fetch(self, url, body=None, headers=None):
             if response.status >= 400:
                 raise urllib.error.HTTPError(current_url, response.status, 'Test request failed', {}, io.BytesIO())
             if response.status in [301, 302, 303, 307]:
-                current_url = response.headers['location']
+                current_url = response.getheader('location')
             else:
-                response.final_url = current_url
+                response.url = current_url
                 return response
 
 
@@ -80,7 +81,7 @@ def fetch(self, uri, headers=None, body=None):
                 headers = {
                     'X-XRDS-Location'.lower(): 'http://unittest/404',
                     }
-                return fetchers.HTTPResponse(uri, 200, headers, '')
+                return HTTPResponse(uri, 200, headers, b'')
             else:
                 raise urllib.error.HTTPError(uri, 404, 'Test request failed', {}, io.BytesIO(b''))
 
@@ -93,7 +94,7 @@ def tearDown(self):
 
     def test_404(self):
         uri = "http://something.unittest/"
-        self.assertRaises(DiscoveryFailure, discover, uri)
+        self.assertRaises(urllib.error.HTTPError, discover, uri)
 
 
 class _TestCase(unittest.TestCase):