Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

obey accounts_data column length when inserting and searching #29926

Merged
merged 2 commits into from
Nov 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion lib/private/Accounts/AccountManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,10 @@ protected function getUser(IUser $user, bool $insertIfNotExists = true): array {
}

public function searchUsers(string $property, array $values): array {
// the value col is limited to 255 bytes. It is used for searches only.
$values = array_map(function (string $value) {
return Util::shortenMultibyteString($value, 255);
}, $values);
$chunks = array_chunk($values, 500);
$query = $this->connection->getQueryBuilder();
$query->select('*')
Expand Down Expand Up @@ -625,8 +629,11 @@ protected function writeUserDataProperties(IQueryBuilder $query, array $data): v
continue;
}

// the value col is limited to 255 bytes. It is used for searches only.
$value = $property['value'] ? Util::shortenMultibyteString($property['value'], 255) : '';

$query->setParameter('name', $property['name'])
->setParameter('value', $property['value'] ?? '');
->setParameter('value', $value);
$query->executeStatement();
}
}
Expand Down
24 changes: 24 additions & 0 deletions lib/public/Util.php
Original file line number Diff line number Diff line change
Expand Up @@ -513,4 +513,28 @@ public static function needUpgrade() {
}
return self::$needUpgradeCache;
}

/**
* Sometimes a string has to be shortened to fit within a certain maximum
* data length in bytes. substr() you may break multibyte characters,
* because it operates on single byte level. mb_substr() operates on
* characters, so does not ensure that the shortend string satisfies the
* max length in bytes.
*
* For example, json_encode is messing with multibyte characters a lot,
* replacing them with something along "\u1234".
*
* This function shortens the string with by $accurancy (-5) from
* $dataLength characters, until it fits within $dataLength bytes.
*
* @since 23.0.0
*/
public static function shortenMultibyteString(string $subject, int $dataLength, int $accuracy = 5): string {
$temp = mb_substr($subject, 0, $dataLength);
// json encodes encapsulates the string in double quotes, they need to be substracted
while ((strlen(json_encode($temp)) - 2) > $dataLength) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are there scenarios where the while loop never ends ? maybe check if $oldTemp != $temp ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, when you provide $accuracy with <= $dataLength * -1

$temp = mb_substr($temp, 0, -$accuracy);
}
return $temp;
}
}
7 changes: 7 additions & 0 deletions tests/lib/UtilTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -310,4 +310,11 @@ public function testAddVendorStyle() {
'myApp/vendor/myFancyCSSFile2',
], \OC_Util::$styles);
}

public function testShortenMultibyteString() {
$this->assertEquals('Short nuff', \OCP\Util::shortenMultibyteString('Short nuff', 255));
$this->assertEquals('ABC', \OCP\Util::shortenMultibyteString('ABCDEF', 3));
// each of the characters is 12 bytes
$this->assertEquals('🙈', \OCP\Util::shortenMultibyteString('🙈🙊🙉', 16, 2));
}
}