[ postfixadmin-Patches-3152352 ] sorting capabilities

[SourceForge.net <no...@so...>]

Patches item #3152352, was opened at 2011-01-06 01:59
Message generated for change (Comment added) made by gingerdog
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=937966&aid=3152352&group_id=191583

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: None
Group: None
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: Martin Jaros (piszta)
Assigned to: Nobody/Anonymous (nobody)
Summary: sorting capabilities

Initial Comment:
it allows to sort records by any column. it is used in virtual list only just now, but another sections are supported too.

changes:
 - functions.inc.php: added function get_sort()
 - list-virtual.php: it uses session for limit (paging) and calls get_sort()
 - templates/list-virtual_mailbox.tpl, templates/list-virtual_alias.tpl: table headers became links


----------------------------------------------------------------------

>Comment By: GingerDog (gingerdog)
Date: 2012-04-19 13:33

Message:
Hi - 

Thanks for taking the time to submit this patch.

Some feedback.

1. It's vulnerable to SQL Injection - although you call safeget &
escape_string on the data from $_GET, what you're embedding within the SQL
query is not enclosed within quote marks (i.e. you're doing the equivalent
of " ...... FROM foo ORDER BY {$_GET['key']} ASC"

I think the best way to solve this is to change the code so that there is a
known 'good' list of fields which you are allowed to do a sort by on - and
not accept any input from the end user.

2. I'm not overly keen on seeing @ within code; I'd rather it was checked
with isset or something instead.

3. I don't like the way the order will flip on each page load if
$_GET['sort'] is present. This will be painful if you're on e.g. page 5 of
15, and trying to advance through the records.

I've pasted in what seems a better get_sort() function below - no doubt SF
will screw up my indentation ... 

thanks
David.



/**
 * This attempts to persist a sort order on records when viewing in a
list.
 * See
https://sourceforge.net/tracker/?func=detail&aid=3152352&group_id=191583&atid=937966
 *
 * @param string $default_column - the default column if neither
$_GET['sort'] or $_POST['sort'] are set. Column MUST be one of the ones
defined in $allowed_columns
 * @param string $default_dir - ASC or DESC
 * @return string some SQL (e.g. 'foobar ASC')
 */
function get_sort($default_column, $default_dir = 'ASC') {

    $allowed_columns = array('address', 'goto', 'modified', 'active');
    $allowed_dirs = array('ASC', 'DESC');

    if(!in_array($default_dir, $allowed_dirs)) {
        die("Invalid sort direction");
    }

    $fSort = false;
    if (isset ($_GET['sort'])) {
        if(in_array($_GET['sort'], $allowed_columns)) {
            $fSort = safeget('sort');
        }
    }
    if (isset ($_POST['sort'])) {
        if(in_array($_POST['sort'], $allowed_columns)) {
            $fSort = safepost('sort');
        }
    }

    // if nothing in $_GET/$_POST; see if we have anything in $_SESSION.
    if(false == $fSort) {
        if (isset($_SESSION['sort'])) {
            $fSort = $_SESSION['sort'];
            $fSortDir = $_SESSION['sort_dir'];
        } else {
            // No? oh, well, let's default to whatever was passed in
            $fSort = $default_column;
            $fSortDir = 'ASC';
        }
    } else {
        // we had something in $_GET/$_POST; store it in $_SESSION for
later on.
        $_SESSION['sort_dir'] = $fSortdir = 'ASC';
        $_SESSION['sort'] = $fSort;
    }
    return $fSort . ' ' . $fSortDir;
}

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=937966&aid=3152352&group_id=191583