Page MenuHomePhabricator

Allow more specific error messages in SubmitControl::validateFields()
Closed, ResolvedPublic

Description

The SubmitControl code currently basically only does the below

			$valid = is_string( $validator ) // regex
				? preg_match( $validator, $this->vals[$field] )
				: $validator( $this->vals[$field], $this->vals );
			if ( !$valid ) {
				// @TODO: check for field-specific message first
				return $this->failure( "invalid_field_$field", 'mwoauth-invalid-field', $field );
			}

Would be nice if things could return Status objects, wrapping error messages... Which is better than these two error messages that are used for all cases. Which tells you something is wrong, but gives you absolutely no clue what it might be

	"mwoauth-invalid-field": "Invalid value provided for \"$1\" field",
	"mwoauth-invalid-field-generic": "Invalid value provided",

Useful for T277387: Use composer/semver to validate oauth registration versions, T277388: OAuth doesn't validate length of oarc_version (CVE-2021-31555)... And various other related tasks. {T260635}, T277379: OAuth doesn't validate length of oarc_grants and oarc_oauth2_allowed_grants JSON, {T277380}.

Similar what was also done for T260631: BotPasswords doesn't validate length of resultant bp_restrictions JSON and T260633: BotPasswords doesn't validate length of resultant bp_grants JSON.


See also: T302764: Add mwoauth-(invalid|missing)-field-$fieldName messages

Event Timeline

Reedy renamed this task from Allow more specific error messages in SubmitControl to Allow more specific error messages in SubmitControl::validateFields().Dec 27 2022, 3:26 AM

There's some overlap here with T302764: Add mwoauth-(invalid|missing)-field-$fieldName messages... Though that code path does allow for a lot more error messages (invalid and missing for each field name), but we just haven't defined any of the messages to be used...

Ideally, we could use the same/similar error handling, and use the sets of messages for both...