Download & Extend

Clean up password reset form

Project:Drupal core
Version:6.x-dev
Component:user.module
Category:bug report
Priority:critical
Assigned:Zen
Status:closed (fixed)

Issue Summary

Attached patch:

  • Removes a now unnecessary #weight attribute.
  • Checks for e-mail address validity before performing a costly user_load.
  • Trims user input.
  • Removes a notice $account->uid when $account is not an object [no account found].
  • email should be e-mail.

-K

AttachmentSizeStatusTest resultOperations
pass-reset-form.patch1.72 KBIgnored: Check issue status.NoneNone

Comments

#1

Status:needs review» fixed

Works for me! Tested and committed.

#2

Category:task» bug report
Status:fixed» needs review

Hi,

I just noticed that my earlier patch does not address usernames using an e-mail address format. The attached patch fixes that.

Thanks,
-K

AttachmentSizeStatusTest resultOperations
password-reset-correction.patch1.12 KBIgnored: Check issue status.NoneNone

#3

Patch still applies cleanly.

#4

Status:needs review» needs work

But no longer :(

# patch -p0 < password-reset-correction.patch
(Stripping trailing CRs from patch.)
patching file modules/user/user.module
Hunk #1 FAILED at 1169.
1 out of 1 hunk FAILED -- saving rejects to file modules/user/user.module.rej

#5

Priority:normal» critical
Status:needs work» needs review

reroll

test:
1. admin/user/user/create - create user with name: name@name.com email: mail@mail.com
2. logout
3. user/password - enter name@name.com

AttachmentSizeStatusTest resultOperations
password-reset-correction_1.patch1.14 KBIgnored: Check issue status.NoneNone

#6

I might be better to roll back to the original version then? It's slightly more easier to read, IMO.

#7

Dries, I assume you mean the original version of the patch. It is not exactly right, as it identifies "Dries@drupal.org" kind of usernames (as supported by drupal.module in previous Drupal versions, possible to install alongside Drupal 6 as a contrib) as an email address and will fail to load the user, not going to try this as a username too.

#8

Status:needs review» reviewed & tested by the community

Just to confirm I tested this on a clean install, and it works fine with name@name usernames. So I'm marking to RTBC.

#9

Status:reviewed & tested by the community» fixed

I looked at the patches, and reread Dries' concern. Indeed, we should not call something a cleanup, when it solves the same problem with over-commented, hard to follow code, when it does not solve any performance problems at the same time. The password request page is so seldom used, that I rolled back to the more cleaner version (but still added two lines of comments there to make it clear).

Committed the attached patch.

AttachmentSizeStatusTest resultOperations
user_pass_fix.patch879 bytesIgnored: Check issue status.NoneNone

#10

Status:fixed» closed (fixed)

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

nobody click here