Open cschwenz opened 7 years ago
The plugin indeed catches "Update my_table failed. ($DBI::errstr)"
as a potential risk due to the use of update
as the leading word in this string, the double-quoting, and the use of an interpolated variable.
For the plugin to understand that it's not a SQL statement, the solution would be to determine if the token matched is in fact argument in a function call and what function calls are considered safe. The simplest I think would be to check for the parent of the current token in https://github.com/guillaumeaubert/Perl-Critic-Policy-ValuesAndExpressions-PreventSQLInjection/blob/master/lib/Perl/Critic/Policy/ValuesAndExpressions/PreventSQLInjection.pm#L636, and return early if the parent token is a function call listed in a configurable whitelist.
I would be happy to take a pull request if you're interested in implementing this, it's a good feature idea. Please note that all contributions to this module must be under the same terms as Perl 5 itself.
Thank you!
Perl::Critic::Policy::ValuesAndExpressions::PreventSQLInjection
should not trigger on either:or:
The attached test file (which has an additional
.txt
to get around GitHub brain-damage) will pass whenPerl::Critic::Policy::ValuesAndExpressions::PreventSQLInjection
correctly handles the above cases; said test file is released under the Artistic 2.0 license (https://opensource.org/licenses/Artistic-2.0).test_PreventSQLInjection_flagging_valid_constructs.t.txt