Search code examples
ruby-on-railsrubyoracleruby-on-rails-2oracle-call-interface

rails query generates oci:error quoted string incorrectly terminated


I have the following code(very legacy) in the application:

  results << User.all(
  :select => "'#{entry.name}' AS user_name, '#{entry.lastname}, #{entry.firstname}' AS user_full_name, display_name", 
  :order => "display_name")

Which generates the following query I get the following oci error:

  OCIError: ORA-01756: quoted string not properly terminated: 
SELECT 'theupsstore2579' AS user_name, 'O'Brien, Perry' AS user_full_name, display_name FROM "Users" WHERE..

Is there a good way to fix my query?

Is using quote_string a good way to do it


Solution

  • Yes there is a good way. You should not ever use interpolated strings eg "string bit #{interpolated_bit}" in a sql query - this is very bad for security and leads to SQL injection attacks.

    In this case - because one of the names includes a ' character (in the last-name O'Brien - and once that value is inserted into your SQL string, it treats it like it would any ' character and ends the string - even though there is more after the ' (namely the Brien bit)

    That then causes a syntax error for your database.

    However - using the interpolated strings this way also leaves you open to SQL injection - what if somebody typed in '; DROP TABLE users; (or equivalent) into their last name field? and your code happily put that into the SQL and ran it? Your way of using interpolated strings is NOT safe. But Rails provides alternatives that are safe - and they are what you should always use.

    eg the built-in arel methods (where select order etc) and use the sanitised ? syntax so instead of

    results << User.all(
      :select => "'#{entry.name}' AS user_name, '#{entry.lastname}, #{entry.firstname}' AS user_full_name, display_name", 
      :order => "display_name")
    

    you could try

    results << User.select("'?' AS user_name, '?, ?' AS user_full_name, display_name", entry.name, entry.lastname, entry.firstname).order("display_name")
    

    (though I question why you want to force all users to have the same name - did you actually want these to be conditions selecting only users with those names instead?)

    I strongly recommend you read through all of the Rails Guides. In this case especially the one on how to use Active Record queries:

    and also you might want to read up the security guide - specifically, in this case, the Rails guide SQL injection section