Jeroen De Dauw has submitted this change and it was merged.
Change subject: Add escaping of identifiers in MYSQL
......................................................................
Add escaping of identifiers in MYSQL
Change-Id: I225a78ed3789b69b3b50582c3c04691c909de26a
---
M src/MySQL/MySQLFieldSqlBuilder.php
M src/MySQL/MySQLIndexSqlBuilder.php
M src/MySQL/MySQLSchemaSqlBuilder.php
M src/MySQL/MySQLTableSqlBuilder.php
M tests/phpunit/MySQL/MySQLFieldSqlBuilderTest.php
M tests/phpunit/MySQL/MySQLIndexSqlBuilderTest.php
M tests/phpunit/MySQL/MySQLSchemaSqlBuilderTest.php
M tests/phpunit/MySQL/MySQLTableSqlBuilderTest.php
8 files changed, 81 insertions(+), 32 deletions(-)
Approvals:
Jeroen De Dauw: Looks good to me, approved
jenkins-bot: Verified
diff --git a/src/MySQL/MySQLFieldSqlBuilder.php
b/src/MySQL/MySQLFieldSqlBuilder.php
index 4034902..3adbc40 100644
--- a/src/MySQL/MySQLFieldSqlBuilder.php
+++ b/src/MySQL/MySQLFieldSqlBuilder.php
@@ -24,8 +24,7 @@
}
public function getFieldSQL( FieldDefinition $field ){
- //todo escape name once identifier escaping is implemented
- $sql = $field->getName() . ' ';
+ $sql = $this->escaper->getEscapedIdentifier( $field->getName()
). ' ';
$sql .= $this->getFieldType( $field->getType() );
diff --git a/src/MySQL/MySQLIndexSqlBuilder.php
b/src/MySQL/MySQLIndexSqlBuilder.php
index 5140173..526c904 100644
--- a/src/MySQL/MySQLIndexSqlBuilder.php
+++ b/src/MySQL/MySQLIndexSqlBuilder.php
@@ -3,6 +3,7 @@
namespace Wikibase\Database\MySQL;
use RuntimeException;
+use Wikibase\Database\Escaper;
use Wikibase\Database\Schema\Definitions\IndexDefinition;
use Wikibase\Database\Schema\Definitions\TableDefinition;
use Wikibase\Database\Schema\IndexSqlBuilder;
@@ -15,10 +16,15 @@
*/
class MySQLIndexSqlBuilder extends IndexSqlBuilder {
+ protected $escaper;
+ protected $tableNameFormatter;
+
/**
+ * @param Escaper $escaper
* @param TableNameFormatter $tableNameFormatter
*/
- public function __construct( TableNameFormatter $tableNameFormatter ) {
+ public function __construct( Escaper $escaper, TableNameFormatter
$tableNameFormatter ) {
+ $this->escaper = $escaper;
$this->tableNameFormatter = $tableNameFormatter;
}
@@ -27,17 +33,17 @@
$sql .= $this->getIndexType( $index->getType() );
if( $index->getType() !== IndexDefinition::TYPE_PRIMARY ){
- $sql .= ' `'.$index->getName().'`';//todo escape index
name?
+ $sql .= ' ' . $this->escaper->getEscapedIdentifier(
$index->getName() );
}
$sql .= ' ON ' . $this->tableNameFormatter->formatTableName(
$tableName );
$columnNames = array();
foreach( $index->getColumns() as $columnName => $intSize ){
- $columnNames[] = $columnName;
+ $columnNames[] = $this->escaper->getEscapedIdentifier(
$columnName );
}
- $sql .= ' (`'.implode( '`,`', $columnNames ).'`)';
+ $sql .= ' (' . implode( ',', $columnNames ). ')';
return $sql;
}
diff --git a/src/MySQL/MySQLSchemaSqlBuilder.php
b/src/MySQL/MySQLSchemaSqlBuilder.php
index 75baf07..06fc9c0 100644
--- a/src/MySQL/MySQLSchemaSqlBuilder.php
+++ b/src/MySQL/MySQLSchemaSqlBuilder.php
@@ -18,11 +18,13 @@
*/
class MySQLSchemaSqlBuilder implements SchemaModificationSqlBuilder {
+ protected $escaper;
protected $fieldSqlBuilder;
protected $tableNameFormatter;
- public function __construct( Escaper $fieldValueEscaper,
TableNameFormatter $tableNameFormatter ) {
- $this->fieldSqlBuilder = new MySQLFieldSqlBuilder(
$fieldValueEscaper );
+ public function __construct( Escaper $escaper, TableNameFormatter
$tableNameFormatter ) {
+ $this->escaper = $escaper;
+ $this->fieldSqlBuilder = new MySQLFieldSqlBuilder( $escaper );
$this->tableNameFormatter = $tableNameFormatter;
}
@@ -34,6 +36,7 @@
*/
public function getRemoveFieldSql( $tableName, $fieldName ) {
$tableName = $this->tableNameFormatter->formatTableName(
$tableName );
+ $fieldName = $this->escaper->getEscapedIdentifier( $fieldName );
return "ALTER TABLE {$tableName} DROP {$fieldName}"; //todo
escape $fieldName?
}
@@ -56,6 +59,7 @@
*/
public function getRemoveIndexSql( $tableName, $indexName ){
$tableName = $this->tableNameFormatter->formatTableName(
$tableName );
+ $indexName = $this->escaper->getEscapedIdentifier( $indexName );
return "DROP INDEX {$indexName} ON {$tableName}";
}
@@ -66,7 +70,7 @@
* @return string
*/
public function getAddIndexSql( $tableName, IndexDefinition $index ){
- $indexSqlBuilder = new MySQLIndexSqlBuilder(
$this->tableNameFormatter );
+ $indexSqlBuilder = new MySQLIndexSqlBuilder( $this->escaper,
$this->tableNameFormatter );
return $indexSqlBuilder->getIndexSQL( $index, $tableName );
}
diff --git a/src/MySQL/MySQLTableSqlBuilder.php
b/src/MySQL/MySQLTableSqlBuilder.php
index 41d3818..c32e488 100644
--- a/src/MySQL/MySQLTableSqlBuilder.php
+++ b/src/MySQL/MySQLTableSqlBuilder.php
@@ -80,18 +80,17 @@
$sql = $this->getIndexType( $index->getType() );
if( $index->getType() !== IndexDefinition::TYPE_PRIMARY ){
- //todo escape name once identifier escaping is
implemented
- $sql .= ' `'.$index->getName().'`';
+ $sql .= ' ' . $this->escaper->getEscapedIdentifier(
$index->getName() );
}
$columnNames = array();
//TODO FIXME Error: 1170 BLOB/TEXT column 'textfield' used in
key specification without a key length (localhost)
//todo $intSize here needs to specify the length of the key for
text fields.
foreach( $index->getColumns() as $columnName => $intSize ){
- $columnNames[] = $columnName;
+ $columnNames[] = $this->escaper->getEscapedIdentifier(
$columnName );
}
- $sql .= ' (`'.implode( '`,`', $columnNames ).'`)';
+ $sql .= ' (' . implode( ',', $columnNames ) . ')';
return $sql;
}
diff --git a/tests/phpunit/MySQL/MySQLFieldSqlBuilderTest.php
b/tests/phpunit/MySQL/MySQLFieldSqlBuilderTest.php
index 1bca0a3..8d99654 100644
--- a/tests/phpunit/MySQL/MySQLFieldSqlBuilderTest.php
+++ b/tests/phpunit/MySQL/MySQLFieldSqlBuilderTest.php
@@ -30,6 +30,12 @@
return '|' . $value . '|';
} ) );
+ $mockEscaper->expects( $this->any() )
+ ->method( 'getEscapedIdentifier' )
+ ->will( $this->returnCallback( function( $value ) {
+ return '-' . $value . '-';
+ } ) );
+
$sqlBuilder = new MySQLFieldSqlBuilder( $mockEscaper );
$actualSQL = $sqlBuilder->getFieldSQL( $field );
@@ -45,7 +51,7 @@
'fieldName',
FieldDefinition::TYPE_INTEGER
),
- 'fieldName INT NULL'
+ '-fieldName- INT NULL'
);
$argLists[] = array(
@@ -57,7 +63,7 @@
FieldDefinition::NO_ATTRIB,
FieldDefinition::AUTOINCREMENT
),
- 'fieldName INT NULL AUTO_INCREMENT'
+ '-fieldName- INT NULL AUTO_INCREMENT'
);
$argLists[] = array(
@@ -66,7 +72,7 @@
FieldDefinition::TYPE_FLOAT,
FieldDefinition::NOT_NULL
),
- 'fieldName FLOAT NOT NULL'
+ '-fieldName- FLOAT NOT NULL'
);
$argLists[] = array(
@@ -76,7 +82,7 @@
FieldDefinition::NOT_NULL,
'1'
),
- 'fieldName TINYINT DEFAULT |1| NOT NULL'
+ '-fieldName- TINYINT DEFAULT |1| NOT NULL'
);
$argLists[] = array(
@@ -86,7 +92,7 @@
FieldDefinition::NOT_NULL,
'foo'
),
- 'fieldName BLOB DEFAULT |foo| NOT NULL'
+ '-fieldName- BLOB DEFAULT |foo| NOT NULL'
);
return $argLists;
diff --git a/tests/phpunit/MySQL/MySQLIndexSqlBuilderTest.php
b/tests/phpunit/MySQL/MySQLIndexSqlBuilderTest.php
index b6c9f32..912fd6e 100644
--- a/tests/phpunit/MySQL/MySQLIndexSqlBuilderTest.php
+++ b/tests/phpunit/MySQL/MySQLIndexSqlBuilderTest.php
@@ -22,12 +22,19 @@
* @dataProvider fieldAndSqlProvider
*/
public function testGetCreateTableSql( IndexDefinition $index,
$expectedSQL ) {
+ $mockEscaper = $this->getMock( 'Wikibase\Database\Escaper' );
+ $mockEscaper->expects( $this->any() )
+ ->method( 'getEscapedIdentifier' )
+ ->will( $this->returnCallback( function( $value ) {
+ return '-' . $value . '-';
+ } ) );
+
$mockTableNameFormatter = $this->getMock(
'Wikibase\Database\TableNameFormatter' );
$mockTableNameFormatter->expects( $this->any() )
->method( 'formatTableName' )
->will( $this->returnArgument(0) );
- $sqlBuilder = new MySQLIndexSqlBuilder( $mockTableNameFormatter
);
+ $sqlBuilder = new MySQLIndexSqlBuilder( $mockEscaper,
$mockTableNameFormatter );
$sql = $sqlBuilder->getIndexSQL( $index, 'tableName' );
$this->assertEquals( $expectedSQL, $sql );
}
@@ -44,7 +51,7 @@
array( 'intField' => 0, 'textField' => 0 ),
IndexDefinition::TYPE_INDEX
),
- 'CREATE INDEX `indexName` ON tableName
(`intField`,`textField`)'
+ 'CREATE INDEX -indexName- ON tableName
(-intField-,-textField-)'
);
$argLists[] = array(
@@ -53,7 +60,7 @@
array( 'intField' => 0, 'textField' => 0 ),
IndexDefinition::TYPE_UNIQUE
),
- 'CREATE UNIQUE INDEX `indexName` ON tableName
(`intField`,`textField`)'
+ 'CREATE UNIQUE INDEX -indexName- ON tableName
(-intField-,-textField-)'
);
$argLists[] = array(
@@ -62,7 +69,7 @@
array( 'intField' => 0, 'textField' => 0 ),
IndexDefinition::TYPE_PRIMARY
),
- 'CREATE PRIMARY KEY ON tableName
(`intField`,`textField`)'
+ 'CREATE PRIMARY KEY ON tableName
(-intField-,-textField-)'
);
return $argLists;
@@ -70,6 +77,13 @@
public function testUnsupportedType() {
$this->setExpectedException( 'RuntimeException', 'does not
support db indexes of type' );
+
+ $mockEscaper = $this->getMock( 'Wikibase\Database\Escaper' );
+ $mockEscaper->expects( $this->any() )
+ ->method( 'getEscapedIdentifier' )
+ ->will( $this->returnCallback( function( $value ) {
+ return '-' . $value . '-';
+ } ) );
$tableNameFormatter = $this->getMockBuilder(
'Wikibase\Database\MediaWiki\MediaWikiTableNameFormatter' )
->disableOriginalConstructor()
@@ -82,7 +96,7 @@
->method( 'getType' )
->will( $this->returnValue( 'foobar' ) );
- $sqlBuilder = new MySQLIndexSqlBuilder( $tableNameFormatter );
+ $sqlBuilder = new MySQLIndexSqlBuilder( $mockEscaper,
$tableNameFormatter );
$sqlBuilder->getIndexSQL( $indexDefinition, 'tableName' );
}
diff --git a/tests/phpunit/MySQL/MySQLSchemaSqlBuilderTest.php
b/tests/phpunit/MySQL/MySQLSchemaSqlBuilderTest.php
index 5b7d886..305186e 100644
--- a/tests/phpunit/MySQL/MySQLSchemaSqlBuilderTest.php
+++ b/tests/phpunit/MySQL/MySQLSchemaSqlBuilderTest.php
@@ -21,10 +21,17 @@
private function newInstance() {
$mockEscaper = $this->getMock( 'Wikibase\Database\Escaper' );
+
$mockEscaper->expects( $this->any() )
->method( 'getEscapedValue' )
->will( $this->returnCallback( function( $input ) {
return "|$input|";
+ } ) );
+
+ $mockEscaper->expects( $this->any() )
+ ->method( 'getEscapedIdentifier' )
+ ->will( $this->returnCallback( function( $value ) {
+ return '-' . $value . '-';
} ) );
$mockTableNameFormatter = $this->getMock(
'Wikibase\Database\TableNameFormatter' );
@@ -40,26 +47,26 @@
public function testGetRemoveFieldSql(){
$instance = $this->newInstance();
$sql = $instance->getRemoveFieldSql( 'tableName', 'fieldName' );
- $this->assertEquals( "ALTER TABLE ||tableName|| DROP
fieldName", $sql );
+ $this->assertEquals( "ALTER TABLE ||tableName|| DROP
-fieldName-", $sql );
}
public function testGetAddFieldSql(){
$instance = $this->newInstance();
$field = new FieldDefinition( 'intField',
FieldDefinition::TYPE_INTEGER, FieldDefinition::NOT_NULL, 42 );
$sql = $instance->getAddFieldSql( 'tableName', $field );
- $this->assertEquals( 'ALTER TABLE ||tableName|| ADD intField
INT DEFAULT |42| NOT NULL', $sql );
+ $this->assertEquals( 'ALTER TABLE ||tableName|| ADD -intField-
INT DEFAULT |42| NOT NULL', $sql );
}
public function testGetRemoveIndexSql(){
$instance = $this->newInstance();
$sql = $instance->getRemoveIndexSql( 'tableName', 'indexName' );
- $this->assertEquals( "DROP INDEX indexName ON ||tableName||",
$sql );
+ $this->assertEquals( "DROP INDEX -indexName- ON ||tableName||",
$sql );
}
public function testGetAddIndexSql(){
$instance = $this->newInstance();
- $sql = $instance->getAddIndexSql( 'tableName', new
IndexDefinition( 'name', array( 'a' => 0, 'b' => 0 ),
IndexDefinition::TYPE_INDEX ) );
- $this->assertEquals( "CREATE INDEX `name` ON ||tableName||
(`a`,`b`)", $sql );
+ $sql = $instance->getAddIndexSql( 'tableName', new
IndexDefinition( 'indexName', array( 'a' => 0, 'b' => 0 ),
IndexDefinition::TYPE_INDEX ) );
+ $this->assertEquals( "CREATE INDEX -indexName- ON ||tableName||
(-a-,-b-)", $sql );
}
}
\ No newline at end of file
diff --git a/tests/phpunit/MySQL/MySQLTableSqlBuilderTest.php
b/tests/phpunit/MySQL/MySQLTableSqlBuilderTest.php
index 6dc7c4e..fbbc73c 100644
--- a/tests/phpunit/MySQL/MySQLTableSqlBuilderTest.php
+++ b/tests/phpunit/MySQL/MySQLTableSqlBuilderTest.php
@@ -35,10 +35,24 @@
->method( 'getEscapedValue' )
->will( $this->returnArgument(0) );
+ $mockEscaper->expects( $this->any() )
+ ->method( 'getEscapedValue' )
+ ->will( $this->returnCallback( function( $value ) {
+ return '|' . $value . '|';
+ } ) );
+
+ $mockEscaper->expects( $this->any() )
+ ->method( 'getEscapedIdentifier' )
+ ->will( $this->returnCallback( function( $value ) {
+ return '-' . $value . '-';
+ } ) );
+
$mockTableNameFormatter = $this->getMock(
'Wikibase\Database\TableNameFormatter' );
$mockTableNameFormatter->expects( $this->any() )
->method( 'formatTableName' )
->will( $this->returnArgument(0) );
+
+
return new MySQLTableSqlBuilder(
self::DB_NAME,
@@ -68,7 +82,7 @@
new FieldDefinition( 'fieldName',
FieldDefinition::TYPE_INTEGER )
)
),
- 'CREATE TABLE `dbName`.tableName (fieldName INT NULL)
ENGINE=InnoDB, DEFAULT CHARSET=binary;'
+ 'CREATE TABLE `dbName`.tableName (-fieldName- INT NULL)
ENGINE=InnoDB, DEFAULT CHARSET=binary;'
);
$argLists[] = array(
@@ -86,7 +100,7 @@
),
)
),
- 'CREATE TABLE `dbName`.tableName (primaryField INT NOT
NULL, textField BLOB NULL, intField INT DEFAULT 42 NOT NULL) ENGINE=InnoDB,
DEFAULT CHARSET=binary;'
+ 'CREATE TABLE `dbName`.tableName (-primaryField- INT
NOT NULL, -textField- BLOB NULL, -intField- INT DEFAULT 42 NOT NULL)
ENGINE=InnoDB, DEFAULT CHARSET=binary;'
);
@@ -107,7 +121,7 @@
),
)
),
- 'CREATE TABLE `dbName`.tableName (textField BLOB NULL,
intField INT DEFAULT 42 NOT NULL, INDEX `indexName` (`textField`,`intField`))
ENGINE=InnoDB, DEFAULT CHARSET=binary;'
+ 'CREATE TABLE `dbName`.tableName (-textField- BLOB
NULL, -intField- INT DEFAULT 42 NOT NULL, INDEX -indexName-
(-textField-,-intField-)) ENGINE=InnoDB, DEFAULT CHARSET=binary;'
);
$argLists[] = array(
@@ -133,7 +147,7 @@
),
)
),
- 'CREATE TABLE `dbName`.tableName (textField BLOB NULL,
intField INT DEFAULT 42 NOT NULL, textField2 BLOB NULL, INDEX `indexName`
(`intField`), UNIQUE INDEX `uniqueIndexName` (`textField2`)) ENGINE=InnoDB,
DEFAULT CHARSET=binary;'
+ 'CREATE TABLE `dbName`.tableName (-textField- BLOB
NULL, -intField- INT DEFAULT 42 NOT NULL, -textField2- BLOB NULL, INDEX
-indexName- (-intField-), UNIQUE INDEX -uniqueIndexName- (-textField2-))
ENGINE=InnoDB, DEFAULT CHARSET=binary;'
);
return $argLists;
--
To view, visit https://gerrit.wikimedia.org/r/89631
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I225a78ed3789b69b3b50582c3c04691c909de26a
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/extensions/WikibaseDatabase
Gerrit-Branch: master
Gerrit-Owner: Addshore <[email protected]>
Gerrit-Reviewer: Jeroen De Dauw <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
