Postgres 'ProtocolViolation' error with overlapping abilities #163

Closed
fattymiller opened this Issue Jan 9, 2015 · 8 comments

Projects

None yet

4 participants

@fattymiller

I have just updated to Ruby 2.2.0 and Rails 4.2 for this application and am getting the following in Postgres when using cancancan.

PG::ProtocolViolation: ERROR:  bind message supplies 0 parameters, but prepared statement "" requires 2

tl;dr
The issue seems to be that the SQL parameters aren't passed in with the query when two or more abilities overlap.

WHERE (("company_users"."acl" = $1 AND "company_users"."user_id" = $2) OR ("company_users"."acl" = $1 AND "company_users"."user_id" = $2))

versus

WHERE "company_users"."acl" = $1 AND "company_users"."user_id" = $2  [["acl", "admin"], ["user_id", 1]]

With my ability.rb file setup with the following:

    can :manage, Company, company_users: { acl: "admin", user_id: user.id }
    can :index, Company, company_users: { acl: "manager", user_id: user.id }

I get the error below, but if either one of the ability declarations are commented out, or they are changed to be exclusive of each other (example: :admin_test and :manager_test) the code works as expected.

When calling Company.accessible_by(User.first.ability) I have observed the following:

Using cancan (1.6.10):

[1] pry(main)> Company.accessible_by(User.first.ability)
  User Load (1.3ms)  SELECT  "users".* FROM "users"  ORDER BY "users"."id" ASC LIMIT 1
DEPRECATION WARNING: sanitize_sql_hash_for_conditions is deprecated, and will be removed in Rails 5.0
. (called from <main> at (pry):1)
DEPRECATION WARNING: sanitize_sql_hash_for_conditions is deprecated, and will be removed in Rails 5.0
. (called from <main> at (pry):1)
  Company Load (1.7ms)  SELECT "companies".* FROM "companies" INNER JOIN "company_users" ON "company_users"."company_id" = "companies"."id" WHERE (("company_users"."acl" = 'manager' AND "company_users"."user_id" = 1) OR ("company_users"."acl" = 'admin' AND "company_users"."user_id" = 1))
=> [#<Company:0x007fcb71d00fe0 id: 1, name: "Company 1", created_at: Fri, 09 Jan 2015 04:43:34 UTC +00:00, updated_at: Fri, 09 Jan 2015 04:43:34 UTC +00:00>]

Using Postgres and cancancan (master, develop or 1.10.0; Postgres 0.17.1 or 0.18.1):

[1] pry(main)> Company.accessible_by(User.first.ability)
  User Load (1.2ms)  SELECT  "users".* FROM "users"  ORDER BY "users"."id" ASC LIMIT 1
  SQL (2.4ms)  SELECT "companies"."id" AS t0_r0, "companies"."name" AS t0_r1, "companies"."created_at" AS t0_r2, "companies"."updated_at" AS t0_r3, "company_users"."id" AS t1_r0, "company_users"."company_id" AS t1_r1, "company_users"."user_id" AS t1_r2, "company_users"."effective" AS t1_r3, "company_users"."expire" AS t1_r4, "company_users"."acl" AS t1_r5, "company_users"."created_at" AS t1_r6, "company_users"."updated_at" AS t1_r7 FROM "companies" LEFT OUTER JOIN "company_users" ON "company_users"."company_id" = "companies"."id" WHERE (("company_users"."acl" = $1 AND "company_users"."user_id" = $2) OR ("company_users"."acl" = $1 AND "company_users"."user_id" = $2))
PG::ProtocolViolation: ERROR:  bind message supplies 0 parameters, but prepared statement "" requires 2
: SELECT "companies"."id" AS t0_r0, "companies"."name" AS t0_r1, "companies"."created_at" AS t0_r2, "companies"."updated_at" AS t0_r3, "company_users"."id" AS t1_r0, "company_users"."company_id" AS t1_r1, "company_users"."user_id" AS t1_r2, "company_users"."effective" AS t1_r3, "company_users"."expire" AS t1_r4, "company_users"."acl" AS t1_r5, "company_users"."created_at" AS t1_r6, "company_users"."updated_at" AS t1_r7 FROM "companies" LEFT OUTER JOIN "company_users" ON "company_users"."company_id" = "companies"."id" WHERE (("company_users"."acl" = $1 AND "company_users"."user_id" = $2) OR ("company_users"."acl" = $1 AND "company_users"."user_id" = $2))
=> #<Company::ActiveRecord_Relation:0x3fc296249300>

Using SQLite3 and cancancan (SQLite 1.3.10):

[1] pry(main)> Company.accessible_by(User.first.ability)
  User Load (0.1ms)  SELECT  "users".* FROM "users"  ORDER BY "users"."id" ASC LIMIT 1
  SQL (0.2ms)  SELECT "companies"."id" AS t0_r0, "companies"."name" AS t0_r1, "companies"."created_at" AS t0_r2, "companies"."updated_at" AS t0_r3, "company_users"."id" AS t1_r0, "company_users"."company_id" AS t1_r1, "company_users"."user_id" AS t1_r2, "company_users"."effective" AS t1_r3, "company_users"."expire" AS t1_r4, "company_users"."acl" AS t1_r5, "company_users"."created_at" AS t1_r6, "company_users"."updated_at" AS t1_r7 FROM "companies" LEFT OUTER JOIN "company_users" ON "company_users"."company_id" = "companies"."id" WHERE (("company_users"."acl" = 'manager' AND "company_users"."user_id" = 1) OR ("company_users"."acl" = 'admin' AND "company_users"."user_id" = 1))
=> [#<Company:0x007f848bf9e888 id: 1, name: "Company 1", created_at: Fri, 09 Jan 2015 06:25:39 UTC +00:00, updated_at: Fri, 09 Jan 2015 06:25:39 UTC +00:00>]

When my ability.rb file is defined as:

    can :admin_test, Company, company_users: { acl: "admin", user_id: user.id }
    can :manager_test, Company, company_users: { acl: "manager", user_id: user.id }

cancancan works as expected with either database:

[1] pry(main)> Company.accessible_by(User.first.ability, :admin_test)
  User Load (3.1ms)  SELECT  "users".* FROM "users"  ORDER BY "users"."id" ASC LIMIT 1
  SQL (2.5ms)  SELECT "companies"."id" AS t0_r0, "companies"."name" AS t0_r1, "companies"."created_at" AS t0_r2, "companies"."updated_at" AS t0_r3, "company_users"."id" AS t1_r0, "company_users"."company_id" AS t1_r1, "company_users"."user_id" AS t1_r2, "company_users"."effective" AS t1_r3, "company_users"."expire" AS t1_r4, "company_users"."acl" AS t1_r5, "company_users"."created_at" AS t1_r6, "company_users"."updated_at" AS t1_r7 FROM "companies" LEFT OUTER JOIN "company_users" ON "company_users"."company_id" = "companies"."id" WHERE "company_users"."acl" = $1 AND "company_users"."user_id" = $2  [["acl", "admin"], ["user_id", 1]]
=> [#<Company:0x007fd62d395a20 id: 1, name: "Company 1", created_at: Fri, 09 Jan 2015 04:43:34 UTC +00:00, updated_at: Fri, 09 Jan 2015 04:43:34 UTC +00:00>]
@arambert
Contributor
arambert commented Jan 9, 2015

👍 same issue here

  PG::ProtocolViolation: ERROR:  bind message supplies 0 parameters, but prepared statement "" requires 1

In my case the user_id is not passed to the request:

  [...] WHERE (("sequences"."user_id" = $1 AND "sequences"."state" = 'started') OR ("users"."id" = $1 AND "sequences"."state" = 'started'))
@bryanrite
Member

Must be a bug with the way we've replaced the deprecated sanitize_sql_hash_for_conditions. I'll take a look at it asap.

@arambert
Contributor

Yes it must be related to how you've replaced sanitize_sql_hash_for_conditions : when I comment the part added for 4.2 (if ActiveRecord::VERSION::MINOR >= 2 && Hash === conditions [...][...] end ) it works again (and the deprecation notices rise again).

@arambert
Contributor

I've replaced the faulty part in https://github.com/CanCanCommunity/cancancan/blob/develop/lib/cancan/model_adapters/active_record_4_adapter.rb with :

def sanitize_sql(conditions)
    if ActiveRecord::VERSION::MINOR >= 2 && Hash === conditions
      table = Arel::Table.new(@model_class.send(:table_name))

      conditions = ActiveRecord::PredicateBuilder.resolve_column_aliases @model_class, conditions
      conditions = @model_class.send(:expand_hash_conditions_for_aggregates, conditions)

      ActiveRecord::PredicateBuilder.build_from_hash(@model_class, conditions, table).map { |b|
        @model_class.send(:connection).visitor.compile b
      }.join(' AND ')
    else
      @model_class.send(:sanitize_sql, conditions)
    end
  end

I've reused sanitize_sql_hash_for_conditions in https://github.com/rails/rails/blob/4-2-stable/activerecord/lib/active_record/sanitization.rb

It seems to work fine.
Do you think it's worth a PR ?

@bryanrite
Member

Yes please!

@JustinAiken

Can confirm; upgrading from cancancan 1.9.2 to 1.10.0 replaced my DEPRECATION WARNING: sanitize_sql_hash_for_conditions is deprecated, and will be removed in Rails 5.0 warning on a specific controller test with a whole nasty bug:

Failure/Error: get :index
     ActiveRecord::StatementInvalid:
       PG::ProtocolViolation: ERROR:  bind message supplies 0 parameters, but prepared statement "" requires 1
       : SELECT COUNT(*) FROM "ramp_tests" WHERE ("ramp_tests"."user_id" = $1)
     # /Users/jaiken/.rvm/gems/ruby-2.1.5/gems/activerecord-4.2.0/lib/active_record/connection_adapters/postgresql_adapter.rb:592:in `async_exec'

Using @arambert 's branch from #168 works flawlessy 👍

@bryanrite
Member

Thanks @JustinAiken

I'll get them merged tonight.

@bryanrite bryanrite closed this in f2ed2a1 Jan 13, 2015
@fattymiller

Thanks everyone, this has worked a treat!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment