[Feature Proposal] Accept Nested Functions w.r.t. Dangerous Query Methods

Summary

I think there’s an opportunity to reduce additional false positives for Dangerous Query Method deprecations/errors, but am cognizant of the trade-off with not writing a sql parser as well as how long it’s existed as is.

Nested Functions

The first potential opportunity would be allowing nested functions. Similar to Allow column name with function (e.g. `length(title)`) as safe SQL string by kamipo · Pull Request #36448 · rails/ra, it seems reasonable to allow functions that accept other functions (e.g. length(trim(title))). As an example (though all DB adapters seem like they’d be able to be changed similarly), the change in activerecord/lib/active_record/connection_adapters/abstract/quoting.rb could look like:

-            ((?:\w+\.)?\w+) | \w+\((?:|\g<2>)\)
+            ((?:\w+\.)?\w+ | \w+\((?:|\g<2>)\))

Collation

Collation seems to me, less reasonable within the rails codebase, though I’m also happy to add this in a PR if I should. In that case, I might need help implementing this outside of the postgres adapter though. In the simplest case, this might look like adding

+        (?:\s+COLLATE\s+\w+)?

above the ASC/DESC portion of the regex.

Questions

  • Would either or both of these be things we’d want in rails, or are they too niche/are we happier leaving them as it’s been through multiple rails versions?
  • Whether or not we want them in rails, I’d appreciate concerns about either opening up injection vectors I’m not thinking about?

Background

Thanks for any thoughts!