Skip to content

Conversation

honestbleeps
Copy link
Contributor

This checks for the presence of a question mark in the URI and changes the separator accordingly so as to not put two questions marks in the URI.

fixes #238

…ecause the redirect_uri may already contain URL parameters
@masci
Copy link
Contributor

masci commented Apr 28, 2015

Changes look good to me. Do you mind to fix the tests?

@masci masci added this to the 0.8.2 milestone Apr 28, 2015
@honestbleeps
Copy link
Contributor Author

sorry about that, I will check out the tests today!

@honestbleeps
Copy link
Contributor Author

tests are running now, should be fixed!

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 95.94% when pulling 27fe47b on honestbleeps:redirect-fix into bb62289 on evonove:master.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry for being pedantic but I don't like except: pass very much ;)
What about something like:

redirect_uri = oauthlib_error.redirect_uri or ""
separator = '&' if '?' in redirect_uri else '?'

Also, I guess a test case checking the double ? problem (a test that fails without your patch) should be added

@masci masci merged commit 27fe47b into django-oauth:master May 23, 2015
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.

redirect uri on error (e.g. AuthCanceled) may contain two question marks, thus sending the wrong exception

3 participants