From 34812a346d945e64175c255c56cd450fb19227d9 Mon Sep 17 00:00:00 2001 From: Tom Needham Date: Fri, 26 May 2017 11:50:10 +0100 Subject: [PATCH 1/5] Migration to fix term column length --- core/Migrations/Version20170516100103.php | 2 +- core/Migrations/Version20170526104128.php | 33 +++++++++++++++++++++++ lib/private/User/User.php | 4 +++ tests/lib/User/UserTest.php | 26 ++++++++++++++++++ 4 files changed, 64 insertions(+), 1 deletion(-) create mode 100644 core/Migrations/Version20170526104128.php diff --git a/core/Migrations/Version20170516100103.php b/core/Migrations/Version20170516100103.php index 787c76465a4f..48c9fb7a24b9 100644 --- a/core/Migrations/Version20170516100103.php +++ b/core/Migrations/Version20170516100103.php @@ -50,7 +50,7 @@ public function changeSchema(Schema $schema, array $options) { $table->addColumn('term', Type::STRING, [ 'notnull' => true, - 'length' => 256 + 'length' => 255 ]); $table->setPrimaryKey(['id']); diff --git a/core/Migrations/Version20170526104128.php b/core/Migrations/Version20170526104128.php new file mode 100644 index 000000000000..9ee2fa5b3f4e --- /dev/null +++ b/core/Migrations/Version20170526104128.php @@ -0,0 +1,33 @@ +getTable("{$prefix}account_terms"); + // Check column length + if($table->getColumn('term')->getLength() === 255) { + // we don't need to adjust it + return; + } + + // Need to shorten the column by one character + // Check if we have any terms taking up the full 256 chars (unlikely) + + /** @var IDBConnection $db */ + $db = \OC::$server->getDatabaseConnection(); + $db->executeQuery("DELETE FROM {$prefix}account_terms WHERE CHAR_LENGTH(term) >= 256;"); + + // Now update the column length + $table->getColumn('term')->setLength(255); + } +} diff --git a/lib/private/User/User.php b/lib/private/User/User.php index 6db1d6e3762f..50b186959deb 100644 --- a/lib/private/User/User.php +++ b/lib/private/User/User.php @@ -456,6 +456,10 @@ public function getSearchTerms() { * @since 10.0.1 */ public function setSearchTerms(array $terms) { + // Check length of terms + $terms = array_filter($terms, function($term) { + return strlen($term) <= 255; + }); $this->mapper->setTermsForAccount($this->account->getId(), $terms); } } diff --git a/tests/lib/User/UserTest.php b/tests/lib/User/UserTest.php index 93d068af1073..29bebc1984ea 100644 --- a/tests/lib/User/UserTest.php +++ b/tests/lib/User/UserTest.php @@ -315,4 +315,30 @@ public function testGetCloudId() { ->willReturn('http://localhost:8888/owncloud'); $this->assertEquals("foo@localhost:8888/owncloud", $this->user->getCloudId()); } + + /** + * @dataProvider setTermsData + * @param array $terms + * @param array $expected + */ + public function testSettingAccountTerms(array $terms, array $expected) { + $account = $this->getMockBuilder(Account::class)->getMock(); + $account->expects($this->once())->method('__call')->with('getId')->willReturn('foo'); + + $this->accountMapper->expects($this->once()) + ->method('setTermsForAccount') + ->with('foo', $expected); + + // Call the method + $user = new User($account, $this->accountMapper, null, $this->config); + $user->setSearchTerms($terms); + } + + public function setTermsData() { + return [ + 'normal terms' => [['term1'], ['term1']], + 'too long terms' => [['term1', str_repeat(".", 300)], ['term1']] + ]; + } + } From f2d26b709d85b946d63f7f0f4f38fbf54e9612f7 Mon Sep 17 00:00:00 2001 From: Tom Needham Date: Tue, 30 May 2017 09:46:55 +0100 Subject: [PATCH 2/5] Extend expression builder to include character length function --- core/Migrations/Version20170526104128.php | 5 ++++- .../ExpressionBuilder/ExpressionBuilder.php | 11 +++++++++++ .../ExpressionBuilder/MySqlExpressionBuilder.php | 9 +++++++++ lib/public/DB/QueryBuilder/IExpressionBuilder.php | 7 +++++++ 4 files changed, 31 insertions(+), 1 deletion(-) diff --git a/core/Migrations/Version20170526104128.php b/core/Migrations/Version20170526104128.php index 9ee2fa5b3f4e..560a6466d8a5 100644 --- a/core/Migrations/Version20170526104128.php +++ b/core/Migrations/Version20170526104128.php @@ -25,7 +25,10 @@ public function changeSchema(Schema $schema, array $options) { /** @var IDBConnection $db */ $db = \OC::$server->getDatabaseConnection(); - $db->executeQuery("DELETE FROM {$prefix}account_terms WHERE CHAR_LENGTH(term) >= 256;"); + $qb = $db->getQueryBuilder(); + $qb->delete('account_terms') + ->where($qb->expr()->gte($qb->expr()->length('term'), new Literal(256))); + $qb->execute(); // Now update the column length $table->getColumn('term')->setLength(255); diff --git a/lib/private/DB/QueryBuilder/ExpressionBuilder/ExpressionBuilder.php b/lib/private/DB/QueryBuilder/ExpressionBuilder/ExpressionBuilder.php index 85ecda2c3b55..cf40f82133a5 100644 --- a/lib/private/DB/QueryBuilder/ExpressionBuilder/ExpressionBuilder.php +++ b/lib/private/DB/QueryBuilder/ExpressionBuilder/ExpressionBuilder.php @@ -28,6 +28,7 @@ use OC\DB\QueryBuilder\Literal; use OC\DB\QueryBuilder\QueryFunction; use OC\DB\QueryBuilder\QuoteHelper; +use OC\Diagnostics\Query; use OCP\DB\QueryBuilder\IExpressionBuilder; use OCP\IDBConnection; @@ -367,4 +368,14 @@ public function castColumn($column, $type) { $this->helper->quoteColumnName($column) ); } + + /** + * Returns a query function to find the number of characters in a string column + * @param string $column + * @return string + */ + public function length($column) { + $column = $this->helper->quoteColumnName($column); + return new QueryFunction("LENGTH({$column})"); + } } diff --git a/lib/private/DB/QueryBuilder/ExpressionBuilder/MySqlExpressionBuilder.php b/lib/private/DB/QueryBuilder/ExpressionBuilder/MySqlExpressionBuilder.php index 845a55aad87d..2eff28795e9e 100644 --- a/lib/private/DB/QueryBuilder/ExpressionBuilder/MySqlExpressionBuilder.php +++ b/lib/private/DB/QueryBuilder/ExpressionBuilder/MySqlExpressionBuilder.php @@ -38,4 +38,13 @@ public function iLike($x, $y, $type = null) { return $this->expressionBuilder->comparison($x, " COLLATE {$characterSet}_general_ci LIKE", $y); } + /** + * Use CHAR_LENGTH on MySQL to return number of characters not bytes. + * @inheritdoc + */ + public function length($column) { + $column = $this->helper->quoteColumnName($column); + return new QueryFunction("CHAR_LENGTH({$column})"); + } + } diff --git a/lib/public/DB/QueryBuilder/IExpressionBuilder.php b/lib/public/DB/QueryBuilder/IExpressionBuilder.php index ec05cf834225..11ddfeca3d13 100644 --- a/lib/public/DB/QueryBuilder/IExpressionBuilder.php +++ b/lib/public/DB/QueryBuilder/IExpressionBuilder.php @@ -324,4 +324,11 @@ public function literal($input, $type = null); * @since 9.0.0 */ public function castColumn($column, $type); + + /** + * @param $column + * @return string + * @since 10.0.3 + */ + public function length($column); } From 729a237a67c0f8c2320a5c6487efc18bae827eee Mon Sep 17 00:00:00 2001 From: Tom Needham Date: Tue, 30 May 2017 09:51:24 +0100 Subject: [PATCH 3/5] Use LENGTHC function on oracle for length query function --- .../ExpressionBuilder/OCIExpressionBuilder.php | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lib/private/DB/QueryBuilder/ExpressionBuilder/OCIExpressionBuilder.php b/lib/private/DB/QueryBuilder/ExpressionBuilder/OCIExpressionBuilder.php index 8a771ead4b6a..6b08a5bf4d37 100644 --- a/lib/private/DB/QueryBuilder/ExpressionBuilder/OCIExpressionBuilder.php +++ b/lib/private/DB/QueryBuilder/ExpressionBuilder/OCIExpressionBuilder.php @@ -160,4 +160,13 @@ public function iLike($x, $y, $type = null) { $y = $this->helper->quoteColumnName($y); return new QueryFunction('REGEXP_LIKE('.$x.', \'^\' || REPLACE('.$y.', \'%\', \'.*\') || \'$\', \'i\')'); } + + /** + * Use LENGTHC on Oracle to return multi-byte safe number of characters not bytes. + * @inheritdoc + */ + public function length($column) { + $column = $this->helper->quoteColumnName($column); + return new QueryFunction("LENGTHC({$column})"); + } } From 5d9d79e23c6b10ca47deebc1d2bdba58dd7b28e0 Mon Sep 17 00:00:00 2001 From: Tom Needham Date: Mon, 5 Jun 2017 16:13:45 +0200 Subject: [PATCH 4/5] Handle 4 byte database encodings, max key length 191 chars --- core/Migrations/Version20170526104128.php | 35 +++++++++++++++++++---- lib/private/User/User.php | 8 +++--- tests/lib/User/UserTest.php | 2 +- 3 files changed, 34 insertions(+), 11 deletions(-) diff --git a/core/Migrations/Version20170526104128.php b/core/Migrations/Version20170526104128.php index 560a6466d8a5..401537f11c77 100644 --- a/core/Migrations/Version20170526104128.php +++ b/core/Migrations/Version20170526104128.php @@ -12,25 +12,48 @@ class Version20170526104128 implements ISchemaMigration { public function changeSchema(Schema $schema, array $options) { + // Get the table $prefix = $options['tablePrefix']; $table = $schema->getTable("{$prefix}account_terms"); + // Check column length - if($table->getColumn('term')->getLength() === 255) { + if($table->getColumn('term')->getLength() === 191) { // we don't need to adjust it return; } // Need to shorten the column by one character - // Check if we have any terms taking up the full 256 chars (unlikely) + // Check if we have any terms taking up 192 or more chars (unlikely) /** @var IDBConnection $db */ $db = \OC::$server->getDatabaseConnection(); + + $qb = $db->getQueryBuilder(); - $qb->delete('account_terms') - ->where($qb->expr()->gte($qb->expr()->length('term'), new Literal(256))); - $qb->execute(); + $qb->select(['id', 'term']) + ->from('account_terms') + ->where($qb->expr()->gte($qb->expr()->length('term'), new Literal(192))); + $results = $qb->execute(); + + // Now shorten these terms + $db->beginTransaction(); + while($longTerm = $results->fetch()) { + $qb->update('account_terms') + ->where($qb->expr()->eq('id', $qb->createNamedParameter($longTerm['id']))) + ->set('term', $qb->createNamedParameter($this->trimTerm($longTerm['term']))) + ->execute(); + } + $db->commit(); // Now update the column length - $table->getColumn('term')->setLength(255); + $table->getColumn('term')->setLength(191); } + + /** + * @param $longTerm + * @return string the shortened string ready for the new db column + */ + public function trimTerm($longTerm) { + return (string) substr($longTerm, 0, 191); + } } diff --git a/lib/private/User/User.php b/lib/private/User/User.php index 50b186959deb..0f8b57bddad5 100644 --- a/lib/private/User/User.php +++ b/lib/private/User/User.php @@ -456,10 +456,10 @@ public function getSearchTerms() { * @since 10.0.1 */ public function setSearchTerms(array $terms) { - // Check length of terms - $terms = array_filter($terms, function($term) { - return strlen($term) <= 255; - }); + // Check length of terms, cut if too long + $terms = array_map(function($term) { + return substr($term, 0, 191); + }, $terms); $this->mapper->setTermsForAccount($this->account->getId(), $terms); } } diff --git a/tests/lib/User/UserTest.php b/tests/lib/User/UserTest.php index 29bebc1984ea..1af0ea725dd8 100644 --- a/tests/lib/User/UserTest.php +++ b/tests/lib/User/UserTest.php @@ -337,7 +337,7 @@ public function testSettingAccountTerms(array $terms, array $expected) { public function setTermsData() { return [ 'normal terms' => [['term1'], ['term1']], - 'too long terms' => [['term1', str_repeat(".", 300)], ['term1']] + 'too long terms' => [['term1', str_repeat(".", 192)], ['term1']] ]; } From 8eaa630ce15d6d57b11e674d611a799531d91e60 Mon Sep 17 00:00:00 2001 From: Tom Needham Date: Wed, 7 Jun 2017 13:46:18 +0200 Subject: [PATCH 5/5] Add test for truncating of user account search terms --- tests/lib/User/UserTest.php | 2 +- version.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/lib/User/UserTest.php b/tests/lib/User/UserTest.php index 1af0ea725dd8..89993a6167ce 100644 --- a/tests/lib/User/UserTest.php +++ b/tests/lib/User/UserTest.php @@ -337,7 +337,7 @@ public function testSettingAccountTerms(array $terms, array $expected) { public function setTermsData() { return [ 'normal terms' => [['term1'], ['term1']], - 'too long terms' => [['term1', str_repeat(".", 192)], ['term1']] + 'too long terms' => [['term1', str_repeat(".", 192)], ['term1', str_repeat(".", 191)]] ]; } diff --git a/version.php b/version.php index 05c9bc9e25d4..66f94445ccb7 100644 --- a/version.php +++ b/version.php @@ -25,7 +25,7 @@ // We only can count up. The 4. digit is only for the internal patchlevel to trigger DB upgrades // between betas, final and RCs. This is _not_ the public version number. Reset minor/patchlevel // when updating major/minor version number. -$OC_Version = [10, 0, 2, 1]; +$OC_Version = [10, 0, 2, 3]; // The human readable string $OC_VersionString = '10.0.2';