From d86285fb80b30bf6a0fc778f1c6372a62ba4d087 Mon Sep 17 00:00:00 2001
From: "Eloy Lafuente (stronk7)" <stronk7@moodle.org>
Date: Mon, 12 Jan 2015 18:17:36 +0100
Subject: [PATCH] MDL-35155 database: better sql_substr() impl. for
 mssql/sqlsrv + unit tests

MSSQL's substring() implementation is somehow silly/strict and unable to
perform implicit casts to integer both for the start and length parameters.

This hits Moodle badly because of another problems (MDL-23997) we decided
to cast to string all bound placeholders long ago.

So this commit just enforces the cast of the start and length parameters to
integer. And includes unit tests for using placeholders on all positions in
the sql_substr() method.
---
 lib/dml/mssql_native_moodle_database.php  |  4 ++--
 lib/dml/sqlsrv_native_moodle_database.php |  4 ++--
 lib/dml/tests/dml_test.php                | 27 ++++++++++++++++++++++++---
 3 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/lib/dml/mssql_native_moodle_database.php b/lib/dml/mssql_native_moodle_database.php
index cebe886481e..44ddc603aca 100644
--- a/lib/dml/mssql_native_moodle_database.php
+++ b/lib/dml/mssql_native_moodle_database.php
@@ -1257,9 +1257,9 @@ class mssql_native_moodle_database extends moodle_database {
 s only returning name of SQL substring function, it now requires all parameters.');
         }
         if ($length === false) {
-            return "SUBSTRING($expr, $start, LEN($expr))";
+            return "SUBSTRING($expr, " . $this->sql_cast_char2int($start) . ", 2^31-1)";
         } else {
-            return "SUBSTRING($expr, $start, $length)";
+            return "SUBSTRING($expr, " . $this->sql_cast_char2int($start) . ", " . $this->sql_cast_char2int($length) . ")";
         }
     }
 
diff --git a/lib/dml/sqlsrv_native_moodle_database.php b/lib/dml/sqlsrv_native_moodle_database.php
index 68efc62376c..f7613988f30 100644
--- a/lib/dml/sqlsrv_native_moodle_database.php
+++ b/lib/dml/sqlsrv_native_moodle_database.php
@@ -1324,9 +1324,9 @@ class sqlsrv_native_moodle_database extends moodle_database {
         }
 
         if ($length === false) {
-            return "SUBSTRING($expr, $start, LEN($expr))";
+            return "SUBSTRING($expr, " . $this->sql_cast_char2int($start) . ", 2^31-1)";
         } else {
-            return "SUBSTRING($expr, $start, $length)";
+            return "SUBSTRING($expr, " . $this->sql_cast_char2int($start) . ", " . $this->sql_cast_char2int($length) . ")";
         }
     }
 
diff --git a/lib/dml/tests/dml_test.php b/lib/dml/tests/dml_test.php
index ce625e0aa35..8618140d4ef 100644
--- a/lib/dml/tests/dml_test.php
+++ b/lib/dml/tests/dml_test.php
@@ -4027,9 +4027,30 @@ class core_dml_testcase extends database_driver_testcase {
             $this->assertInstanceOf('coding_exception', $e);
         }
 
-        $sql = "SELECT id, ".$DB->sql_substr("name", ":param1 + 1")." AS name FROM {{$tablename}}";
-        $record = $DB->get_record_sql($sql, array('param1' => 4));
-        $this->assertEquals(substr($string, 5-1), $record->name);
+        // Cover the function using placeholders in all positions.
+        $start = 4;
+        $length = 2;
+        // 1st param (target).
+        $sql = "SELECT id, ".$DB->sql_substr(":param1", $start)." AS name FROM {{$tablename}}";
+        $record = $DB->get_record_sql($sql, array('param1' => $string));
+        $this->assertEquals(substr($string, $start - 1), $record->name); // PHP's substr is 0-based.
+        // 2nd param (start).
+        $sql = "SELECT id, ".$DB->sql_substr("name", ":param1")." AS name FROM {{$tablename}}";
+        $record = $DB->get_record_sql($sql, array('param1' => $start));
+        $this->assertEquals(substr($string, $start - 1), $record->name); // PHP's substr is 0-based.
+        // 3rd param (length).
+        $sql = "SELECT id, ".$DB->sql_substr("name", $start, ":param1")." AS name FROM {{$tablename}}";
+        $record = $DB->get_record_sql($sql, array('param1' => $length));
+        $this->assertEquals(substr($string, $start - 1,  $length), $record->name); // PHP's substr is 0-based.
+        // All together.
+        $sql = "SELECT id, ".$DB->sql_substr(":param1", ":param2", ":param3")." AS name FROM {{$tablename}}";
+        $record = $DB->get_record_sql($sql, array('param1' => $string, 'param2' => $start, 'param3' => $length));
+        $this->assertEquals(substr($string, $start - 1,  $length), $record->name); // PHP's substr is 0-based.
+
+        // Try also with some expression passed.
+        $sql = "SELECT id, ".$DB->sql_substr("name", "(:param1 + 1) - 1")." AS name FROM {{$tablename}}";
+        $record = $DB->get_record_sql($sql, array('param1' => $start));
+        $this->assertEquals(substr($string, $start - 1), $record->name); // PHP's substr is 0-based.
     }
 
     public function test_sql_length() {
-- 
2.11.4.GIT