toolbelt
toolbelt copied to clipboard
verify=False per request doesn't work on App Engine
hi all! i noticed recently that the app engine adapter only supports enabling or disabling SSL certification validation globally, not per request. this is a bit of a surprise abstraction leak since it still happily accepts verify=False or =True in e.g. requests.get(), but silently ignores it.
you can reproduce this on https://shell-hrd.appspot.com/ , which has the latest stable versions of requests (2.18.4), toolbelt (0.8.0), and urllib3 (1.22):
>>> import requests
>>> requests.get('https://hnz.io/', verify=False)
...
Traceback (most recent call last):
File "...//shell.py", line 269, in get
exec compiled in statement_module.__dict__
File "<string>", line 1, in <module>
File ".../local/lib/python2.7/site-packages/requests/api.py", line 72, in get
return request('get', url, params=params, **kwargs)
File ".../local/lib/python2.7/site-packages/requests/api.py", line 58, in request
return session.request(method=method, url=url, **kwargs)
File ".../local/lib/python2.7/site-packages/requests/sessions.py", line 508, in request
resp = self.send(prep, **send_kwargs)
File ".../local/lib/python2.7/site-packages/requests/sessions.py", line 618, in send
r = adapter.send(request, **kwargs)
File ".../local/lib/python2.7/site-packages/requests/adapters.py", line 519, in send
raise SSLError(e, request=request)
SSLError: Invalid and/or missing SSL certificate for URL: https://hnz.io/
urllib3's contrib.appengine.AppEngineManager also seems to set its validate_certificate globally, or at least once per instance (which i'm guessing is usually long lived), so i'm not sure if you'd rather this to be filed here, there, or elsewhere. happy to move it if you want.
thanks in advance!
forgot to mention, the toolbelt setup code in https://shell-hrd.appspot.com/ is:
from requests_toolbelt.adapters import appengine
appengine.monkeypatch()
I'll defer to @jonparrott on this as most of the code is theirs (both in urllib3 and the toolbelt). It looks like the urllib3 connection pool has been behaving that way for 2 years and I think it was an honest oversight rather than anything deliberate or intentional. If Jon can confirm that, I will happily take on the task of updating this if you don't want to, @snarfed
So verify/validate_certificate is not a usual parameter to urllib3's urlopen method. Looking at requests it seems it modifies the actual connection it gets from the pool to do verification.
It seems we should just add a validate_certificate option to AppEngineManager.urlopen that can override the class-level attribute. Toolbelt's adapter can then just specify that if verify is set.
I am going on vacation for the next week, so if someone else wants to do this go for it. :)
I wonder if we could just instantiate two AppEngineManager objects and then route to each one based on verify.
@sigmavirus24 that seems pretty reasonable.