MySQL, PHP passing ORDER BY parameter

Question

I am using jTable and am attempting to sort records.

Currently the 'jtSorting' jTable variable is being passed correctly to my PHP script via the GET variable. I am calling a MySQL stored procedure like so:

$res = $mysqli->query("CALL getReadReportsPaging($startIndex, $pageSize, '$sorting');");

The query returns records and does not give an error, but does not return the records sorted by specified field. Records always return by Primary ID ASC.

Here is the Stored Procedure:

DELIMITER $$

CREATE DEFINER=`root`@`localhost` PROCEDURE `getReadReportsPaging`(startIndex INTEGER, pageSize INTEGER, sorting VARCHAR(255))
BEGIN
    SELECT * FROM reports 
    WHERE `new`=0
    ORDER BY sorting
    LIMIT startIndex, pageSize;
END

I tried calling the Procedure and passing in a column name with ASC or DESC like so:

call getReadReportsPaging(
    0,
    10,
    'ReportingName DESC'
);

Which also returns records with Primary ID ASC.

Any assistance appreciated.

I was doing that due to [this](http://stackoverflow.com/questions/1346209/unknown-column-in-field-list-error-on-mysql-update-query) reason. And [this](http://stackoverflow.com/questions/24293054/jtable-jquery-plugin-why-is-my-mysql-stored-procedure-failing/24312288#24312288). — andrsnn, Jun 19 '14 at 17:37
Also MySQL throws an error when not passed in with single quotes? — andrsnn, Jun 19 '14 at 17:39
I'm wondering why go through the effort of using a stored procedure for something that is a really simple query in php. — VikingBlooded, Jun 19 '14 at 17:41
I agree with VikingBlooded. Unless this is not the whole stored procedure, it would be much simpler to build the string in PHP (using prepared statements, however, for user input). For reusability/maintainability you could even wrap the building of the string in a PHP function. Maybe not completely orthodox, but it'd be much easier for sure. — 11684, Jun 19 '14 at 17:43
I'm not sure what you mean with 'dynamic SQL query'. _I_ mean building the query by doing string concatenation. — 11684, Jun 19 '14 at 17:45
Also could you explain why it would be easier? Calling a Stored Procedure and passing in parameters seems alot easier. — andrsnn, Jun 19 '14 at 17:47
You have two options : building the query in PHP using prepared statement or you pass in the ORDER BY clause to the stored procedure, build up the SQL statement in a string and then execute this statement using EXEC or sp_ExecuteSql. Have a look at [this](http://www.4guysfromrolla.com/webtech/010704-1.shtml) for the second option. — ForguesR, Jun 19 '14 at 17:47
Okay this seems to be the closest answer to my question. Is there a reason why I cannot call a static procedure multiple times for results like I am trying to above? — andrsnn, Jun 19 '14 at 17:55
You can't because you are passing a string to the query and the query see a string not a column name. That's why I say you are trying to do a dynamic query, — ForguesR, Jun 19 '14 at 17:59
Ah I see. That explains why you pass a string to MySQL in quotes. Thank you! — andrsnn, Jun 19 '14 at 18:06
When using `mysqli` you should be using parameterized queries and [`bind_param`](http://php.net/manual/en/mysqli-stmt.bind-param.php) to add user data to your query. **DO NOT** use string interpolation to accomplish this because you will create severe [SQL injection bugs](http://bobby-tables.com/). — tadman, Jun 19 '14 at 18:24

11684 · Accepted Answer · 2014-06-19T18:38:25.163

I think this:

function buildQuery($startIndex, $pageSize, $sorting) {
    return "SELECT * FROM reports WHERE `new`=0 ORDER BY $sorting LIMIT $startIndex, $pageSize";
}

$res = $mysqli->query(buildQuery($startIndex, $pageSize, $sorting));

Is a lot easier than defining:

DELIMITER $$

CREATE DEFINER=`root`@`localhost` PROCEDURE `getReadReportsPaging`(startIndex INTEGER, pageSize INTEGER, sorting VARCHAR(255))
BEGIN
    SET @Query = "SELECT * FROM reports WHERE `new`=0 ORDER BY ";
    @Query = CONCAT(@Query, sorting);
    @Query = CONCAT(@Query, " LIMIT ");
    @Query = CONCAT(@Query, startindex);
    @Query = CONCAT(@Query, ", ");
    @Query = CONCAT(@Query, pageSize);
    PREPARE stmt FROM @Query;
    EXECUTE stmt;
END

and then calling:

$mysqli->query("CALL getReadReportsPaging($startIndex, $pageSize, '$sorting');");

I'd go with the first. Shorter and more readable.

Edit:
If the user can set $pageSize, $sorting or $startIndex, the methods I outlined are very unsafe. I don't know how to sanitise them in mySQL, but I can do that in PHP. In PHP I'd do this with prepared statements, but you want to use a variable for sorting. That is a column name, and you can't use prepared statements parameters for column names. Actually, you can only use them for values. So I suggest you do obtain the column name to sort on and ASC|DESC separately, (or just split on the first space) and then do something like this:

$columns = array(/* the column names of the table `reports` as strings */);

if (!in_array($sortingColumn, $columns) {
    $sortingColumn = /* insert default sorting column name here */;
}

if ($sortingDirection != "ASC" && $sortingDirection != "DESC") { // I may be forgetting a sorting direction here.
    $sortingDirection = /* insert default sorting direction here */;
}

$stmt = $mysqli->prepare("SELECT * FROM reports WHERE `new`=0  ORDER BY $sortingColumn $sortingDirection LIMIT ?, ?");
$stmt->bindParam("ii", intval($startIndex), intval($pageSize)); // If the input to intval cannot be parsed as int it will return 0. This prevents injection in $startIndex and $pageSize.

$stmt->execute();

The above snippet of PHP code can be wrapped into a function as well. (Which could even return the $stmt without calling $stmt->execute().)

To keep the mySQL way safe, you would need to somehow perform the same checks in mySQL (which I think is somewhat impractical). But here goes:

DELIMITER $$

CREATE DEFINER=`root`@`localhost` PROCEDURE `getReadReportsPaging`(startIndex INTEGER, pageSize INTEGER, sortingDirection VARCHAR(255), sortingColumn VARCHAR(255))
    BEGIN
        SET @Query = "SELECT * FROM reports WHERE `new`=0 ORDER BY ";
        IF NOT sortingColumn IN (/* list of column names here */) THEN
            sortingColumn = /* default sorting column */;
        @Query = CONCAT(@Query, sortingColumn);
        @Query = CONCAT(@Query, " ");
        IF NOT sortingDirection IN ('ASC', 'DESC' /* I may be forgetting some directions) THEN
            sortingDirection = /* default sorting direction */;
        @Query = CONCAT(@Query, sortingDirection);
        @Query = CONCAT(@Query, " LIMIT ");
        @Query = CONCAT(@Query, CAST(startindex AS SIGNED)); // I assume this will return an int even if the input is not parseable as int.
        @Query = CONCAT(@Query, ", ");
        @Query = CONCAT(@Query, CAST(pageSize AS SIGNED));
        PREPARE stmt FROM @Query;
        EXECUTE stmt;
    END

Disclaimer: 1) I am not a security expert and 2) I did not test all the above code. I hope it will help you anyway.

Makes sense it is alot easier but does it open up any possibility for injection? Basically I am asking now are both ways equally secure or is one more favorable then the other? — andrsnn, Jun 19 '14 at 18:01
@user2210274 Depends. If one (or more) of the variables you pass in (either startIndex, pageSize or sorting) is input from the user, yes. (Even if it is from a select or checkbox. The user does not need the page with the form to send a request.) I'll edit my answer. — 11684, Jun 19 '14 at 18:03
@user2210274 The two ways I outlined are equally vulnerable to SQL injection. — 11684, Jun 19 '14 at 18:04
Okay thank you! Ill have to read more about injection, I've always been told the latter is more secure. — andrsnn, Jun 19 '14 at 18:08
@user2210274 Your SQL way is more secure than my PHP way (had the SQL way worked). But my SQL way is just as secure as my PHP way (because it is basically the same method, but implemented in two languages). — 11684, Jun 19 '14 at 18:09

MySQL, PHP passing ORDER BY parameter

1 Answers1