Gergő Tisza has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/390136 )
Change subject: Verify string length before inserting into the database
......................................................................
Verify string length before inserting into the database
Depending on configuration settings, the DB might quietly truncate
long values. Failing loudly is preferable.
Change-Id: Ia0bc3c11c30e60db96fba7bc001e34ec03b79273
---
M i18n/en.json
M i18n/qqq.json
M src/ReadingListRepository.php
M tests/src/ReadingListRepositoryTest.php
4 files changed, 44 insertions(+), 0 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/ReadingLists
refs/changes/36/390136/1
diff --git a/i18n/en.json b/i18n/en.json
index f8ee29d..afa13e0 100644
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -23,6 +23,7 @@
"readinglists-db-error-user-required": "This method cannot be called
without specifying the user.",
"readinglists-db-error-list-limit": "Users cannot have more than $1
lists.",
"readinglists-db-error-entry-limit": "List $1 cannot have more than $2
entries.",
+ "readinglists-db-error-too-long": "Value for field $1 cannot be longer
than $2 bytes.",
"readinglists-apierror-project-title-param": "<var>$1project</var> and
<var>$1title</var> must be used together.",
"readinglists-apierror-too-old": "Timestamps passed to
<var>$1changedsince</var> cannot be older than <kbd>$2</kbd>.",
"apihelp-query+readinglists-summary": "List or filter the user's
reading lists and show metadata about them.",
diff --git a/i18n/qqq.json b/i18n/qqq.json
index 6c00bd5..8414fa4 100644
--- a/i18n/qqq.json
+++ b/i18n/qqq.json
@@ -24,6 +24,7 @@
"readinglists-db-error-user-required": "Error message used when calling
a method that operates on a single user, but the user was not specified when
the repository object was constructed.",
"readinglists-db-error-list-limit": "Error message used when the user
has as many or more lists than the maximum allowed, and tries to add another
one.\n\nParameters:\n* $1 - the maximum allowed number of lists per user.",
"readinglists-db-error-entry-limit": "Error message used when the user
has as many or more entries in the given list than the maximum allowed, and
tries to add another one.\n\nParameters:\n* $1 - the ID of the list in
question.\n$2 - the maximum allowed number of entries per list.",
+ "readinglists-db-error-too-long": "Error message used when the user
tries to set list/entry string fields to a longer value than what the database
schema allows.\n\nParameters:\n* $1 - DB field name.\n$2 - DB field maximum
length (in bytes).",
"readinglists-apierror-project-title-param": "{{doc-apierror}}\n$1 is
the module prefix.",
"readinglists-apierror-too-old": "{{doc-apierror}}\n$1 is the module
prefix, $2 is the expiry date for deleted lists/entries.",
"apihelp-query+readinglists-summary":
"{{doc-apihelp-summary|query+readinglists}}",
diff --git a/src/ReadingListRepository.php b/src/ReadingListRepository.php
index a2dc7b2..dcf499b 100644
--- a/src/ReadingListRepository.php
+++ b/src/ReadingListRepository.php
@@ -32,6 +32,17 @@
*/
class ReadingListRepository implements IDBAccessObject, LoggerAwareInterface {
+ /** @var array Database field lengths in bytes (only for the string
types). */
+ public static $fieldLength = [
+ 'rl_name' => 255,
+ 'rl_description' => 767,
+ 'rl_color' => 6,
+ 'rl_image' => 255,
+ 'rl_icon' => 32,
+ 'rle_project' => 255,
+ 'rle_title' => 255,
+ ];
+
/** @var int|null Max allowed lists per user */
private $listLimit;
@@ -191,6 +202,11 @@
*/
public function addList( $name, $description = '', $color = '', $image
= '', $icon = '' ) {
$this->assertUser();
+ $this->assertFieldLength( 'rl_name', $name );
+ $this->assertFieldLength( 'rl_description', $description );
+ $this->assertFieldLength( 'rl_color', $color );
+ $this->assertFieldLength( 'rl_image', $image );
+ $this->assertFieldLength( 'rl_icon', $icon );
if ( !$this->isSetupForUser( self::READ_LOCKING ) ) {
throw new ReadingListRepositoryException(
'readinglists-db-error-not-set-up' );
}
@@ -274,6 +290,11 @@
$id, $name = null, $description = null, $color = null, $image =
null, $icon = null
) {
$this->assertUser();
+ $this->assertFieldLength( 'rl_name', $name );
+ $this->assertFieldLength( 'rl_description', $description );
+ $this->assertFieldLength( 'rl_color', $color );
+ $this->assertFieldLength( 'rl_image', $image );
+ $this->assertFieldLength( 'rl_icon', $icon );
$this->selectValidList( $id, self::READ_LOCKING );
$data = array_filter( [
@@ -340,6 +361,8 @@
*/
public function addListEntry( $id, $project, $title ) {
$this->assertUser();
+ $this->assertFieldLength( 'rle_project', $project );
+ $this->assertFieldLength( 'rle_title', $title );
$this->selectValidList( $id, self::READ_LOCKING );
if ( $this->entryLimit && $this->getEntryCount( $id,
self::READ_LATEST ) >= $this->entryLimit ) {
throw new ReadingListRepositoryException(
'readinglists-db-error-entry-limit',
@@ -999,6 +1022,21 @@
}
/**
+ * Ensures that the value to be written to the database does not exceed
the DB field length.
+ * @param string $field Field name.
+ * @param string $value Value to write.
+ * @throws ReadingListRepositoryException
+ */
+ private function assertFieldLength( $field, $value ) {
+ if ( !isset( self::$fieldLength[$field] ) ) {
+ throw new LogicException( 'Tried to assert length for
invalid field ' . $field );
+ }
+ if ( strlen( $value ) > self::$fieldLength[$field] ) {
+ throw new ReadingListRepositoryException(
'readinglists-db-error-too-long',
+ [ $field, self::$fieldLength[$field] ] );
+ }
+ }
+ /**
* Get list data, and optionally lock the list.
* List must exist, belong to the current user and not be deleted.
* @param int $id List id
diff --git a/tests/src/ReadingListRepositoryTest.php
b/tests/src/ReadingListRepositoryTest.php
index d2e8ef6..e6d4e12 100644
--- a/tests/src/ReadingListRepositoryTest.php
+++ b/tests/src/ReadingListRepositoryTest.php
@@ -178,6 +178,10 @@
'rl_is_default' => '0',
'rl_deleted' => '0',
], $data );
+
+ $this->assertFailsWith( 'readinglists-db-error-too-long',
function () use ( $repository ) {
+ $repository->addList( 'boom', str_pad( '', 1000, 'x' )
);
+ } );
}
// @codingStandardsIgnoreLine
MediaWiki.NamingConventions.LowerCamelFunctionsName.FunctionName
--
To view, visit https://gerrit.wikimedia.org/r/390136
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia0bc3c11c30e60db96fba7bc001e34ec03b79273
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/ReadingLists
Gerrit-Branch: master
Gerrit-Owner: Gergő Tisza <gti...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
