From 96f12cc588ca411265645c8c318b52c2aa4891de Mon Sep 17 00:00:00 2001
From: Gary Pendergast <pento@git.wordpress.org>
Date: Mon, 20 Apr 2015 05:41:37 +0000
Subject: [PATCH] Clean up some edge cases in `sanitize_sql_orderby()`.

Props vortfu, dd32.



git-svn-id: https://develop.svn.wordpress.org/trunk@32164 602fd350-edb4-49c9-b593-d223f7449a82
---
 src/wp-includes/formatting.php                | 22 +++---
 .../tests/formatting/SanitizeOrderby.php      | 79 ++++++++++++-------
 2 files changed, 61 insertions(+), 40 deletions(-)

diff --git a/src/wp-includes/formatting.php b/src/wp-includes/formatting.php
index 679566b44c..eacc2600d0 100644
--- a/src/wp-includes/formatting.php
+++ b/src/wp-includes/formatting.php
@@ -1362,21 +1362,23 @@ function sanitize_title_with_dashes( $title, $raw_title = '', $context = 'displa
 }
 
 /**
- * Ensures a string is a valid SQL order by clause.
+ * Ensures a string is a valid SQL 'order by' clause.
  *
- * Accepts one or more columns, with or without ASC/DESC, and also accepts
- * RAND().
+ * Accepts one or more columns, with or without a sort order (ASC / DESC).
+ * e.g. 'column_1', 'column_1, column_2', 'column_1 ASC, column_2 DESC' etc.
+ *
+ * Also accepts 'RAND()'.
  *
  * @since 2.5.1
  *
- * @param string $orderby Order by string to be checked.
- * @return false|string Returns the order by clause if it is a match, false otherwise.
+ * @param string $orderby Order by clause to be validated.
+ * @return string|bool Returns $orderby if valid, false otherwise.
  */
-function sanitize_sql_orderby( $orderby ){
-	preg_match('/^\s*([a-z0-9_]+(\s+(ASC|DESC))?(\s*,\s*|\s*$))+|^\s*RAND\(\s*\)\s*$/i', $orderby, $obmatches);
-	if ( !$obmatches )
-		return false;
-	return $orderby;
+function sanitize_sql_orderby( $orderby ) {
+	if ( preg_match( '/^\s*(([a-z0-9_]+|`[a-z0-9_]+`)(\s+(ASC|DESC))?\s*(,\s*(?=[a-z0-9_`])|$))+$/i', $orderby ) || preg_match( '/^\s*RAND\(\s*\)\s*$/i', $orderby ) ) {
+		return $orderby;
+	}
+	return false;
 }
 
 /**
diff --git a/tests/phpunit/tests/formatting/SanitizeOrderby.php b/tests/phpunit/tests/formatting/SanitizeOrderby.php
index 48ef66255b..371d63b52e 100644
--- a/tests/phpunit/tests/formatting/SanitizeOrderby.php
+++ b/tests/phpunit/tests/formatting/SanitizeOrderby.php
@@ -1,38 +1,57 @@
 <?php
 
-/* // @todo These tests need to be rewritten for sanitize_sql_orderby
+/**
+ * @group sanitize_sql_orderby
+ */
 class Tests_Formatting_SanitizeOrderby extends WP_UnitTestCase {
-	function test_empty() {
-		$cols = array('a' => 'a');
-		$this->assertEquals( '', sanitize_sql_orderby('', $cols) );
-		$this->assertEquals( '', sanitize_sql_orderby('  ', $cols) );
-		$this->assertEquals( '', sanitize_sql_orderby("\t", $cols) );
-		$this->assertEquals( '', sanitize_sql_orderby(null, $cols) );
-		$this->assertEquals( '', sanitize_sql_orderby(0, $cols) );
-		$this->assertEquals( '', sanitize_sql_orderby('+', $cols) );
-		$this->assertEquals( '', sanitize_sql_orderby('-', $cols) );
+
+	/**
+	 * @covers ::sanitize_sql_orderby
+	 * @dataProvider valid_orderbys
+	 */
+	function test_valid( $orderby ) {
+		$this->assertEquals( $orderby, sanitize_sql_orderby( $orderby ) );
+	}
+	function valid_orderbys() {
+		return array(
+			array( '1' ),
+			array( '1 ASC' ),
+			array( '1 ASC, 2' ),
+			array( '1 ASC, 2 DESC' ),
+			array( '1 ASC, 2 DESC, 3' ),
+			array( '       1      DESC' ),
+			array( 'field ASC' ),
+			array( 'field1 ASC, field2' ),
+			array( 'field_1 ASC, field_2 DESC' ),
+			array( 'field1, field2 ASC' ),
+			array( '`field1`' ),
+			array( '`field1` ASC' ),
+			array( '`field` ASC, `field2`' ),
+			array( 'RAND()' ),
+			array( '   RAND(  )   ' ),
+		);
 	}
 
-	function test_unknown_column() {
-		$cols = array('name' => 'post_name', 'date' => 'post_date');
-		$this->assertEquals( '', sanitize_sql_orderby('unknown_column', $cols) );
-		$this->assertEquals( '', sanitize_sql_orderby('+unknown_column', $cols) );
-		$this->assertEquals( '', sanitize_sql_orderby('-unknown_column', $cols) );
-		$this->assertEquals( '', sanitize_sql_orderby('-unknown1,+unknown2,unknown3', $cols) );
-		$this->assertEquals( 'post_name ASC', sanitize_sql_orderby('name,unknown_column', $cols) );
-		$this->assertEquals( '', sanitize_sql_orderby('!@#$%^&*()_=~`\'",./', $cols) );
+	/**
+	 * @covers ::sanitize_sql_orderby
+	 * @dataProvider invalid_orderbys
+	 */
+	function test_invalid( $orderby ) {
+		$this->assertFalse( sanitize_sql_orderby( $orderby ) );
 	}
-
-	function test_valid() {
-		$cols = array('name' => 'post_name', 'date' => 'post_date', 'random' => 'rand()');
-		$this->assertEquals( 'post_name ASC', sanitize_sql_orderby('name', $cols) );
-		$this->assertEquals( 'post_name ASC', sanitize_sql_orderby('+name', $cols) );
-		$this->assertEquals( 'post_name DESC', sanitize_sql_orderby('-name', $cols) );
-		$this->assertEquals( 'post_date ASC, post_name ASC', sanitize_sql_orderby('date,name', $cols) );
-		$this->assertEquals( 'post_date ASC, post_name ASC', sanitize_sql_orderby(' date , name ', $cols) );
-		$this->assertEquals( 'post_name DESC, post_date ASC', sanitize_sql_orderby('-name,date', $cols) );
-		$this->assertEquals( 'post_name ASC, post_date ASC', sanitize_sql_orderby('name ,+ date', $cols) );
-		$this->assertEquals( 'rand() ASC', sanitize_sql_orderby('random', $cols) );
+	function invalid_orderbys() {
+		return array(
+			array( '' ),
+			array( '1 2' ),
+			array( '1, 2 3' ),
+			array( '1 DESC, ' ),
+			array( 'field-1' ),
+			array( 'field DESC,' ),
+			array( 'field1 field2' ),
+			array( 'field RAND()' ),
+			array( 'RAND() ASC' ),
+			array( '`field1` ASC, `field2' ),
+			array( 'field, !@#$%^' ),
+		);
 	}
 }
-*/