Page MenuHomePhabricator

Block and Nuke broken in REL1_27 branch due to whitelist truncation - please backport patch from master
Closed, ResolvedPublic

Description

The whitelist is truncated at 200 characters, some testing code got left in it seems, which completely breaks this extension:
BanPests.php:
$file = fread($fh,200);

Event Timeline

iank created this task.Aug 21 2017, 8:02 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 21 2017, 8:02 AM
iank added a comment.EditedAug 21 2017, 8:18 AM

This was fixed on 2017-5-31 in master by niharika29. Until then, no one
had used this extension on any site with a user list more than 200 bytes
long or else they accidentally banned and nuked legitimate
users. (edit) it would have been nice if this had originally been flagged as
critical and backported to
the stable branch and all other branches. Also, based on all this, there
is no way this extension should be described as "Release status: stable"
on https://www.mediawiki.org/wiki/Extension:BlockAndNuke.

@iank: Thanks for reporting this. Please criticize ideas, not people.
Niharika29 also had the freedom to ignore https://www.mediawiki.org/w/index.php?title=Topic:S27v0luobpyhyjl9&topic_showPostId=trjfnwig076af8hp#flow-post-trjfnwig076af8hp and not contribute a patch to the master branch at all. Would that have been better? I doubt.
You (or anyone else who cares about other branches) are free to propose backport patches of https://gerrit.wikimedia.org/r/#/c/356362/ in Gerrit for older branches.
You are also free to propose changes to wiki page content on the corresponding talk page.
Thanks for your understanding! :)

Aklapper renamed this task from Block and Nuke broken in REL1_27 branch due to whitelist truncation to Block and Nuke broken in REL1_27 branch due to whitelist truncation - please backport patch from master.Aug 21 2017, 10:33 AM
iank added a comment.Sep 22 2017, 8:48 PM

@Aklapper yes, sorry about that. I edited my original comment accordingly.

Change 379869 had a related patch set uploaded (by MarcoAurelio; owner: Niharika29):
[mediawiki/extensions/BlockAndNuke@REL1_28] Read whitelist using file_get_contents to read it all at once

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

Change 379871 had a related patch set uploaded (by MarcoAurelio; owner: Niharika29):
[mediawiki/extensions/BlockAndNuke@REL1_29] Read whitelist using file_get_contents to read it all at once

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

I've cherry-picked @Niharika merged patch to REL_1.28 and REL_1.29. Gerrit interface does not allow me to cherry-pick to REL_1.27 due to "merge conflict". I can try manually though, but never did such a thing so I don't promise anything.

Change 379904 had a related patch set uploaded (by MarcoAurelio; owner: Niharika29):
[mediawiki/extensions/BlockAndNuke@REL1_27] Read whitelist using file_get_contents to read it all at once

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

@iank @Niharika @Aklapper I think that the needed cherry-picks are ready, from REL1_27 to REL1_29. As for REL1_30, I think that'd be master so it doesn't let me do it. Please review and merge as appropriate. Currently I think only @MarkAHershberger is the only active person with +2 rights in the extension itself. Regards.

Any qualms in merging those REL* patches? If not, can anyone do that? There ain't active mantainers here.

Change 379871 merged by jenkins-bot:
[mediawiki/extensions/BlockAndNuke@REL1_29] Read whitelist using file_get_contents to read it all at once

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

Change 379869 merged by jenkins-bot:
[mediawiki/extensions/BlockAndNuke@REL1_28] Read whitelist using file_get_contents to read it all at once

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

Umherirrender closed this task as Resolved.Oct 2 2017, 10:27 AM
Umherirrender assigned this task to MarcoAurelio.
Umherirrender triaged this task as Normal priority.
Restricted Application added a project: User-MarcoAurelio. · View Herald TranscriptOct 2 2017, 10:27 AM

Change 379904 merged by jenkins-bot:
[mediawiki/extensions/BlockAndNuke@REL1_27] Read whitelist using file_get_contents to read it all at once

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