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(oauth2) dont try to hide credentials in a multipart req body #2176

Merged
merged 1 commit into from
Mar 14, 2017

Conversation

p0pr0ck5
Copy link
Contributor

@p0pr0ck5 p0pr0ck5 commented Mar 8, 2017

Summary

Don't try to remove credentials from request bodies when the request is a multipart upload. This prevents a thread abort when trying to index a non-existent table.

Full changelog

  • fix(oauth2) dont try to hide credentials in a multipart req body

Issues resolved

Fix #1952

@p0pr0ck5 p0pr0ck5 force-pushed the fix/oauth2_multipart_hide_cred branch from d841124 to 597a843 Compare March 8, 2017 18:02
@p0pr0ck5
Copy link
Contributor Author

p0pr0ck5 commented Mar 8, 2017

Failed test seems unrelated to this change.

if ngx.req.get_method() ~= "GET" then -- Remove from body
local content_type = req_get_headers()[CONTENT_TYPE]
local is_upload = content_type and
string_find(content_type:lower(), "multipart/form-data", 1, true)
Copy link
Member

Choose a reason for hiding this comment

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

no need for lower() here

Since the 0.6.9 release, all the header names in the Lua table returned are converted to the pure lower-case form by default, unless the raw argument is set to true (default to false).

@p0pr0ck5 p0pr0ck5 force-pushed the fix/oauth2_multipart_hide_cred branch 2 times, most recently from 7acf6b3 to 767944c Compare March 13, 2017 16:51
@p0pr0ck5
Copy link
Contributor Author

@Tieske able to take another look at this one? rebased on top of next to fix a merge conflict

@thibaultcha
Copy link
Member

To be more protective, shouldn't get_post_args() only be called when the MIME type is application/x-www-form-urlencoded?

@p0pr0ck5 p0pr0ck5 force-pushed the fix/oauth2_multipart_hide_cred branch from 767944c to 1dd16bb Compare March 13, 2017 23:58
@thibaultcha thibaultcha merged commit b16a5cc into next Mar 14, 2017
@thibaultcha thibaultcha deleted the fix/oauth2_multipart_hide_cred branch March 14, 2017 02:03
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