Page MenuHomePhabricator

The code in HTMLUserTextField, as used on Special:Block, would incorrectly check IP ranges (if it worked)
Closed, ResolvedPublic

Description

The code in HTMLUserTextField, as used on Special:Block, would incorrectly check IP ranges (if it worked). This is a regression from https://gerrit.wikimedia.org/r/#/c/233762/ (rMWb73c0c4d3fc9: Enable IP ranges in HTMLUserTextField) + https://gerrit.wikimedia.org/r/#/c/218281/ (rMWd56758e13435: SpecialBlock: Switch to OOUI form).

In a rare case of three wrongs making a right, several separate issues cancel each other out so the page is not broken in practice currently.

  • SpecialBlock does not pass the 'iprangelimits' parameter in HTMLForm descriptor for the 'Target' field. This means that hardcoded value of [ 'IPv4' => '16', 'IPv6' => '32' ] is used instead of $wgBlockCIDRLimit, defined as [ 'IPv4' => 16, 'IPv6' => 19 ]. This would prevent blocks of IPv6 ranges smaller than /19 but greater than /32, which should be allowed.
  • The condition in HTMLUserTextField that checks the range limits (lines 44-49) does not work with the parameters that SpecialBlock uses. Specifically, if 'exists' => false is given (this is the default), all the other checks (e.g. for IP range) are skipped. I'm not sure what this should be, but it is definitely wrong. It should probably be split into a few different checks.
  • The code in SpecialBlock::validateTarget() (Block::TYPE_RANGE case) is currently correct, but it is a duplicate of HTMLUserTextField::isValidIPRange() and should be removed once the above issues are both fixed.

Event Timeline

Hmm... Should this code be primarily in HTMLUserTextField? BlockUser would also need to validate block targets, but that's a service - I don't think services should call HTMLUserTextField. So, what about creating a service, conceptually similar to BlockPermissionChecker?

Change 813727 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):

[mediawiki/core@master] HTMLUserTextField: Fix validation

https://gerrit.wikimedia.org/r/813727

Change 813727 merged by jenkins-bot:

[mediawiki/core@master] HTMLUserTextField: Fix validation

https://gerrit.wikimedia.org/r/813727