Search code examples
phpmysqlsqljquery-jtable

MySQL, PHP passing ORDER BY parameter


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.


Solution

  • 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.