I ran the Coder module on Station and fixed a bunch of inconsistencies, see the attached patch.

Used grep+sed for changes, and nothing seems broken, so it should be safe.

In addition, the following files are missing CVS tags:

station/archive/scripts/ripper.inc
station/archive/scripts/ripper.phpstation/archive/station_archive.admin.inc
station/archive/station_archive.legacy_pages.inc
station/catalog/station_catalog.pages.inc
station/archive/scripts/ripper.inc
station/archive/scripts/ripper.phpstation/archive/station_archive.admin.inc
station/archive/station_archive.legacy_pages.inc
station/catalog/station_catalog.pages.inc
station/playlist/station_playlist.module

Also, it gives the following complaints:

station/catalog/station_catalog.pages.inc
Line 42: In SQL strings, Use db_query() placeholders in place of variables. This is a potential source of SQL injection attacks when the variable can come from user data.
    $result = pager_query(db_rewrite_sql("SELECT n.nid, sc.* FROM {node} n INNER JOIN {station_catalog} sc ON n.nid = sc.nid WHERE LOWER(sc.$field) LIKE LOWER('%%%s%%')") . tablesort_sql($header), $perpage, 0, NULL, $value);
station/playlist/station_playlist.module
Line 94: In SQL strings, Use db_query() placeholders in place of variables. This is a potential source of SQL injection attacks when the variable can come from user data.
        $result = db_query(db_rewrite_sql("SELECT COUNT(n.nid) FROM {node} n INNER JOIN {{$db_info['table']}} sp ON n.nid = sp.$col WHERE sp.$col = %d AND n.status = 1"), $object->nid);

Cheers!

CommentFileSizeAuthor
station.patch48.89 KBtim.plunkett

Comments

drewish’s picture

drewish commented
Status: Needs review » Needs work

Noticed the change to station_playlist_nodeapi():

-    $result = db_query_range('SELECT DISTINCT %s AS val FROM {station_playlist_track} WHERE LOWER(%s) LIKE LOWER("%s%%") ORDER BY val', $field, $field, $string, 0, 10);
+    $result = db_query_range("SELECT DISTINCT '%s' AS val FROM {station_playlist_track} WHERE LOWER('%s') LIKE LOWER('%s%%') ORDER BY val", $field, $field, $string, 0, 10);

I'm not sure this is correct... or at least that it might be using the wrong quoting mechanism. Doesn't mysql have one for data values and one for fieldnames?

Went ahead committed everything but station_playlist.module and the *.views_defaults.inc changes. The views defaults are generated as exports from views and I want to have to do a bunch of manual editing after exporting to avoid huge diffs.

If you want to look at the remainder and re-roll I'd love to see it.

drewish’s picture

drewish commented

I think that SQL issue was basically the same one you'd identified elsewhere in the module. I'm not sure what the best fix is but we need to document that we're doing wacky stuff.

drewish’s picture

drewish commented
Status: Needs work » Fixed

I'm trying to clean things up in the queue and this is basically finished.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.