Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AR SQL escaping issue #4037

Closed
joshk opened this issue Dec 19, 2011 · 9 comments
Closed

AR SQL escaping issue #4037

joshk opened this issue Dec 19, 2011 · 9 comments
Assignees

Comments

@joshk
Copy link
Contributor

joshk commented Dec 19, 2011

Hey Guys,

Using the following code:

Repository.where("repositories.name ~* ? OR repositories.owner_name ~* ?", query, query)

If the query is: josh/
it is outputted as 'josh/'
which escapes the ' and causes:

ActiveRecord::StatementInvalid: PGError: ERROR: invalid regular expression: invalid escape \ sequence : SELECT "repositories".* FROM "repositories" WHERE ("repositories"."last_build_started_at" IS NOT NULL) AND (repositories.name ~* 'cap\' OR repositories

Thanks,

Josh

@ghost ghost assigned tenderlove Dec 19, 2011
@tenderlove
Copy link
Member

This is a problem with escaping. Normal PG escape will not change the backslash since it has no meaning. Unfortunately using the string in terms of a regexp gives the backslash meaning. This patch will demo the error:

diff --git a/activerecord/test/cases/adapters/postgresql/quoting_test.rb b/activerecord/test/cases/adapters/postgresql/quoting_test.rb
index 172055f..a1df55e 100644
--- a/activerecord/test/cases/adapters/postgresql/quoting_test.rb
+++ b/activerecord/test/cases/adapters/postgresql/quoting_test.rb
@@ -1,4 +1,5 @@
 require "cases/helper"
+require 'models/author'

 module ActiveRecord
   module ConnectionAdapters
@@ -8,6 +9,12 @@ module ActiveRecord
           @conn = ActiveRecord::Base.connection
         end

+        def test_omg
+          string = 'ePoS\\'
+          Author.create!(:name => string)
+          assert_equal 1, Author.where("name ~* ? or name ~* ?", string, string).to_a.length
+        end
+
         def test_type_cast_true
           c = Column.new(nil, 1, 'boolean')
           assert_equal 't', @conn.type_cast(true, nil)

We should fix this by using prepared statements rather than string substitution for the question marks.

@isaacsanders
Copy link
Contributor

Any movement on this?

@Silex
Copy link

Silex commented Jun 4, 2012

+1, I am also affected. I guess we'd adapt the postgresql adapter so it also escapes \ right?

@Silex
Copy link

Silex commented Jun 11, 2012

@tenderlove: do you know of a workaround for this?

@voltechs
Copy link

/ is a forward slash, and since when is / an escape character?

@thiagofm
Copy link

@tenderlove do you know if this problem can be solved inside the postgres adapter or it should be fowarded to the pg gem?

@JonRowe
Copy link
Contributor

JonRowe commented Mar 13, 2013

@tenderlove has there been any work towards producing prepared statements instead of string substitutions as you suggested? Or could/should this be fixed by modifying how things are escaped?

@matthewd
Copy link
Member

This is notabug.

We correctly escape the given string when passing it to PG. The problem is that the ~* operator interprets meaning within that string's data: it treats the string as a regular expression.

A less confusing / slash-based example would be 'josh('.

As far as Rails is concerned, this is Just A String. Moreover, even if we knew it was being used as a regular expression, we couldn't apply the second level of escaping: given that you're setting out to use a regular expression operator, it doesn't seem unreasonable to assume you actually meant to write a regular expression. Consider the two possible interpretations of 'joshk?'.

In this particular case, you can use the '***=' director prefix; for a more complex example where you actually intend some characters to have normal regex interpretation, you'll need to specifically escape all the (regexp-) significant characters in the portion of your input intended to match as plain text:

my_re = "^(matthewd?|#{search.gsub(/\W/, '\\\\\&')})"

(or use Regexp.escape... but it'd be worth checking it will escape all the characters that are significant in PostgreSQL AREs)


See also:

query = 'josh\\'
'bob' =~ /#{query}/ # => ArgumentError: too short escape sequence

@rafaelfranca
Copy link
Member

Thank you @matthewd for the detailed comment. I agree it is not a bug so I'm closing. Feel free to continue the discussion here if needed.

thibaudgg added a commit to acp-admin/acp-admin that referenced this issue Oct 20, 2020
This patch ensures that member emails are well regexp escaped before
to be inserted in the `including_email` scope query.

See rails/rails#4037 for more info.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants