Comment by inopinatus
Comment by inopinatus 6 months ago
Some good recommendations. Feedback on this one:
> "X41 recommends to disallow the creation of un-escaped SqlLiteral objects with user input in favor of a complete model of SQL"
Rails already has a sufficient model of SQL in its Arel layer. Complete? Not exactly, because SQL is never implemented by the standard, but certainly sufficient, and very composable and extensible. Sadly the core team killed off the public documentation of Arel a few majors ago. Nevertheless I still use Arel whenever Active Record doesn't expose enough of the model, such as expressing left-open intervals as inequalities¹. Sometimes incorrectly called a "private API", but it's not anyone's private possession, Arel is just undocumented in recent releases.
The recommendation's language is also just a touch naive, because it's nigh-impossible to outright disallow developers doing practically whatever they want to in Ruby, there's no isolated sandbox. The question is what incentives are in place to stop them wanting to.
Active Record already has excellent support for bound values via the predicate builder, and it's only egregiously bad code that concatenates raw user-supplied values directly into query strings. Nevertheless for those few remaining places in the API where this could happen inadvertently such as #calculate, a variant recommendation - similar in spirit, but not identical - might be that where it doesn't already, Active Record treats raw strings supplied as requiring escape unless explictly wrapped with Arel.sql(), or just accept an Arel node/AST (many already do on the QT). That is, force the developer to confess their sin in writing, or do it relationally.
But IMO the wizards shouldn't keep Arel locked up in the tower, either.
[1] https://gist.github.com/inopinatus/c84c78483b30fb2d5588db9be...
> The recommendation is also a touch naive as framed, because it's nigh-impossible to outright disallow developers doing practically whatever they want to in Ruby
Sure but defaults matter.
Nothing is truly private in most languages. In C/C++, you can poke raw memory. In rust you can transmute. In Java you can override the classloader and edit classes before they’re passed to the JVM and so on. But most environments have a golden path for getting things done which most developers will walk. This is exactly what removing documentation does - it signals to developers that some API isn’t part of the expected golden path.
Every sql library needs ways to pass raw sql strings to the database. (Just as every browser framework needs a way to embed raw html strings). But it should require you to explicitly acknowledge you’re doing something unsafe. Rust’s unsafe keyword. React’s unsafelySetInnerHTML, and so on. It’s not about denying it. It’s about making the obvious thing safe and the unsafe thing not obvious.