Skip to content

Conversation

@tommarmstrong
Copy link
Contributor

This change strips new line characters from the response body when requesting access tokens from an oauth server. This solves an error I was encountering when trying to authenticate against the WordPress API which returns a new line after the headers and before the URI-encoded response.

This is the format specified in Section 2.3 of RFC 5849 however validation of the response was failing as \r and \n are not safe characters as defined by oauthlib. This caused an exception to be thrown when calling oauthlib's urldecode function on line 30 of oauth1_session.py. By striping those characters before calling urldecode, the presence of a new line in the response body does not cause an exception to be thrown.

@coveralls
Copy link

coveralls commented Oct 13, 2016

Coverage Status

Coverage remained the same at 86.758% when pulling 9159b57 on tommarmstrong:master into 244e835 on requests:master.

@Lukasa
Copy link
Member

Lukasa commented Oct 14, 2016

One HTTP line delimiter (CRLF) should have been fine, because that's used as the delimiter for the end of the header block. However, your bug seems to be caused by Wordpress adding an extra line delimiter, which is factually part of the request body.

While they really shouldn't do that, I see no reason not to strip whitespace from the token body here.

I freaking hate OAuth. 😭

@Lukasa Lukasa merged commit 40a0754 into requests:master Oct 14, 2016
@tommarmstrong
Copy link
Contributor Author

Hahaha, relatable. Thanks for merging :)

@tommarmstrong
Copy link
Contributor Author

Hi @Lukasa, when do you think this will make it into a release? Thanks

@Lukasa
Copy link
Member

Lukasa commented Feb 9, 2017

Hrm, probably sometime soon. I'll add a TODO to release early next week.

@tommarmstrong
Copy link
Contributor Author

tommarmstrong commented Feb 9, 2017 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants