enwikipedia-acc / waca

English Wikipedia Account Creation Interface
https://accounts.wmflabs.org/internal.php
The Unlicense
33 stars 30 forks source link

Exception on Antispoof cache save #713

Closed stwalkerster closed 1 year ago

stwalkerster commented 2 years ago
SQLSTATE[22001]: String data, right truncated: 1406 Data too long for column 'username' at row 1

#0 ./includes/DataObjects/AntiSpoofCache.php(110): PDOStatement->execute()
#1 ./includes/Providers/CachedApiAntispoofProvider.php(61): Waca\DataObjects\AntiSpoofCache->save()
#2 ./includes/Validation/RequestValidationHelper.php(251): Waca\Providers\CachedApiAntispoofProvider->getSpoofs(...)

Probably just need to truncate on the sql inserts and selects.

methecooldude commented 2 years ago

Is it worth just changing the SQL schema to allow up to 85 characters in the username field per https://en.wikipedia.org/wiki/Wikipedia:Naming_conventions_(technical_restrictions)#Restrictions_on_usernames 'It may not be more than 85 bytes long.'. I'm aware of Unicode using more bytes, but what are the chances of a request that long?

stwalkerster commented 2 years ago
MariaDB [production]> show create table antispoofcache;
+----------------+---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| Table          | Create Table                                                                                                                                                                                                                                                                                                                                                                                                                                      |
+----------------+---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| antispoofcache | CREATE TABLE `antispoofcache` (
  `id` int(11) unsigned NOT NULL AUTO_INCREMENT,
  `username` varchar(255) COLLATE utf8mb4_unicode_520_ci NOT NULL,
  `data` text COLLATE utf8mb4_unicode_520_ci NOT NULL,
  `timestamp` timestamp NOT NULL DEFAULT current_timestamp(),
  `updateversion` int(11) unsigned NOT NULL DEFAULT 0,
  PRIMARY KEY (`id`)
) ENGINE=InnoDB AUTO_INCREMENT=453727 DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_520_ci |
+----------------+---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
1 row in set (0.001 sec)

It's already 255 characters long in a (real) UTF-8 character set.

The username column is the username from the request itself, not the API response.

methecooldude commented 2 years ago

Oh, Rich needs to read properly... I thought it was coming from WP... heh

stwalkerster commented 1 year ago

Since request.username is a VARCHAR(512) and there's already length validation on the request form for that, I'm sorely tempted to just bump antispoofcache.username to VARCHAR(512) too.

I'll https://github.com/enwikipedia-acc/waca/labels/good%20first%20issue too, as this should just be a case of writing a small database patch file (follow the guidance in the example ./sql/patches/patch00-example.sql) and bumping the latest schema version number in SiteConfiguration.php.

The necessary SQL to change the table in the patch file will be something like:

ALTER TABLE antispoofcache MODIFY COLUMN username VARCHAR(512) NOT NULL;