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

Return 405 correctly even if version is using as header and wrong request method #1362

Merged
merged 3 commits into from
Apr 11, 2016

Conversation

namusyaka
Copy link
Contributor

This reverts commit a0166e7.

I'll try to fix this problem drastically.

@namusyaka namusyaka changed the title Revert "Return 405 even if version is using as header" Return 405 correctly even if version is using as header and wrong request method Apr 11, 2016
@namusyaka
Copy link
Contributor Author

@dblock Could you take a look this pull request?
I think this approach can fix potential bugs and issues.
If ok, we can release 0.16.2 version ;)

@@ -231,7 +231,7 @@ def run

run_filters before_validations, :before_validation

run_validators validations, request unless @method_not_allowed
run_validators validations, request unless env[Grape::Env::GRAPE_METHOD_NOT_ALLOWED]
Copy link
Member

Choose a reason for hiding this comment

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

I see it now :) Better.

Copy link
Member

Choose a reason for hiding this comment

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

You might want to audit the use if @variable elsewhere, we have at least an @app_response in a formatter middleware, @named_params in a DSL helper and @versions in routing.

@dblock dblock merged commit 5122b9a into master Apr 11, 2016
@dblock dblock mentioned this pull request Apr 11, 2016
@dblock
Copy link
Member

dblock commented Apr 11, 2016

This worked everywhere where I saw problems, feel free to cut 0.16.2, which is #1360. I would review the use of @... first though.

@namusyaka
Copy link
Contributor Author

The @method_not_allowed variable usage is wrong. It returns true eternally if method_not_allowed is set even once.
Other @~ vars looks like no problem.

@dblock
Copy link
Member

dblock commented Apr 11, 2016

I should have caught @ in the code reviews too, my bad.

@namusyaka
Copy link
Contributor Author

I've mistaken the coding. Sorry for the bad patch. Btw, we can release the v.0.16.2 now. I'll prepare it soon.

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.

2 participants