grabs / moodle-mod_unilabel

1 stars 10 forks source link

it is better to use the table name in the api calls instead of a constant #5

Closed danmarsden closed 5 years ago

danmarsden commented 5 years ago

code that "obscures" the table name being used - eg: $DB->delete_records($this->get_namespace() make it hard to review. I'm not quite sure of the benefits you are getting by using the namespace in that field instead of specifying the actual table name? - in particular this is an issue for reviewers because the "table name" field is a vector for sql injections. Ideally it is preferred to be able to look at a SQL/db function call and see that it is not vulnerable to sql injections without having to go looking through code to see how the variable is created/used.

danmarsden commented 5 years ago

I don't think we'd block approval for this issue.

grabs commented 5 years ago

Thank for your hint. I replaced these calls with the proper string.