Skip to content

Commit

Permalink
API Create orderBy() method to handle raw SQL
Browse files Browse the repository at this point in the history
  • Loading branch information
@emteknetnz
emteknetnz committed Dec 5, 2022
1 parent 7860e46 commit 81da517
Show file tree
Hide file tree
Showing 5 changed files with 253 additions and 54 deletions.
149 changes: 103 additions & 46 deletions src/ORM/DataList.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -203,10 +203,14 @@ public function sql(&$parameters = [])
/**
* Return a new DataList instance with a WHERE clause added to this list's query.
*
* This method accepts raw SQL so could be vulnerable to SQL injection attacks if used incorrectly,
* it's preferable to use filter() instead which does not allow raw SQL.
*
* Supports parameterised queries.
* See SQLSelect::addWhere() for syntax examples, although DataList
* won't expand multiple method arguments as SQLSelect does.
*
*
* @param string|array|SQLConditionGroup $filter Predicate(s) to set, as escaped SQL statements or
* paramaterised queries
* @return static
Expand All @@ -222,6 +226,9 @@ public function where($filter)
* Return a new DataList instance with a WHERE clause added to this list's query.
* All conditions provided in the filter will be joined with an OR
*
* This method accepts raw SQL so could be vulnerable to SQL injection attacks if used incorrectly,
* it's preferable to use filterAny() instead which does not allow raw SQL
*
* Supports parameterised queries.
* See SQLSelect::addWhere() for syntax examples, although DataList
* won't expand multiple method arguments as SQLSelect does.
Expand All @@ -237,8 +244,6 @@ public function whereAny($filter)
});
}



/**
* Returns true if this DataList can be sorted by the given field.
*
Expand Down Expand Up @@ -308,72 +313,118 @@ public function distinct($value)
}

/**
* Return a new DataList instance as a copy of this data list with the sort
* order set.
* Return a new DataList instance as a copy of this data list with the sort order set
*
* @see SS_List::sort()
* @see SQLSelect::orderby
* Raw SQL is not accepted, only actual field names can be passed
*
* @param string|array $args
* @example $list = $list->sort('Name'); // default ASC sorting
* @example $list = $list->sort('Name DESC'); // DESC sorting
* @example $list = $list->sort('Name ASC, Age DESC');
* @example $list = $list->sort('Name', 'ASC');
* @example $list = $list->sort(array('Name'=>'ASC', 'Age'=>'DESC'));
*
* @param string|array Escaped SQL statement. If passed as array, all keys and values are assumed to be escaped.
* @return static
* @example $list = $list->sort(['Name' => 'ASC', 'Age' => 'DESC']);
* @example $list->sort(null); // wipe any existing sort
*/
public function sort()
public function sort(...$args): static
{
$count = func_num_args();

$count = count($args);
if ($count == 0) {
return $this;
}

if ($count > 2) {
throw new InvalidArgumentException('This method takes zero, one or two arguments');
}

if ($count == 2) {
$col = null;
$dir = null;
list($col, $dir) = func_get_args();

// Validate direction
if (!in_array(strtolower($dir ?? ''), ['desc', 'asc'])) {
user_error('Second argument to sort must be either ASC or DESC');
list($column, $direction) = $args;
$sort = [$column => $direction];
} else {
$sort = $args[0];
if (!is_string($sort) && !is_array($sort) && !is_null($sort)) {
throw new InvalidArgumentException('sort() arguments must either be a string, an array, or null');
}
if (is_null($sort)) {
// Set an an empty array here to cause any existing sort on the DataLists to be wiped
// later on in this method
$sort = [];
} elseif (empty($sort)) {
throw new InvalidArgumentException('Invalid sort paramater');
}
// If $sort is string then convert string to array to allow for validation
if (is_string($sort)) {
$newSort = [];
// Making the assumption here there are no commas in column names
// Other parts of silverstripe will break if there are commas in column names
foreach (explode(',', $sort) as $colDir) {
// Using regex instead of explode(' ') in case column name includes spaces
if (preg_match('/^(.+) ([^"]+)$/i', trim($colDir), $matches)) {
list($column, $direction) = [$matches[1], $matches[2]];
} else {
list($column, $direction) = [$colDir, 'ASC'];
}
$newSort[$column] = $direction;
}
$sort = $newSort;
}
}
// Validate that column names are not raw sql
if (is_array($sort)) {
foreach ($sort as $column => $direction) {
$this->validateColumnHasNoRawSql($column);
$this->validateSortDirection($direction);
}
}
return $this->alterDataQuery(function (DataQuery $query, DataList $list) use ($sort) {
// Wipe the sort
$query->sort(null, null);
foreach ($sort as $column => $direction) {
$list->applyRelation($column, $relationColumn, true);
$query->sort($relationColumn, $direction, false);
}
});
}

$sort = [$col => $dir];
} else {
$sort = func_get_arg(0);
private function validateColumnHasNoRawSql(string $column): void
{
// Remove quotes around the column name
$col = trim(trim($column), '"');
// Using regex because $this->canSortBy() failed on complex relations e.g. Team.Founder.Surname and
// also increased the number of db queries
// Search for non-alphanumeric characters in the column name rather than ^[^a-zA-Z0-9_]$ to account
// for column names that include macrons etc
if (preg_match('#[()[\]{}+\-*/=:?\\;|&\'"`<>^%,\s]#', $col)) {
$message = implode(' ', [
"Column name $column includes characters that could lead to SQL injection vulnerabilities.",
'Rename this column or use DataList::orderBy() for sorting instead.'
]);
throw new InvalidArgumentException($message);
}
}

return $this->alterDataQuery(function (DataQuery $query, DataList $list) use ($sort) {
private function validateSortDirection(string $direction): void
{
$dir = strtolower($direction);
if ($dir !== 'asc' && $dir !== 'desc') {
throw new InvalidArgumentException("Invalid sort direction $direction");
}
}

if (is_string($sort) && $sort) {
if (false !== stripos($sort ?? '', ' asc') || false !== stripos($sort ?? '', ' desc')) {
$query->sort($sort);
} else {
$list->applyRelation($sort, $column, true);
$query->sort($column, 'ASC');
}
} elseif (is_array($sort)) {
// sort(array('Name'=>'desc'));
$query->sort(null, null); // wipe the sort

foreach ($sort as $column => $direction) {
// Convert column expressions to SQL fragment, while still allowing the passing of raw SQL
// fragments.
$list->applyRelation($column, $relationColumn, true);
$query->sort($relationColumn, $direction, false);
}
}
/**
* Set an explicit ORDER BY statement using raw SQL
*
* This method accepts raw SQL so could be vulnerable to SQL injection attacks if used incorrectly,
* it's preferable to use sort() instead which does not allow raw SQL
*/
public function orderBy(string $orderBy): static
{
return $this->alterDataQuery(function (DataQuery $query) use ($orderBy) {
$query->sort($orderBy, null, true);
});
}

/**
* Return a copy of this list which only includes items with these characteristics
*
* Raw SQL is not accepted, only actual field names can be passed
*
* @see Filterable::filter()
*
* @example $list = $list->filter('Name', 'bob'); // only bob in the list
Expand Down Expand Up @@ -433,6 +484,8 @@ public function addFilter($filterArray)
/**
* Return a copy of this list which contains items matching any of these characteristics.
*
* Raw SQL is not accepted, only actual field names can be passed
*
* @example // only bob in the list
* $list = $list->filterAny('Name', 'bob');
* // SQL: WHERE "Name" = 'bob'
Expand Down Expand Up @@ -608,6 +661,8 @@ protected function createSearchFilter($filter, $value)
/**
* Return a copy of this list which does not contain any items that match all params
*
* Raw SQL is not accepted, only actual field names can be passed
*
* @example $list = $list->exclude('Name', 'bob'); // exclude bob from list
* @example $list = $list->exclude('Name', array('aziz', 'bob'); // exclude aziz and bob from list
* @example $list = $list->exclude(array('Name'=>'bob, 'Age'=>21)); // exclude bob that has Age 21
Expand Down Expand Up @@ -648,6 +703,8 @@ public function exclude()
/**
* Return a copy of this list which does not contain any items with any of these params
*
* Raw SQL is not accepted, only actual field names can be passed
*
* @example $list = $list->excludeAny('Name', 'bob'); // exclude bob from list
* @example $list = $list->excludeAny('Name', array('aziz', 'bob'); // exclude aziz and bob from list
* @example $list = $list->excludeAny(array('Name'=>'bob, 'Age'=>21)); // exclude bob or Age 21
Expand Down Expand Up @@ -1190,7 +1247,7 @@ public function removeByFilter($filter)
*/
public function shuffle()
{
return $this->sort(DB::get_conn()->random());
return $this->orderBy(DB::get_conn()->random());
}

/**
Expand Down
8 changes: 7 additions & 1 deletion src/ORM/DataObject.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3422,7 +3422,13 @@ public static function get_one($callerClass = null, $filter = "", $cache = true,

$item = null;
if (!$cache || !isset(self::$_cache_get_one[$callerClass][$cacheKey])) {
$dl = DataObject::get($callerClass)->where($filter)->sort($orderBy);
$dl = DataObject::get($callerClass);
if (!empty($filter)) {
$dl = $dl->where($filter);
}
if (!empty($orderBy)) {
$dl = $dl->sort($orderBy);
}
$item = $dl->first();

if ($cache) {
Expand Down
5 changes: 5 additions & 0 deletions src/ORM/DataQuery.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -716,12 +716,17 @@ public function whereAny($filter)
/**
* Set the ORDER BY clause of this query
*
* Note: while the similarly named DataList::sort() does not allow raw SQL, DataQuery::sort() does allow it
*
* Raw SQL can be vulnerable to SQL injection attacks if used incorrectly, so it's preferable not to use it
*
* @see SQLSelect::orderby()
*
* @param string $sort Column to sort on (escaped SQL statement)
* @param string $direction Direction ("ASC" or "DESC", escaped SQL statement)
* @param bool $clear Clear existing values
* @return $this
*
*/
public function sort($sort = null, $direction = null, $clear = true)
{
Expand Down
Loading

0 comments on commit 81da517

Please sign in to comment.