Reject blank ids so that they did not accidentally go into query for existing records

rails / protected_attributes

Protect attributes from mass-assignment in ActiveRecord models.

MIT License

228 stars 92 forks source link

Reject blank ids so that they did not accidentally go into query for existing records #79

Closed prathamesh-sonpatki closed 8 years ago

prathamesh-sonpatki commented 8 years ago

For an attribute collection like -

[{"id" => 1}, {"id" => "'"}]

attribute_ids will become [1, ""] as per existing code.
As the first element of attribute collection has a valid id, we try to load "existing records" with attribute_ids [1, ""] resulting into an error on (PG) database adapter.
After this change, the attribute_ids will reject all the blank values so that the query will be unaffected.

rails-bot commented 8 years ago

Thanks for the pull request, and welcome! The Rails team is excited to review your changes, and you should hear from @rafaelfranca (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

prathamesh-sonpatki commented 8 years ago

Looking into test failures.

prathamesh-sonpatki commented 8 years ago

Not sure locally build is green for Gemfile-rails-4.2.

prathamesh-sonpatki commented 8 years ago

@rafaelfranca How should we add test for this issue? I am not able to reproduce it for sqlite, but it happens on a PG app which I am migrating from Rails 3.2 to Rails 4.0.13

This is the query and the error -

SELECT "customer_invoice_accessorials".* 
FROM "customer_invoice_accessorials"  
WHERE "customer_invoice_accessorials"."customer_invoice_id" = $1 
AND "customer_invoice_accessorials"."id" IN ('1', '')

ActiveRecord::StatementInvalid: PG::InvalidTextRepresentation: ERROR:  invalid input syntax for integer: ""
LINE 1: ..." = $1 AND "customer_invoice_accessorials"."id" IN ('1', '')
                                                                    ^

prathamesh-sonpatki commented 8 years ago

We were able to solve it other way without interfering with protected_attributes :smile: