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 HTTP 400 Bad Request when include is present and not allowed #351

Merged
merged 1 commit into from Jul 31, 2015

Conversation

koppen
Copy link
Contributor

@koppen koppen commented Jul 31, 2015

From http://jsonapi.org/format/#fetching-includes:

If an endpoint does not support the include parameter, it must respond with
400 Bad Request to any requests that include it.

This change makes jsonapi-resources adhere to cases where
allowed_request_params has been configured to not allow the include
parameter. Previously, this configuration option was silently ignored and
resources were included in the output regardless.

From http://jsonapi.org/format/#fetching-includes:

> If an endpoint does not support the include parameter, it must respond with
> 400 Bad Request to any requests that include it.

This change makes jsonapi-resources adhere to cases where
`allowed_request_params` has been configured to not allow the `include`
parameter. Previously, this configuration option was silently ignored and
resources were included in the output regardless.
@lgebhardt lgebhardt mentioned this pull request Jul 31, 2015
@lgebhardt
Copy link
Member

You have uncovered a bigger issue. The allowed_request_params configuration option is no longer being used (except in your new code). The original intent wasn't to allow includes (or the other params) to be excluded, but to provide a master list, that could be added to, of params to enforce with strong parameters. This use of strong params was removed a while ago, but I never took this configuration option out as I should have.

So I reworked this feature based on your PR, see #352. I think a boolean in the configuration for each of the optional parameters is clearer.

JSONAPI.configure do |config|
  config.allow_include = false
  #config.allow_sort = true
  #config.allow_filter = true
end

@koppen
Copy link
Contributor Author

koppen commented Jul 31, 2015

That's awesome and much better, cheers! :)

@dgeb dgeb merged commit 78fb155 into cerebris:master Jul 31, 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.

None yet

3 participants