ryanb / cancan

Authorization Gem for Ruby on Rails.
MIT License
6.27k stars 783 forks source link

Deep conditions nesting on sqlite => stack overflow #414

Open clyfe opened 13 years ago

clyfe commented 13 years ago

CanCan constructs sql like so

... where cond0 OR (
  cond1 OR (
    cond2 ...
  )
)

this useless deep nesting on sqlite3 results in a Parser Stack Overflow error.

A better SQL would be:

... where cond0 OR cond1 OR cond2 ...

I propose we fix this in CanCan::ModelAdapters::ActiveRecordAdapter like so:

# fix nested imbrication
def merge_conditions(sql, conditions_hash, behavior)
  if conditions_hash.blank?
    behavior ? true_sql : false_sql
  else
    conditions = sanitize_sql(conditions_hash)
    case sql
    when true_sql
      behavior ? true_sql : "not (#{conditions})"
    when false_sql
      behavior ? conditions : false_sql
    else
      behavior ? "(#{conditions}) OR #{sql}" : "(not (#{conditions}) AND #{sql})" # fix here
    end
  end
end
ryanb commented 13 years ago

Thanks for reporting this and providing a fix. I'm marking this to be fixed.

xhoy commented 10 years ago

Dear submitter, Since cancan/raynB hasn't been active for more than 6 months and no body else then ryam himself has commit permissions the cancan project is on a stand still. Since cancan has several issues including missing support for rails 4 cancan is moving forward to cancancan. More details on: #994

If your feel that your pull request or bug is still applicable (and hasn't been merged in to cancan) it would be really appreciated if you would resubmit it to cancancan (https://github.com/cancancommunity/cancancan)

We hope to see you on the other side!