Page MenuHomePhabricator

SimpleCaptcha::loadText() loads section 0 when passed false, as all callers do
Closed, ResolvedPublic

Description

Looks like rECOE800f6cb1b8f4: Simplify function Captcha::loadText for readability introduced a bug in SimpleCaptcha::loadText() where passing false for $section (as the calling code path via confirmEditMerged() does, despite @param documentation specifying strings) would start loading section 0 instead of using the whole page.

That means the $wgCaptchaRegex check where it tries to count only added instances of the regex will not work right because the $oldtext contains only the lead section while $newtext contains the whole page content.

This might be fixed by adjusting the check, or by fixing confirmEditMerged() to pass a string rather than false.

See https://en.wikipedia.org/w/index.php?title=Wikipedia:Village_pump_(technical)&oldid=873420998#And_why_a_CAPTCHA?

Event Timeline

Anomie updated the task description. (Show Details)

Changing the if statement in the loadText function back to != instead of !== seems to fix it.

if ( $section != '' ) {

@Porplemontage: Thanks for taking a look at the code!
You are very welcome to use developer access to submit the proposed code changes as a Git branch directly into Gerrit which makes it easier to review them quickly and provide feedback. If you don't want to set up Git/Gerrit, you can also use the Gerrit Patch Uploader. Thanks again!

Change 511960 had a related patch set uploaded (by Porplemontage; owner: Porplemontage):
[mediawiki/extensions/ConfirmEdit@master] Fix bug in Captcha::loadText which broke the $wgCaptchaRegex check

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

Florian assigned this task to Porplemontage.
Florian removed a project: Patch-For-Review.

Change 511960 merged by jenkins-bot:
[mediawiki/extensions/ConfirmEdit@master] Fix bug in Captcha::confirmEditMerged which breaks the $wgCaptchaRegex check

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