pat/riddle — pull request #71 by epidemian (2013-07-22 02:41:58+00:00)
Title: SQL escaping for snippets

This is more or less what i had in mind for issue #63 (and #70).

I think trying to make `Riddle::Query.escape` do both MySQL escaping and SphinxQL escaping is a bit confusing, so i used another function for escaping the snippets text, which doesn't need to be SphinxQL escaped.

Now, i'm quite confused about Riddle's code at many points. Some things i'd like to understand better:

- What's the specific purpose of `Riddle::Query.escape`? In what ways does it differ from `Riddle.escape`?
- Why does `Riddle::Query.escape` [adds double backslashes](https://github.com/pat/riddle/blob/fceffb92ba3117c6120ccd0bb5d267371e7ec987/lib/riddle/query.rb#L103) for special SphinsQL characters? Shouldn't `string.gsub(/[\\\(\)\|\-!@~"&\/\^\$=]/) { |m| "\\#{m}" }` be enough (based on [PHP's API](http://code.google.com/p/sphinxsearch/source/browse/trunk/api/sphinxapi.php#1624))?
- Why does `Riddle::Query.escape` [remove all backslashes](https://github.com/pat/riddle/blob/master/spec/riddle/query_spec.rb#L91-L93) from its input?
- Why is there a separate `Riddle::Client#excerpts` function with its own separate specs? What's the difference between that and `Riddle::Query.snippets` and why does ThinkingSphinx use the later?

So, i hope it's clear that i don't have a good grasp of Riddle's inner workings. That being said, shouldn't Riddle escape all strings that come "from the outside" before using them as string is SQL queries? (both to avoid SQL syntax erros and potential SQL injections) And also apply SphinxQL escaping only to the strings that will be used in `MATCH` statements _before_ the SQL escaping, or maybe leave that to the user so they can decide if they want extended matching or not (note that if you `Mysql2::Client.escape(Riddle.escape('@'))` then you get that double backslash that allows Sphinx to read the `@` as a literal).