function user_login_name_validate($form, &$form_state) {
  if (isset($form_state['values']['name']) && user_is_blocked($form_state['values']['name'])) {

but in user_login_authenticate_validate():

  if (!empty($form_state['values']['name']) && !empty($password)) {

and user_authenticate(), which gets given that value uses empty() too:

function user_authenticate($name, $password) {
  $uid = FALSE;
  if (!empty($name) && !empty($password)) {

This creates an inconsistency which makes it difficult for custom/contrib modules to do form alteration around user login (eg http://drupal.org/project/email_registration).

On D7, there is also user_account_form_validate() which uses !empty().

Files: 
CommentFileSizeAuthor
#5 use-empty-1996644-5.patch748 bytesapkwilson
PASSED: [[SimpleTest]]: [MySQL] 40,088 pass(es). View
#1 use_empty-1996644.patch793 bytesabghosh82
PASSED: [[SimpleTest]]: [MySQL] 55,895 pass(es). View

Comments

abghosh82’s picture

Assigned: Unassigned » abghosh82
Status: Active » Needs review
FileSize
793 bytes
PASSED: [[SimpleTest]]: [MySQL] 55,895 pass(es). View

I agree using !empty as it covers isset as well as existence of a variable.

Submiting a patch, see if this is what you have meant.

fjd’s picture

Status: Needs review » Reviewed & tested by the community

Patch applied cleanly. Verified isset($form_state['values']['name']) has been changed to !empty($form_state['values']['name']) in /core/modules/user/user.module.

Created new user and blocked them. Verified user sees the error message when they try to log on.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Consistency++

Committed 8cc7520 and pushed to 8.x. Thanks!

alexpott’s picture

Version: 8.x-dev » 7.x-dev
Status: Fixed » Patch (to be ported)

Setting appropriate statuses

apkwilson’s picture

Status: Patch (to be ported) » Needs review
FileSize
748 bytes
PASSED: [[SimpleTest]]: [MySQL] 40,088 pass(es). View

Patch ported (if I did it right, anyway).

lauriii’s picture

Patch applies and works

joachim’s picture

Status: Needs review » Reviewed & tested by the community

Thanks for reviewing.

David_Rothstein’s picture

Issue summary: View changes
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.