22

I have a table companies, which has two columns named name and address. By running the following code, new data are inserted into the table:

my_name = "my company name"
my_address = "ABC"

query = "INSERT INTO companies (name,address) VALUES ('#{my_name}','#{my_address}');"

ActiveRecord::Base.connection.execute(query);

If I change my_name value from "my company name" to "John's company", I will get a syntax error. This is because the query becomes:

"INSERT INTO companies (name,address) VALUES ('John's company','ABC');"

and 'John's company' has a single quotation mark within it.

Given that I have already used double quotation mark for the query string definition, how can I get rid of this error regarding the single quotation mark in my value?

sawa
  • 165,429
  • 45
  • 277
  • 381
Leem.fin
  • 40,781
  • 83
  • 202
  • 354
  • if my_name.include?('\'') my_name.sub(/'/,'\'') ? – Leem.fin Nov 18 '11 at 08:52
  • 5
    This is a really bad idea... you risk SQL injection attacks even if you escape single quotes with backslashes. You need to understand character encodings, but there is a serious (known) exploit to this naive approach. Use bind parameters in your query and allow the DBMS to transport the values safely. – d11wtq Nov 18 '11 at 10:28

1 Answers1

82

If you must do it this way then use the quote method on the connection object:

quote(value, column = nil)
Quotes the column value to help prevent SQL injection attacks.

So something like this:

my_name    = ActiveRecord::Base.connection.quote("John O'Neil")
my_address = ActiveRecord::Base.connection.quote("R'lyeh")

query = "INSERT INTO companies (name,address) VALUES (#{my_name}, #{my_address})"

ActiveRecord::Base.connection.execute(query);

Never ever try to handle your own quoting. And don't try to use double quotes for quoting an SQL string literal, that's what single quotes are for; double quotes are for quoting identifiers (such as table and column names) in most databases but MySQL uses backticks for that.

mu is too short
  • 426,620
  • 70
  • 833
  • 800
  • Since I am using variable for my_name, I guess I can use my_name = ActiveRecord::Base.connection.quote(NAME_VARIABLE) – Leem.fin Nov 18 '11 at 09:09
  • @Leem.fin: Right. Or you could put it right inside the string interpolation but that would make your code harder to read. – mu is too short Nov 18 '11 at 09:13
  • 1
    I think there is an error in your code, you should remove the single quotation mark in the query VALUES part, since the quote() method has already added quotation mark for the string. Now if I run the code like you said, I got SQL syntax error since I have ' 'John's company' ' as the value – Leem.fin Nov 18 '11 at 09:20
  • Doesn't active record support bind values? This sort of string escaping feels like more effort than it needs to be (and potentially riskier, if the escaping algorithm isn't too smart). PHP granted, but there's an old discuss on the risks of simple backslash escaping here, including a demonstration http://shiflett.org/blog/2006/jan/addslashes-versus-mysql-real-escape-string (it relates to character encoding hacks). Usually when you use bind parameters, the DBMS transports the values for you, without interpolating them into the string. – d11wtq Nov 18 '11 at 10:33
  • Looking at the source of `#quote_string`, it looks vulnerable to this attack too. Depends on the character set you're using. – d11wtq Nov 18 '11 at 10:37
  • 1
    Checkout this post as well for alternatives to what you want to do + how to sanitize your params http://stackoverflow.com/questions/4483049/how-to-execute-a-raw-update-sql-with-dynamic-binding-in-rails – Moiz Raja Nov 18 '11 at 11:18
  • @d11wtq: I couldn't find any prepared statement support last time I looked but the AR documentation is, um, a little thin to be charitable. Rails often has more attitude than good sense, their way or the highway and all that adolescent trash. If the `quote` implementation is vulnerable then AR itself is vulnerable, AFAIK Rails doesn't use prepared statements and bound parameters internally (but perhaps they have fixed this since I last looked). – mu is too short Nov 18 '11 at 17:00
  • @user420504: That's definitely worth mentioning. I'm not sure how standardized the raw interfaces are but that might not matter. – mu is too short Nov 18 '11 at 17:35
  • @lynks: Lovecraft always seems appropriate when talking about such things, never call up what you can't put down and all that. – mu is too short Aug 13 '12 at 17:24
  • Is it possible to use this methodology when querying objects other than strings? I want to submit an SQL query containing an integer and two datetime objects. – Argus9 Aug 01 '13 at 21:24
  • @Argus9: Have you tried seeing what `ActiveRecord::Base.connection.quote` does with `Fixnum` and `Time` objects in the console? It should do The Right Thing. – mu is too short Aug 01 '13 at 21:34
  • Yeah, I took a look in my debugger console and see that it is, in fact, returning the proper variables. Thanks! – Argus9 Aug 01 '13 at 21:47