Merge lp:~alecu/ubuntu-sso-client/use-pycurl-1-2 into lp:ubuntu-sso-client/stable-1-2

Proposed by Alejandro J. Cura on 2012-06-15
Status: Merged
Approved by: dobey on 2012-06-22
Approved revision: 691
Merged at revision: 690
Proposed branch: lp:~alecu/ubuntu-sso-client/use-pycurl-1-2
Merge into: lp:ubuntu-sso-client/stable-1-2
Diff against target: 571 lines (+458/-12)
5 files modified
ubuntu_sso/account.py (+2/-3)
ubuntu_sso/credentials.py (+4/-4)
ubuntu_sso/tests/test_credentials.py (+5/-5)
ubuntu_sso/utils/curllib.py (+147/-0)
ubuntu_sso/utils/tests/test_curllib.py (+300/-0)
To merge this branch: bzr merge lp:~alecu/ubuntu-sso-client/use-pycurl-1-2
Reviewer Review Type Date Requested Status
dobey (community) Approve on 2012-06-22
Manuel de la Peña (community) 2012-06-15 Approve on 2012-06-18
Review via email: mp+110638@code.launchpad.net

Commit message

- Use pycurl instead of httplib (LP: #882055).

To post a comment you must log in.
Manuel de la Peña (mandel) wrote :

I do approve the code but please can you remove your name and email from the headers, AFAIK it was agreed in the mailing list to remove those.

review: Approve
Alejandro J. Cura (alecu) wrote :

Thanks for the review!

> I do approve the code but please can you remove your name and email from the
> headers, AFAIK it was agreed in the mailing list to remove those.

It was agreed, yes, but for new code.
These branches are for the stable-1-x series, so I try to follow the convention of that tree.

dobey (dobey) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'ubuntu_sso/account.py'
2--- ubuntu_sso/account.py 2011-11-23 19:49:20 +0000
3+++ ubuntu_sso/account.py 2012-06-15 22:15:26 +0000
4@@ -20,7 +20,6 @@
5
6 import os
7 import re
8-import urllib2
9
10 # Unable to import 'lazr.restfulclient.*'
11 # pylint: disable=F0401
12@@ -32,7 +31,7 @@
13 from oauth import oauth
14
15 from ubuntu_sso.logger import setup_logging
16-from ubuntu_sso.utils import timestamp_checker
17+from ubuntu_sso.utils import curllib, timestamp_checker
18
19
20 logger = setup_logging("ubuntu_sso.account")
21@@ -140,7 +139,7 @@
22 # download captcha and save to 'filename'
23 logger.debug('generate_captcha: server answered: %r', captcha)
24 try:
25- res = urllib2.urlopen(captcha['image_url'])
26+ res = curllib.urlopen(captcha['image_url'])
27 with open(filename, 'wb') as f:
28 f.write(res.read())
29 except:
30
31=== modified file 'ubuntu_sso/credentials.py'
32--- ubuntu_sso/credentials.py 2011-11-23 19:49:20 +0000
33+++ ubuntu_sso/credentials.py 2012-06-15 22:15:26 +0000
34@@ -39,7 +39,6 @@
35
36 import sys
37 import traceback
38-import urllib2
39
40 from functools import wraps
41
42@@ -49,6 +48,7 @@
43 from ubuntu_sso import NO_OP, utils
44 from ubuntu_sso.keyring import Keyring
45 from ubuntu_sso.logger import setup_logging
46+from ubuntu_sso.utils import curllib
47
48
49 logger = setup_logging('ubuntu_sso.credentials')
50@@ -245,13 +245,13 @@
51 parameters=parameters)
52 oauth_req.sign_request(oauth.OAuthSignatureMethod_HMAC_SHA1(),
53 consumer, token)
54- request = urllib2.Request(url, headers=oauth_req.to_header())
55- logger.debug('Opening the url "%s" with urllib2.urlopen.',
56+ request = curllib.Request(url, headers=oauth_req.to_header())
57+ logger.debug('Opening the url "%s" with curllib.urlopen.',
58 request.get_full_url())
59 # This code is blocking, we should change this.
60 # I've tried with deferToThread an twisted.web.client.getPage
61 # but the returned deferred will never be fired (nataliabidart).
62- response = urllib2.urlopen(request)
63+ response = curllib.urlopen(request)
64 logger.debug('Url opened. Response: %s.', response.code)
65 returnValue(response.code)
66
67
68=== modified file 'ubuntu_sso/tests/test_credentials.py'
69--- ubuntu_sso/tests/test_credentials.py 2011-11-23 19:49:20 +0000
70+++ ubuntu_sso/tests/test_credentials.py 2012-06-15 22:15:26 +0000
71@@ -280,7 +280,7 @@
72 self.patch(credentials.Keyring, 'get_credentials',
73 lambda kr, app: defer.succeed(TOKEN))
74 error = 'Bla'
75- self.patch(credentials.urllib2, 'urlopen',
76+ self.patch(credentials.curllib, 'urlopen',
77 lambda *a, **kw: self.fail(error))
78 self._cred_cleared = False
79 self.patch(self.obj, 'clear_credentials',
80@@ -359,14 +359,14 @@
81 code=200)
82 return response
83
84- self.patch(credentials.urllib2, 'urlopen', faked_urlopen)
85+ self.patch(credentials.curllib, 'urlopen', faked_urlopen)
86 self.patch(credentials.utils.timestamp_checker, "get_faithful_time",
87 time.time)
88
89 @inlineCallbacks
90 def test_ping_url_if_url_is_none(self):
91 """self.ping_url is opened."""
92- self.patch(credentials.urllib2, 'urlopen', self.fail)
93+ self.patch(credentials.curllib, 'urlopen', self.fail)
94 self.obj.ping_url = None
95 yield self.obj._ping_url(app_name=APP_NAME, email=EMAIL,
96 credentials=TOKEN)
97@@ -378,7 +378,7 @@
98 yield self.obj._ping_url(app_name=APP_NAME, email=EMAIL,
99 credentials=TOKEN)
100
101- self.assertIsInstance(self._request, credentials.urllib2.Request)
102+ self.assertIsInstance(self._request, credentials.curllib.Request)
103 self.assertEqual(self._request.get_full_url(),
104 self.obj.ping_url + EMAIL)
105
106@@ -402,7 +402,7 @@
107 def test_ping_url_error(self):
108 """Exception is handled if ping fails."""
109 error = 'Blu'
110- self.patch(credentials.urllib2, 'urlopen', lambda r: self.fail(error))
111+ self.patch(credentials.curllib, 'urlopen', lambda r: self.fail(error))
112
113 yield self.obj._ping_url(APP_NAME, EMAIL, TOKEN)
114
115
116=== added file 'ubuntu_sso/utils/curllib.py'
117--- ubuntu_sso/utils/curllib.py 1970-01-01 00:00:00 +0000
118+++ ubuntu_sso/utils/curllib.py 2012-06-15 22:15:26 +0000
119@@ -0,0 +1,147 @@
120+# -*- coding: utf-8 -*-
121+
122+# Author: Alejandro J. Cura <alecu@canonical.com>
123+#
124+# Copyright 2011 Canonical Ltd.
125+#
126+# This program is free software: you can redistribute it and/or modify it
127+# under the terms of the GNU General Public License version 3, as published
128+# by the Free Software Foundation.
129+#
130+# This program is distributed in the hope that it will be useful, but
131+# WITHOUT ANY WARRANTY; without even the implied warranties of
132+# MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
133+# PURPOSE. See the GNU General Public License for more details.
134+#
135+# You should have received a copy of the GNU General Public License along
136+# with this program. If not, see <http://www.gnu.org/licenses/>.
137+
138+"""Web client based on pycurl, with an API similar to urllib2."""
139+
140+import httplib
141+
142+from email import parser
143+from StringIO import StringIO
144+
145+import pycurl
146+
147+
148+class HTTPError(Exception):
149+ """Error that happens while doing a web request."""
150+
151+ def __init__(self, url, code, message, headers=None, fp=None):
152+ """Initialize this instance."""
153+ super(HTTPError, self).__init__()
154+ self.code = code
155+ self.message = message
156+ if fp:
157+ self.fp = fp
158+ else:
159+ self.fp = StringIO(message)
160+
161+ def read(self):
162+ """Return the error message."""
163+ return self.fp.read()
164+
165+ def __str__(self):
166+ return "<%s: %s>" % (self.code, self.message)
167+
168+
169+class UnauthorizedError(HTTPError):
170+ """An HTTP error when there's an auth problem."""
171+
172+
173+class Request(object):
174+ """An HTTP request object."""
175+
176+ def __init__(self, url, data=None, headers=None):
177+ """Initialize this instance."""
178+ super(Request, self).__init__()
179+ self.url = url
180+ self.data = data
181+ self.headers = headers
182+
183+ def get_full_url(self):
184+ """Return the url."""
185+ return self.url
186+
187+ def get_data(self):
188+ """Return the data."""
189+ return self.data
190+
191+
192+class Response(StringIO):
193+ """An HTTP response object."""
194+
195+ code = -1
196+ headers = None
197+
198+ def __init__(self, url):
199+ """Initialize this instance."""
200+ StringIO.__init__(self)
201+ self.url = url
202+
203+ def finish(self, code, headers):
204+ """Finish a response and rewind it."""
205+ self.code = code
206+ self.headers = headers
207+ self.seek(0)
208+
209+
210+class HeaderParser(parser.FeedParser):
211+ """A parser for the HTTP headers, based on the stdlib email package."""
212+
213+ first_line = True
214+
215+ def feed(self, data):
216+ """Feed some data, but chomp the first line."""
217+ if self.first_line:
218+ self.first_line = False
219+ return
220+ parser.FeedParser.feed(self, data)
221+
222+
223+def urlopen(request, data=None):
224+ """Open a given url using curl."""
225+ if isinstance(request, basestring):
226+ request = Request(request, data)
227+
228+ curl = pycurl.Curl()
229+ try:
230+ request_headers = []
231+ if isinstance(request.url, unicode):
232+ request.url = request.url.encode("utf-8")
233+ curl.setopt(pycurl.URL, request.url)
234+ if request.headers:
235+ for key, value in request.headers.items():
236+ request_headers.append("%s: %s" % (key, value))
237+ curl.setopt(pycurl.HTTPHEADER, request_headers)
238+ response = Response(request.url)
239+ response_headers_parser = HeaderParser()
240+ curl.setopt(pycurl.WRITEFUNCTION, response.write)
241+ curl.setopt(pycurl.HEADERFUNCTION, response_headers_parser.feed)
242+ curl.setopt(pycurl.FOLLOWLOCATION, 1)
243+ curl.setopt(pycurl.MAXREDIRS, 5)
244+ curl.setopt(pycurl.SSL_VERIFYPEER, 1)
245+ curl.setopt(pycurl.SSL_VERIFYHOST, 2)
246+ if request.data:
247+ curl.setopt(pycurl.POST, 1)
248+ curl.setopt(pycurl.POSTFIELDS, request.data)
249+ curl.perform()
250+ code = curl.getinfo(pycurl.HTTP_CODE)
251+ response_headers = response_headers_parser.close()
252+ response.finish(code, response_headers)
253+ except pycurl.error as e:
254+ raise HTTPError(request.url, e[0], curl.errstr())
255+ else:
256+ if code in (200, 0):
257+ return response
258+ else:
259+ if code == 401:
260+ errorclass = UnauthorizedError
261+ else:
262+ errorclass = HTTPError
263+ message = httplib.responses.get(code, "Unknown error")
264+ raise errorclass(request.url, code, message, response)
265+ finally:
266+ curl.close()
267
268=== added file 'ubuntu_sso/utils/tests/test_curllib.py'
269--- ubuntu_sso/utils/tests/test_curllib.py 1970-01-01 00:00:00 +0000
270+++ ubuntu_sso/utils/tests/test_curllib.py 2012-06-15 22:15:26 +0000
271@@ -0,0 +1,300 @@
272+# -*- coding: utf-8 -*-
273+
274+# Author: Alejandro J. Cura <alecu@canonical.com>
275+#
276+# Copyright 2011 Canonical Ltd.
277+#
278+# This program is free software: you can redistribute it and/or modify it
279+# under the terms of the GNU General Public License version 3, as published
280+# by the Free Software Foundation.
281+#
282+# This program is distributed in the hope that it will be useful, but
283+# WITHOUT ANY WARRANTY; without even the implied warranties of
284+# MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
285+# PURPOSE. See the GNU General Public License for more details.
286+#
287+# You should have received a copy of the GNU General Public License along
288+# with this program. If not, see <http://www.gnu.org/licenses/>.
289+
290+"""Tests for the pycurl based web client."""
291+
292+import cgi
293+import urllib
294+
295+from twisted.application import internet, service
296+from twisted.internet import defer, threads
297+from twisted.trial.unittest import TestCase
298+from twisted.web import http, resource, server
299+
300+from ubuntu_sso.utils import curllib
301+
302+
303+SAMPLE_URL = "http://protocultura.net/"
304+SAMPLE_KEY = "result"
305+SAMPLE_VALUE = "sample result"
306+SAMPLE_RESOURCE = '{"%s": "%s"}' % (SAMPLE_KEY, SAMPLE_VALUE)
307+SAMPLE_CREDENTIALS = dict(
308+ consumer_key="consumer key",
309+ consumer_secret="consumer secret",
310+ token="the real token",
311+ token_secret="the token secret",
312+)
313+SAMPLE_HEADERS = {SAMPLE_KEY: SAMPLE_VALUE}
314+SAMPLE_PARAMS = SAMPLE_HEADERS
315+SAMPLE_RAW_HEADERS = [
316+ "HTTP/1.1 200 OK\r\n",
317+ "%s: %s\r\n" % (SAMPLE_KEY, SAMPLE_VALUE),
318+ "Multiline: This\r\n",
319+ " is a sample\r\n",
320+ " multiline header.\r\n",
321+ "\r\n",
322+]
323+
324+
325+# pylint: disable=C0103
326+# t.w.resource methods have freeform cased names
327+
328+class MockResource(resource.Resource):
329+ """A simple web resource."""
330+ isLeaf = True
331+ contents = ""
332+
333+ def getChild(self, name, request):
334+ """Get a given child resource."""
335+ if name == '':
336+ return self
337+ return resource.Resource.getChild(self, name, request)
338+
339+ def render_GET(self, request):
340+ """Make a bit of html out of these resource's content."""
341+ return self.contents
342+
343+
344+class VerifyHeadersResource(resource.Resource):
345+ """A resource that verifies the headers received."""
346+
347+ def render_GET(self, request):
348+ """Make a bit of html out of these resource's content."""
349+ headers = request.requestHeaders.getRawHeaders(SAMPLE_KEY)
350+ if headers != [SAMPLE_VALUE]:
351+ request.setResponseCode(http.BAD_REQUEST)
352+ return "ERROR: Expected header not present."
353+ return SAMPLE_RESOURCE
354+
355+
356+class VerifyPostResource(resource.Resource):
357+ """A resource that verifies the post was ."""
358+
359+ def render_POST(self, request):
360+ """Make a bit of html out of these resource's content."""
361+ values = cgi.escape(request.args[SAMPLE_KEY][0])
362+ if values != SAMPLE_VALUE:
363+ request.setResponseCode(http.BAD_REQUEST)
364+ return "ERROR: Expected value not present."
365+ return SAMPLE_RESOURCE
366+
367+
368+class HeadedResource(resource.Resource):
369+ """A resource that sends some response headers."""
370+
371+ def render_GET(self, request):
372+ """Make a bit of html out of these resource's content."""
373+ request.setHeader(SAMPLE_KEY, SAMPLE_VALUE)
374+ return SAMPLE_RESOURCE
375+
376+# pylint: enable=C0103
377+
378+
379+class MockWebService(object):
380+ """A mock webservice for testing"""
381+
382+ def __init__(self):
383+ """Start up this instance."""
384+ root = resource.Resource()
385+ mock_resource = MockResource()
386+ mock_resource.contents = SAMPLE_RESOURCE
387+ root.putChild("mock_resource", mock_resource)
388+ root.putChild("throwerror", resource.NoResource())
389+ unauthorized = resource.ErrorPage(resource.http.UNAUTHORIZED,
390+ "Unauthorized", "Unauthorized")
391+ root.putChild("unauthorized", unauthorized)
392+ root.putChild("verifyheaders", VerifyHeadersResource())
393+ root.putChild("verifypost", VerifyPostResource())
394+ root.putChild("headed_resource", HeadedResource())
395+
396+ site = server.Site(root)
397+ application = service.Application('web')
398+ self.service_collection = service.IServiceCollection(application)
399+ #pylint: disable=E1101
400+ self.tcpserver = internet.TCPServer(0, site)
401+ self.tcpserver.setServiceParent(self.service_collection)
402+ self.service_collection.startService()
403+
404+ def get_url(self):
405+ """Build the url for this mock server."""
406+ #pylint: disable=W0212
407+ port_num = self.tcpserver._port.getHost().port
408+ return "http://localhost:%d/" % port_num
409+
410+ def stop(self):
411+ """Shut it down."""
412+ #pylint: disable=E1101
413+ return self.service_collection.stopService()
414+
415+
416+class HeaderParserCase(TestCase):
417+ """Test the HeaderParser class."""
418+
419+ def parse(self, lines):
420+ """Feed a parser with some lines, and return the result."""
421+ parser = curllib.HeaderParser()
422+ for line in lines:
423+ parser.feed(line)
424+ return parser.close()
425+
426+ def test_skips_first_element(self):
427+ """It skips the first element (the status code)."""
428+ test_headers = SAMPLE_RAW_HEADERS[0:1]
429+ result = self.parse(test_headers)
430+ self.assertEqual(len(result.keys()), 0)
431+
432+ def test_skips_last_element(self):
433+ """It skips the last element (an empty line with CR/LF)."""
434+ test_headers = [SAMPLE_RAW_HEADERS[0], SAMPLE_RAW_HEADERS[-1]]
435+ result = self.parse(test_headers)
436+ self.assertEqual(len(result.keys()), 0)
437+
438+ def test_parses_the_rest(self):
439+ """It parses all the rest."""
440+ result = self.parse(SAMPLE_RAW_HEADERS)
441+ self.assertEqual(len(result.keys()), 2)
442+ self.assertEqual(result[SAMPLE_KEY], SAMPLE_VALUE)
443+
444+
445+class CurllibTestCase(TestCase):
446+ """Tests for the curllib."""
447+
448+ @defer.inlineCallbacks
449+ def setUp(self):
450+ """Initialize this testcase."""
451+ yield super(CurllibTestCase, self).setUp()
452+ self.ws = MockWebService()
453+ self.url = self.ws.get_url()
454+ self.addCleanup(self.ws.stop)
455+
456+ def urlopen_in_thread(self, *args, **kwargs):
457+ """Run curllib in a thread, so it doesn't block the mock webserver."""
458+ return threads.deferToThread(curllib.urlopen, *args, **kwargs)
459+
460+ @defer.inlineCallbacks
461+ def test_urlopen(self):
462+ """Test a simple urlopen."""
463+ response = yield self.urlopen_in_thread(self.url + "mock_resource")
464+ self.assertEqual(response.read(), SAMPLE_RESOURCE)
465+
466+ @defer.inlineCallbacks
467+ def test_urlopen_unicode(self):
468+ """Test an unicode url."""
469+ url_path = u"mock_resource?test=ñandú"
470+ response = yield self.urlopen_in_thread(self.url + url_path)
471+ self.assertEqual(response.read(), SAMPLE_RESOURCE)
472+
473+ @defer.inlineCallbacks
474+ def test_urlopen_receiving_headers(self):
475+ """Test urlopen receiving headers."""
476+ response = yield self.urlopen_in_thread(self.url + "headed_resource")
477+ self.assertEqual(response.headers[SAMPLE_KEY], SAMPLE_VALUE)
478+
479+ @defer.inlineCallbacks
480+ def test_urlopen_sending_headers(self):
481+ """Test urlopen sending headers."""
482+ request = curllib.Request(self.url + "verifyheaders",
483+ headers=SAMPLE_HEADERS)
484+ response = yield self.urlopen_in_thread(request)
485+ self.assertEqual(SAMPLE_RESOURCE, response.read())
486+
487+ @defer.inlineCallbacks
488+ def test_urlopen_post_parameters(self):
489+ """Test urlopen with POST parameters."""
490+ data = urllib.urlencode(SAMPLE_PARAMS)
491+ request = curllib.Request(self.url + "verifypost", data=data)
492+ response = yield self.urlopen_in_thread(request)
493+ self.assertEqual(SAMPLE_RESOURCE, response.read())
494+
495+ @defer.inlineCallbacks
496+ def test_urlopen_unauthorized(self):
497+ """Test urlopen with unauthorized urls."""
498+ d = self.urlopen_in_thread(self.url + "unauthorized")
499+ e = yield self.assertFailure(d, curllib.UnauthorizedError)
500+ self.assertEqual(e.code, 401)
501+
502+ @defer.inlineCallbacks
503+ def test_urlopen_some_other_error(self):
504+ """Test urlopen with some other error."""
505+ d = self.urlopen_in_thread(self.url + "throwerror")
506+ e = yield self.assertFailure(d, curllib.HTTPError)
507+ self.assertEqual(e.code, 404)
508+
509+ @defer.inlineCallbacks
510+ def test_connection_failure(self):
511+ """Test a failure to connect."""
512+ invalid_url = "http://localhost:99999/" # the port is way over 65535!
513+ d = self.urlopen_in_thread(invalid_url)
514+ e = yield self.assertFailure(d, curllib.HTTPError)
515+ self.assertEqual(e.code, curllib.pycurl.E_URL_MALFORMAT)
516+
517+
518+class ResponseTestCase(TestCase):
519+ """Tests for the Response class."""
520+
521+ def test_rewinds_on_finish(self):
522+ """The buffer is rewinded when the response is finished."""
523+ response = curllib.Response(SAMPLE_URL)
524+ response.write(SAMPLE_KEY)
525+ response.write(SAMPLE_VALUE)
526+ response.finish(200, {})
527+ self.assertEqual(response.read(), SAMPLE_KEY + SAMPLE_VALUE)
528+
529+
530+class RequestTestCase(TestCase):
531+ """Tests for the Request class."""
532+
533+ def test_get_full_url(self):
534+ """Test the get_full_url method."""
535+ request = curllib.Request(SAMPLE_URL)
536+ self.assertEqual(request.get_full_url(), SAMPLE_URL)
537+
538+
539+class FakeCurl(object):
540+ """A fake Curl that records options set."""
541+
542+ def __init__(self):
543+ """Initialize this fake."""
544+ self.options = {}
545+
546+ def setopt(self, key, value):
547+ """Save a copy of the option."""
548+ self.options[key] = value
549+
550+ def getinfo(self, key):
551+ """Fake a finished operation."""
552+ if key == curllib.pycurl.HTTP_CODE:
553+ return 200
554+
555+ def perform(self):
556+ """Do nothing."""
557+
558+ def close(self):
559+ """Do nothing, too."""
560+
561+
562+class SslVerificationTestCase(TestCase):
563+ """Tests the curllib SSL verification."""
564+
565+ def test_ssl_is_verified(self):
566+ """The ssl verification flags are set on the curl object."""
567+ fake_curl = FakeCurl()
568+ self.patch(curllib.pycurl, "Curl", lambda: fake_curl)
569+ curllib.urlopen("http://localhost:1234")
570+ self.assertEqual(fake_curl.options[curllib.pycurl.SSL_VERIFYPEER], 1)
571+ self.assertEqual(fake_curl.options[curllib.pycurl.SSL_VERIFYHOST], 2)

Subscribers

People subscribed via source and target branches