Remove the check for SQL "from" and "join" from minaccept script

Bug #1417364 reported by Aaron Wells on 2015-02-03
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mahara
High
Aaron Wells
1.10
High
Unassigned
1.8
High
Unassigned
1.9
High
Unassigned

Bug Description

When you use Mahara's makefile to push a patch to reviews.mahara.org, it runs a minaccept script which checks for various simple errors. One of the things it includes is a check for the word "from" or "join" not followed by a curly bracket. The idea is that this is supposed to be a check for SQL queries that haven't properly escaped table names. In order to support the $cfg->dbprefix setting, raw SQL is supposed to surround table names with curly brackets, e.g. "select * from {view}".

The problem is, this check throws a LOT of false positives, because the word "from" is a common English word, often used in comments, function names, and variables. It also throws a false positive if you format a multi-line query so that "from" is on a line by itself (which I like to do). These false positives then lead people to ignore the rest of the output from the minaccept script.

Robert suggested that we should do this:

1. Drop the from/join check in minaccept

2. Update the Behat test so that it runs with a $cfg->dbprefix setting, so that running the Behat tests will help check for problems with this.

Aaron Wells (u-aaronw) on 2015-02-03
description: updated
Mahara Bot (dev-mahara) wrote :

Patch for "1.8_STABLE" branch: https://reviews.mahara.org/4246

Mahara Bot (dev-mahara) wrote :

Patch for "1.9_STABLE" branch: https://reviews.mahara.org/4247

Mahara Bot (dev-mahara) wrote :

Patch for "1.10_STABLE" branch: https://reviews.mahara.org/4248

Reviewed: https://reviews.mahara.org/4245
Committed: http://gitorious.org/mahara/mahara/commit/6f348572c4b7000680427937c2a0b6cc57d6ab8b
Submitter: Aaron Wells (<email address hidden>)
Branch: master

commit 6f348572c4b7000680427937c2a0b6cc57d6ab8b
Author: Aaron Wells <email address hidden>
Date: Tue Feb 3 16:45:39 2015 +1300

Removing the from/join check in minaccept

Bug 1417364: This check causes too many false positives and doesn't catch
actual problems often enough.

Change-Id: I659cafd3dcdbcf254d66a72bcb4f2f6a1ba2ddba

Mahara Bot (dev-mahara) wrote :

Reviewed: https://reviews.mahara.org/4246
Committed: http://gitorious.org/mahara/mahara/commit/756e3345d94196068370ca21ca8bb5ab0dfd073a
Submitter: Aaron Wells (<email address hidden>)
Branch: 1.8_STABLE

commit 756e3345d94196068370ca21ca8bb5ab0dfd073a
Author: Aaron Wells <email address hidden>
Date: Tue Feb 3 16:45:39 2015 +1300

Removing the from/join check in minaccept

Bug 1417364: This check causes too many false positives and doesn't catch
actual problems often enough.

Change-Id: I659cafd3dcdbcf254d66a72bcb4f2f6a1ba2ddba

Mahara Bot (dev-mahara) wrote :

Reviewed: https://reviews.mahara.org/4247
Committed: http://gitorious.org/mahara/mahara/commit/70c938dad2472e87a9d1818ec781fd26dbbf7059
Submitter: Aaron Wells (<email address hidden>)
Branch: 1.9_STABLE

commit 70c938dad2472e87a9d1818ec781fd26dbbf7059
Author: Aaron Wells <email address hidden>
Date: Tue Feb 3 16:45:39 2015 +1300

Removing the from/join check in minaccept

Bug 1417364: This check causes too many false positives and doesn't catch
actual problems often enough.

Change-Id: I659cafd3dcdbcf254d66a72bcb4f2f6a1ba2ddba

Mahara Bot (dev-mahara) wrote :

Reviewed: https://reviews.mahara.org/4248
Committed: http://gitorious.org/mahara/mahara/commit/41fcf0ebabfdd5ce172b67057f5c1e97126e1a1e
Submitter: Aaron Wells (<email address hidden>)
Branch: 1.10_STABLE

commit 41fcf0ebabfdd5ce172b67057f5c1e97126e1a1e
Author: Aaron Wells <email address hidden>
Date: Tue Feb 3 16:45:39 2015 +1300

Removing the from/join check in minaccept

Bug 1417364: This check causes too many false positives and doesn't catch
actual problems often enough.

Change-Id: I659cafd3dcdbcf254d66a72bcb4f2f6a1ba2ddba

Robert Lyon (robertl-9) on 2015-02-03
Changed in mahara:
status: In Progress → Fix Committed
milestone: none → 15.04.0
Aaron Wells (u-aaronw) on 2015-02-12
Changed in mahara:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers