Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix retrieving oauth2 params from body #1853

Closed
wants to merge 1 commit into from
Closed

Conversation

eddmann
Copy link
Contributor

@eddmann eddmann commented Nov 29, 2016

Summary

There is an issue when you supply a JSON request with an empty body to the OAuth2 plugin.
As the body is deemed to be 'nil' the JSON decode function does not like this (as it expects a string) and throws.

Issues resolved

This commit short circuits the retrieval process if the request method is GET or HEAD (as these do not provide bodies), and in the event that no body is supplied in another request type it provides a default empty JSON object to satisfy the decode process.

There is an issue when you supply a JSON request with an empty body to the OAuth2 plugin.
As the body is deemed to be 'nil' the JSON decode function does not like this (as it expects a string) and throws.
This commit short circuits the retrieval process if the request method is GET or HEAD (as these do not provide bodies), and in the event that no body is supplied in another request type it provides a default empty JSON object to satisfy the decode process.
@eddmann
Copy link
Contributor Author

eddmann commented Nov 29, 2016

Having issues setting up the kong-vagrant box as it complains about resty not being available so have not been able to include and run test-suite against the changes.

@Tieske
Copy link
Member

Tieske commented Nov 30, 2016

see also #1773

@djMax
Copy link

djMax commented Dec 15, 2016

This bug hits us too...

@subnetmarco
Copy link
Member

Closed with #1915

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.

4 participants