Page MenuHomePhabricator

Avoid checking for large revision count on action=delete if the user doesn't have delete permission
Open, MediumPublicBUG REPORT

Description

When doing action=delete on a page, MediaWiki is checking if the page has a large revision count, and displaying a warning to the user telling it can't delete the page because it has many revisions, but that message is being displayed even if the user has no delete permission (for example, being a not logged-in user or a normal user), which doesn't make sense.


Steps to reproduce:

Go to https://en.wikipedia.org/wiki/Special:MostRevisions , pick one page, go to the page history and change action=history by action=delete on the URL

Be sure to not be an admin, or simply access it while not logged in.


Expected results:

You get a message like "The action you have requested is limited to users in the group: Administrators."


Actual results:

In addition to the "The action you have requested is limited to users in the group: Administrators." message, you get a "This page's edit history may exceed 5,000 revisions. To prevent accidental disruption, its deletion is restricted to stewards. Please contact one for assistance. For past actions at this page, see its deletion log."


Version: 1.24rc
Severity: trivial
See Also:
https://bugzilla.wikimedia.org/show_bug.cgi?id=69556

Details

Reference
bz69562

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 22 2014, 3:30 AM
bzimport set Reference to bz69562.
bzimport added a subscriber: Unknown Object (MLST).

If anyone wants to work on this, the code in Article::delete specifies that

Deletion permission checks
$permissionErrors = $title->getUserPermissionsErrors( 'delete', $user );
if ( count( $permissionErrors ) ) {
	throw new PermissionsError( 'delete', $permissionErrors );
}

which, in call, results in a call to Title::getUserPermissionsErrors, defined as follows

Title permission checks
public function getUserPermissionsErrors(
	$action, $user, $rigor = PermissionManager::RIGOR_SECURE, $ignoreErrors = []
) {
	// TODO: this is for b/c, eventually will be removed
	if ( $rigor === true ) {
		$rigor = PermissionManager::RIGOR_SECURE; // b/c
	} elseif ( $rigor === false ) {
		$rigor = PermissionManager::RIGOR_QUICK; // b/c
	}
	return MediaWikiServices::getInstance()->getPermissionManager()
		->getPermissionErrors( $action, $user, $this, $rigor, $ignoreErrors );
}

finally, PermissionManager::getPermissionErrors calls PermissionManager::getPermissionErrorsInternal

Permission manager checks
public function getPermissionErrors(
	$action,
	User $user,
	LinkTarget $page,
	$rigor = self::RIGOR_SECURE,
	$ignoreErrors = []
) {
	$errors = $this->getPermissionErrorsInternal( $action, $user, $page, $rigor );
	// Remove the errors being ignored.
	foreach ( $errors as $index => $error ) {
		$errKey = is_array( $error ) ? $error[0] : $error;
		if ( in_array( $errKey, $ignoreErrors ) ) {
			unset( $errors[$index] );
		}
		if ( $errKey instanceof MessageSpecifier && in_array( $errKey->getKey(), $ignoreErrors ) ) {
			unset( $errors[$index] );
		}
	}
	return $errors;
}

PermissionManager::getPermissionErrorsInternal takes the parameters:

getPermissionErrorsInternal
private function getPermissionErrorsInternal(
	$action,
	User $user,
	LinkTarget $page,
	$rigor = self::RIGOR_SECURE,
	$short = false
)

if $short is true, once the first error is found is checks end, rather than finding all errors. However, getPermissionErrors has no mechanism to pass a value for short to getPermissionErrorsInternal, and so the internal function will always retrieve all applicable errors when called (indirectly) by Article::delete

Aklapper changed the subtype of this task from "Task" to "Bug Report".Feb 5 2022, 2:33 PM
Aklapper removed a subscriber: wikibugs-l-list.