Specification improvement and bug fix to sign_request #138
Conversation
Per the OAuth Body Hash specifications (3.2)[1], if there is no body entity, then the hash should be done over the empty string. Also, when httplib2 handles a redirect, the follow is performed with None as the request body. This causes a TypeError to be thrown when attempting to hash the body here. This commit fixes the bug while improving the specification alignment. [1]http://oauth.googlecode.com/svn/spec/ext/body_hash/1.0/oauth-bodyhash.html
|
Is this similar to the fix or different from #163? |
|
It is similar to the fix in #163 but more correct. #163 skips the hash if the request does not have a body. My change properly hashes the empty string when there's no body, as specified in section 3.2. I also noticed you asked for a regression test in #163, I'm happy to add that here. |
|
@rickhanlonii You answered my other question in that PR as well. Please add a regression test. @jaitaiwan I'm 👍 after the test has been added. Can you take a peak? |
|
Added a test. Before my change the test I added would fail with: Error
Traceback (most recent call last):
File "/Library/Python/2.7/site-packages/mock.py", line 1190, in patched
return func(*args, **keywargs)
File "/Users/euler/dev/python-oauth2/tests/test_oauth.py", line 1322, in test_url_with_query_string_body_none
client.request(uri, 'GET', body=None)
File "/Users/euler/dev/python-oauth2/oauth2/__init__.py", line 668, in request
req.sign_request(self.method, self.consumer, self.token)
File "/Users/euler/dev/python-oauth2/oauth2/__init__.py", line 499, in sign_request
self['oauth_body_hash'] = base64.b64encode(sha(self.body).digest())
TypeError: must be string or buffer, not None |
|
MMmmmm ... regression tests. 👍 cc @jaitaiwan |
|
@jaitaiwan aaaaannnnnnnnnd Travis has arisen from the dead. PRs are building and tests are passing in 2.6 and 2.7. |
|
Fantastic. LGTM |
|
@rickhanlonii can you do me a favour and get this rebased off master? |
|
Sure will 👌 |
| @@ -485,8 +485,14 @@ def sign_request(self, signature_method, consumer, token): | |||
| """Set the signature parameter to the result of sign.""" | |||
|
|
|||
| if not self.is_form_encoded: | |||
| # according to | |||
| # according to | |||
Per the OAuth Body Hash specifications (3.2), if there is no body entity, then the hash should be done over the empty string.
Also, when
httplib2handles a redirect, the follow is performed withNoneas the request body. This causes a TypeError to be thrown here when attempting to hash the body.This commit prevents the TypeError, while improving the specification alignment.
"Win, win, win." -Michael Scott
Should fix issue #112
Should close pull #113
Many thanks to @holm!