SQL Like Escaping as a method on String #14222

Closed
wants to merge 3 commits into from

5 participants

@D1plo1d

This would add the patch from #6104 (amended to include the \ escape character and || operator) to all Strings.

This does not have the backwards compatibility issues brought up in #6104 because it is a new helper method with no previous implementation to break.

IMO this should be a part of Rails so that the Rails team can iterate on a single sql injection protection implementation for like statements that covers edge cases that might not be immediately obvious.

The other option (the current option) is for developers such as myself to implement a potentially flawed DIY solution. Those edge cases (ex. db-specific special characters) are not going to be obvious at first but with many eyes they will be spotted.

@D1plo1d D1plo1d SQL Like Escaping as a method on String
This would add the patch from #6104 (amended to include the \ escape character and || operator) to all Strings.

This does not have the backwards compatibility issues brought up in #6104 because it is a new helper method with no previous implementation to break.

IMO this should be a part of Rails so that the Rails team can iterate on a single sql injection protection implementation for like statements that covers edge cases that might not be immediately obvious.

The other option (the current option) is for developers such as myself to implement a potentially flawed DIY solution. Those edge cases (ex. db-specific special characters) are not going to be obvious at first but with many eyes they will be spotted.
47c39c3
@senny
Ruby on Rails member

@D1plo1d thank you for your contribution.

I don't think we should broaden the String API for this very specific method. It's use case is extremely limited. In my opinion, such a helper could reside in ActiveRecord but extending String is not the way to go.

/cc @rafaelfranca @carlosantoniodasilva

@senny
Ruby on Rails member

@D1plo1d are you going to update the PR with the move suggested by @rafaelfranca ?

@D1plo1d

@senny Thanks for the reminder, I'd totally forgotten about this! I've updated the PR with @rafaelfranca 's suggestion.

@senny senny commented on an outdated diff Apr 15, 2014
activerecord/lib/active_record/sanitization.rb
@@ -107,6 +107,12 @@ def sanitize_sql_hash_for_assignment(attrs, table)
end.join(', ')
end
+ # Sanitizes a string so that it is safe to use within a sql
+ # like statement.
+ def sanitize_sql_like
@senny
Ruby on Rails member
senny added a note Apr 15, 2014

I don't think this is going to work... You need to take the string the sanitize as argument, no?

Please add a test-case to illustrate the use-case and make sure it actually works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@D1plo1d

I'm not sure how to test this so if someone else can handle that it would be very helpful. Sorry about the copy + paste though, I've fix the method now.

@senny senny added a commit that closed this pull request Apr 16, 2014
@D1plo1d D1plo1d SQL Like escaping helper method. [Rob Gilson & Yves Senn]
Closes #14222.

This is a follow up to #6104

This does not have the backwards compatibility issues brought up in
implementation to break.
fe4b0ee
@senny senny closed this in fe4b0ee Apr 16, 2014
@senny
Ruby on Rails member

@D1plo1d updated your patch with tests and docs.

@senny senny added a commit that referenced this pull request Apr 16, 2014
@senny senny `sanitize_sql_like` escapes `escape_character` not only backslash.
* This is a follow up to: fe4b0ee
* The originating PR is #14222
* It should fix the build
973a452
@D1plo1d

awesome, thanks @senny!

@felixbuenemann

❤️ Great to see this in rails!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment