See T203554 for a longer explanation. Throttle count or, more likely, throttle period may have a comma inside (being numerical values). However this may create problems since we currently implode the two values with a comma, thus leading to potential internal failures. The linked task will make us stop imploding such values, but existing ones (if any) must be specifically handled within the code, possibly before merging the patch for T203554 (as the implementation takes the presence of a comma a a sign that parameters were imploded).
When we'll be sure that this won't be a problem, we will be able to go on with the linked task.
Description
Description
Details
Details
Customize query in gerrit
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Open | None | T209023 Major overhaul for AbuseFilter | |||
| Open | None | T203587 Major overhaul for "throttle" action in AbuseFilter | |||
| Resolved | Daimona | T203585 Throttle parameters may have an undesired comma inside | |||
| Resolved | Urbanecm | T209565 Dry run for normalizeThrottleParameters.php |
Event Timeline
Comment Actions
AFAICS, having a comma is only a problem if it's inside the throttle count. In several points of the code we do something like
list( $count, $period ) = explode( ',', $parameters );
And if, for instance, the specified count is "3,5" and the specified period is "300", the variables from the example above will be $count === 3 and period === 5. Now, this is really, really unlikely to happen. First because the action count is naturally an integer. Second because it could only happen where the comma is the decimal separator. In any case, having a non-integer is totally wrong. I guess values with commas can be just rounded to the closest integer, and we'd better do it inside the maintenance script for T203336.
EDIT: Actually, prior to the OOUI switch, trying to put a comma inside those fields produced some kind of errors, and throttle parameters weren't saved. Given this, I'll just add a check to the mentioned script for commas, throwing a fatalError if parameters aren't fine.
Comment Actions
Change 457086 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Fix throttle validation to avoid DB query error
Comment Actions
Change 457086 abandoned by Daimona Eaytoy:
Fix throttle validation to avoid DB query error
Reason:
Superseded by I7831dbb0bab55807392ac1f7915d6cb0cb713593. The few things of the maintenance script that we still need will be included inside the one in Iee56f4726337a42a04e7912e1ee67738fac0b488
Comment Actions
Change 459368 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Change throttle selector to restore old functionality, overall improvement
Comment Actions
Change 468007 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@REL1_32] Change throttle selector to restore old functionality, overall improvement
Comment Actions
Change 459368 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Change throttle selector to restore old functionality, overall improvement
Comment Actions
Change 459245 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Add normalizeThrottleParameters script to update.php
Comment Actions
Change 468007 merged by Daimona Eaytoy:
[mediawiki/extensions/AbuseFilter@REL1_32] Change throttle selector to restore old functionality, overall improvement
Comment Actions
Change 459245 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Add normalizeThrottleParameters script to update.php
Comment Actions
Change 459245 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Add normalizeThrottleParameters script to update.php