MDL-29313 enforce varchar size limit

admin/xmldb/actions/edit_field/edit_field.js

@@ -99,7 +99,7 @@ function transformForm(event) {
             decimalsTip.innerHTML = ' 0...length or empty';
             break;
         case '4':  // XMLDB_TYPE_CHAR
-            lengthTip.innerHTML = ' 1...255';
+            lengthTip.innerHTML = ' 1...'.xmldb_field::CHAR_MAX_LENGTH;
             decimalsTip.innerHTML = '';
             decimalsField.disabled = true;
             decimalsField.value = '';

admin/xmldb/actions/edit_field_save/edit_field_save.class.php

@@ -192,7 +192,7 @@ class edit_field_save extends XMLDBAction {
     /// Char checks
         if ($type == XMLDB_TYPE_CHAR) {
             if (!(is_numeric($length) && !empty($length) && intval($length)==floatval($length) &&
-                  $length > 0 && $length <= 255)) {
+                  $length > 0 && $length <= xmldb_field::CHAR_MAX_LENGTH)) {
                 $errors[] = $this->str['charincorrectlength'];
             }
             if ($default !== NULL && $default !== '') {

lib/ddl/mysql_sql_generator.php

@@ -281,7 +281,7 @@ class mysql_sql_generator extends sql_generator {
     /// Change the name of the field to perform the change
         $xmldb_field_clone->setName($xmldb_field_clone->getName() . ' ' . $newname);
 
-        $fieldsql = $this->getFieldSQL($xmldb_field_clone);
+        $fieldsql = $this->getFieldSQL($xmldb_table, $xmldb_field_clone);
 
         $sql = 'ALTER TABLE ' . $this->getTableName($xmldb_table) . ' CHANGE ' . $fieldsql;
 

lib/ddl/postgres_sql_generator.php

@@ -280,7 +280,7 @@ class postgres_sql_generator extends sql_generator {
                 $results[] = 'ALTER TABLE ' . $tablename . ' ALTER COLUMN ' . $fieldname . ' DROP DEFAULT'; /// Drop default clause
             }
             $alterstmt = 'ALTER TABLE ' . $tablename . ' ALTER COLUMN ' . $this->getEncQuoted($xmldb_field->getName()) .
-                         ' TYPE' . $this->getFieldSQL($xmldb_field, null, true, true, null, false);
+                         ' TYPE' . $this->getFieldSQL($xmldb_table, $xmldb_field, null, true, true, null, false);
         /// Some castings must be performed explicity (mainly from text|char to numeric|integer)
             if (($oldmetatype == 'C' || $oldmetatype == 'X') &&
                 ($xmldb_field->getType() == XMLDB_TYPE_NUMBER || $xmldb_field->getType() == XMLDB_TYPE_FLOAT)) {

lib/ddl/simpletest/testddl.php

@@ -1596,6 +1596,56 @@ class ddl_test extends UnitTestCase {
         }
     }
 
+    public function test_char_size_limit() {
+        $DB = $this->tdb;
+        $dbman = $DB->get_manager();
+
+        $maxstr = '';
+        for($i=0; $i<xmldb_field::CHAR_MAX_LENGTH; $i++) {
+            $maxstr .= '言'; // random long string that should fix exactly the limit for one char column
+        }
+
+        $table = new xmldb_table('testtable');
+        $table->add_field('id', XMLDB_TYPE_INTEGER, '10', XMLDB_UNSIGNED, XMLDB_NOTNULL, XMLDB_SEQUENCE, null);
+        $table->add_field('name', XMLDB_TYPE_CHAR, xmldb_field::CHAR_MAX_LENGTH, null, XMLDB_NOTNULL, null);
+        $table->add_key('primary', XMLDB_KEY_PRIMARY, array('id'));
+
+        // Drop if exists
+        if ($dbman->table_exists($table)) {
+            $dbman->drop_table($table);
+        }
+        $dbman->create_table($table);
+        $tablename = $table->getName();
+        $this->tables[$tablename] = $table;
+
+        $rec = new stdClass();
+        $rec->name = $maxstr;
+
+        $id = $DB->insert_record($tablename, $rec);
+        $this->assertTrue(!empty($id));
+
+        $rec = $DB->get_record($tablename, array('id'=>$id));
+        $this->assertIdentical($rec->name, $maxstr);
+
+
+        $table = new xmldb_table('testtable');
+        $table->add_field('id', XMLDB_TYPE_INTEGER, '10', XMLDB_UNSIGNED, XMLDB_NOTNULL, XMLDB_SEQUENCE, null);
+        $table->add_field('name', XMLDB_TYPE_CHAR, xmldb_field::CHAR_MAX_LENGTH+1, null, XMLDB_NOTNULL, null);
+        $table->add_key('primary', XMLDB_KEY_PRIMARY, array('id'));
+
+        // Drop if exists
+        if ($dbman->table_exists($table)) {
+            $dbman->drop_table($table);
+        }
+
+        try {
+            $dbman->create_table($table);
+            $this->assertTrue(false);
+        } catch (Exception $e) {
+            $this->assertTrue($e instanceof coding_exception);
+        }
+    }
+
  // Following methods are not supported == Do not test
 /*
     public function testRenameIndex() {

lib/ddl/sql_generator.php

@@ -240,7 +240,7 @@ abstract class sql_generator {
             if ($xmldb_field->getSequence()) {
                 $sequencefield = $xmldb_field->getName();
             }
-            $table .= "\n    " . $this->getFieldSQL($xmldb_field);
+            $table .= "\n    " . $this->getFieldSQL($xmldb_table, $xmldb_field);
             $table .= ',';
         }
     /// Add the keys, separated by commas
@@ -370,7 +370,10 @@ abstract class sql_generator {
     /**
      * Given one correct xmldb_field, returns the complete SQL line to create it
      */
-    public function getFieldSQL($xmldb_field, $skip_type_clause = NULL, $skip_default_clause = NULL, $skip_notnull_clause = NULL, $specify_nulls_clause = NULL, $specify_field_name = true)  {
+    public function getFieldSQL($xmldb_table, $xmldb_field, $skip_type_clause = NULL, $skip_default_clause = NULL, $skip_notnull_clause = NULL, $specify_nulls_clause = NULL, $specify_field_name = true)  {
+        if ($error = $xmldb_field->validateDefinition($xmldb_table)) {
+            throw new coding_exception($error);
+        }
 
         $skip_type_clause = is_null($skip_type_clause) ? $this->alter_column_skip_type : $skip_type_clause;
         $skip_default_clause = is_null($skip_default_clause) ? $this->alter_column_skip_default : $skip_default_clause;
@@ -595,7 +598,7 @@ abstract class sql_generator {
         $tablename = $this->getTableName($xmldb_table);
 
     /// Build the standard alter table add
-        $sql = $this->getFieldSQL($xmldb_field, $skip_type_clause,
+        $sql = $this->getFieldSQL($xmldb_table, $xmldb_field, $skip_type_clause,
                                   $skip_default_clause,
                                   $skip_notnull_clause);
         $altertable = 'ALTER TABLE ' . $tablename . ' ADD ' . $sql;
@@ -642,7 +645,7 @@ abstract class sql_generator {
 
     /// Build de alter sentence using the alter_column_sql template
         $alter = str_replace('TABLENAME', $this->getTableName($xmldb_table), $this->alter_column_sql);
-        $colspec = $this->getFieldSQL($xmldb_field, $skip_type_clause,
+        $colspec = $this->getFieldSQL($xmldb_table, $xmldb_field, $skip_type_clause,
                                       $skip_default_clause,
                                       $skip_notnull_clause,
                                       true);

lib/xmldb/xmldb_field.php

@@ -36,6 +36,17 @@ class xmldb_field extends xmldb_object {
     var $sequence;
     var $decimals;
 
+    /**
+     * Note:
+     *  - Oracle: VARCHAR2 has a limit of 4000 bytes
+     *  - SQL Server: NVARCHAR has a limit of 40000 chars
+     *  - MySQL: VARCHAR 65,535 chars
+     *  - PostgreSQL: no limit
+     *
+     * @const maximum length of text field
+     */
+    const CHAR_MAX_LENGTH = 255; //TODO: bump up to 1333
+
     /**
      * Creates one new xmldb_field
      */
@@ -785,6 +796,53 @@ class xmldb_field extends xmldb_object {
 
         return $o;
     }
+
+    /**
+     * Validates the field restrictions.
+     *
+     * The error message should not be localised because it is intended for developers,
+     * end users and admins should never see these problems!
+     *
+     * @param xmldb_table $xmldb_table optional when object is table
+     * @return string null if ok, error message if problem found
+     */
+    function validateDefinition(xmldb_table $xmldb_table=null) {
+        if (!$xmldb_table) {
+            return 'Invalid xmldb_field->validateDefinition() call, $xmldb_table si required.';
+        }
+
+        switch ($this->getType()) {
+            case XMLDB_TYPE_INTEGER:
+            break;
+
+            case XMLDB_TYPE_NUMBER:
+            break;
+
+            case XMLDB_TYPE_FLOAT:
+            break;
+
+            case XMLDB_TYPE_CHAR:
+                if ($this->getLength() > self::CHAR_MAX_LENGTH) {
+                    return 'Invalid field definition in table {'.$xmldb_table->getName(). '}: XMLDB_TYPE_CHAR field "'.$this->getName().'" is too long.'
+                           .' Limit is '.self::CHAR_MAX_LENGTH.' chars.';
+                }
+            break;
+
+            case XMLDB_TYPE_TEXT:
+            break;
+
+            case XMLDB_TYPE_BINARY:
+            break;
+
+            case XMLDB_TYPE_DATETIME:
+            break;
+
+            case XMLDB_TYPE_TIMESTAMP:
+            break;
+        }
+
+        return null;
+    }
 }
 
 /// TODO: Delete for 2.1 (deprecated in 2.0).