On Wed, Nov 4, 2009 at 9:13 AM, Matteo Vaccari <[email protected]> wrote: > > It all began when a collegue said that "our free text search throws if the > user enters something with a dot". A bit of investigation showed that > ActiveRecord produced a different set of queries depending on the user input > containing a dot or not. The :include is handled with a single query with a > join, or with two separate queries. > > This is not a big problem in most cases; it only has a performance impact. > In our case there was a failure because the :include joins a table which > another scope joins for a different reason. We would never have noticed > this otherwise. > > I dug a bit, and found that the reason is that method > ActiveRecord::Base.tables_in_string treats anything before a dot like a > table name. So a condition like > > ["x = ?", params[:input]] > > can produce a query "x = 'foo.bar'", and tables_in_string reports that we > reference a table named "foo". > > It's not a big problem for our application; the simple workaround for me was > to filter out dots in user input. But I think it's not good that AR can > behave differently according to user input. So I filed a ticket and > prepared a patch. > > https://rails.lighthouseapp.com/projects/8994/tickets/3446-tables_in_string-matches-literal-string-containing-dot#ticket-3446-2
Attempting to parse SQL using regular expressions may work for simple cases, but not more complex ones. The following will still fail with your patch: irb(main):001:0> puts "name = 'foo\\\\' AND 'foo.bar ' = number" name = 'foo\\' AND 'foo.bar ' = number irb(main):002:0> puts "name = 'foo\\\\' AND 'foo.bar ' = number".gsub(/'(\\'|[^'])*'|"(\\"|[^"])*"/, '') name = foo.bar ' = number irb(main):003:0> "name = 'foo\\\\' AND 'foo.bar ' = number".gsub(/'(\\'|[^'])*'|"(\\"|[^"])*"/, '').scan(/([a-zA-Z_][\.\w]+).?\./).flatten => ["foo"] To be fair, ActiveRecord attempts to parse SQL using regular expressions all over the place, and your patch handles the most common current failure cases, so it's still probably a good candidate for inclusion. Jeremy --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
