GetBetaFeaturePreferences hook doesn't validate all the entries against $wgBetaFeaturesWhitelist
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	Jdlrobson
	Dec 11 2015, 1:10 AM

Description

I can register a beta feature in this hook that is not in $wgBetaFeaturesWhitelist but it still shows when I visit Special:Preferences despite not working.

Expected:
When a feature is not whitelisted it should not show up in beta features.

Details

	Subject	Repo	Branch	Lines +/-
	BetaFeatures only add whitelisted preferences	mediawiki/extensions/BetaFeatures	master	+17 -15

Customize query in gerrit

Event Timeline

Jdlrobson created this task.Dec 11 2015, 1:10 AM

Jdlrobson raised the priority of this task from to Needs Triage.

Jdlrobson updated the task description. (Show Details)

Jdlrobson added a project: BetaFeatures.

Jdlrobson subscribed.

Restricted Application added subscribers: StudiesWorld, Aklapper. · View Herald TranscriptDec 11 2015, 1:10 AM

I'm moderately sure this is a regression, but I can't say how.

if (
		is_array( $wgBetaFeaturesWhitelist ) &&
		!in_array( $key, $wgBetaFeaturesWhitelist )
	) {
	$success = false;
} elseif ( isset( $depHooks[$key] ) ) {
	$success = Hooks::run( $depHooks[$key] );
}

if ( $success !== true ) {
	// Skip this preference!
	continue;
}

Thoughts as to why this doesn't work any more appreciated.

In T121182#1872059, @Jdforrester-WMF wrote:

if (
		is_array( $wgBetaFeaturesWhitelist ) &&
		!in_array( $key, $wgBetaFeaturesWhitelist )
	) {
	$success = false;
} elseif ( isset( $depHooks[$key] ) ) {
	$success = Hooks::run( $depHooks[$key] );
}

if ( $success !== true ) {
	// Skip this preference!
	continue;
}

Thoughts as to why this doesn't work any more appreciated.

The above block is encolsed in

if ( isset( $info['dependent'] ) && $info['dependent'] === true ) {

Clearly, if dependencies are not given, whitelist check is completely bypassed.

Change 258418 had a related patch set uploaded (by Sumit):
BetaFeatures only add whitelisted preferences

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

gerritbot added a project: Patch-For-Review.Dec 11 2015, 4:23 AM

In T121182#1872144, @Sumit wrote:

In T121182#1872059, @Jdforrester-WMF wrote:

Thoughts as to why this doesn't work any more appreciated.

The above block is encolsed in

if ( isset( $info['dependent'] ) && $info['dependent'] === true ) {

Clearly, if dependencies are not given, whitelist check is completely bypassed.

Well, don't I feel a fool. :-)

Change 258418 merged by jenkins-bot:
BetaFeatures only add whitelisted preferences

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

Jdforrester-WMF closed this task as Resolved.Dec 12 2015, 1:09 AM

Jdforrester-WMF assigned this task to Sumit.

Jdforrester-WMF edited projects, added MW-1.27-release (WMF-deploy-2015-12-15_(1.27.0-wmf.9)); removed Patch-For-Review.

Jdforrester-WMF set Security to None.

GetBetaFeaturePreferences hook doesn't validate all the entries against $wgBetaFeaturesWhitelistClosed, ResolvedPublicActions

Description

Details

Event Timeline

GetBetaFeaturePreferences hook doesn't validate all the entries against $wgBetaFeaturesWhitelist
Closed, ResolvedPublic
Actions