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

Add in-line comment fixture #209

Closed
wants to merge 1 commit into from

Conversation

thommahoney
Copy link

This commit adds a new fixture that demonstrates the expected behavior
using a query from the MySQL documentation page on comments:
https://dev.mysql.com/doc/refman/5.6/en/comments.html

This commit adds a new fixture that demonstrates the expected behavior
using a query from the MySQL documentation page on comments:
https://dev.mysql.com/doc/refman/5.6/en/comments.html
@drbrain
Copy link

drbrain commented May 22, 2015

In particular, we're making use of /*! MySQL-specific code */ to enable MySQL-specific query optimizations.

@benweint
Copy link
Contributor

Thanks for reporting this!

This is unfortunately a difficult problem to solve.

The short version is that in order to mask out literal values in SQL queries, we need to be able to identify where strings start and end. Without knowledge of what quoting mode is in use by the database server (for MySQL, specifically the value of the NO_BACKSLASH_ESCAPES setting), it turns out to be impossible to reliably determine where string boundaries are in the presence of comments (which may include embedded string delimiters).

To work around this, we've taken the conservative approach of masking all text after the first comment-initiator sequence that we see in any query. As you've seen, this over-obfuscates in some cases.

Do your optimization-enabling comments need to be inline within the query rather than at the end?

There are alternate obfuscation heuristics we could use, but all of them come with tradeoffs, so we're hesitant to optimize for the inline-comment case unless it's something that's widely used.

Note that we do support using a custom SQL obfuscation strategy by calling NewRelic::Agent::Database::Obfuscator.set_sql_obfuscator(...). This is a pretty old API and is not currently tagged as public, though it looks like it should be. This comment describes how to use it.

If you're certain that your inline comments will never contain embedded quote delimiters, you could probably achieve what you're after like this:

NewRelic::Agent::Database::Obfuscator.instance.set_sql_obfuscator(:before) do |sql|
  sql.gsub(/\/\*(?:[^\/]|\/[^*])*?(?:\*\/|\/\*.*)/, '?')
end

@kwugirl
Copy link
Contributor

kwugirl commented Aug 28, 2015

I'm going to close out this pull request since it seems there wasn't further discussion here, but do let us know if there's still a concern.

@kwugirl kwugirl closed this Aug 28, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants