As per title - this essentially makes it impossible to properly validate a new user programmatically (i.e. by calling drupal_execute('user_register')). Simple one-line patch to fix.

Comments

drewish’s picture

seems likea logical change but that the || !$user->uid bit is rather cryptic. i'd suggest a comment at the least or perhaps a different "wording".

hunmonk’s picture

StatusFileSize
new2.92 KB

makes sense to me. while i was digging around in there, it struck me that we should be trimming $edit['name'] and $edit['mail'] as well -- otherwise i think you could get a situation where both 'sam' and 'sam ' could register.

attached patch corrects.

hunmonk’s picture

StatusFileSize
new2.99 KB

...with drewish's comment suggestion.

moshe weitzman’s picture

Status: Needs review » Reviewed & tested by the community

i tested this, and did a code review. RTBC. is a simple fix, the diff ismore than a few lines only cuz of indentation changes.

dries’s picture

Status: Reviewed & tested by the community » Needs work

I don't think the trimming is necessary. It should be catched by the validate functions:

For the username:

  if (substr($name, 0, 1) == ' ') return t('The username cannot begin with a space.');
  if (substr($name, -1) == ' ') return t('The username cannot end with a space.');

For the e-mail address there is no such equivalent. Looks like we want to clean this up a little bit more. Maybe the trim() should not have been added to this patch. They are fairly unrelated, and complicate the patch's original intend.

walkah’s picture

As I mentioned to Chad in IRC - the trim()'s certainly aren't part of this patch - while I think they're probably useful for validation, they're not actually addressing what this patch is about. I'd vote for committing the original patch and create a new issue to address name/mail validation (trimming , etc).

moshe weitzman’s picture

Status: Needs work » Reviewed & tested by the community

RTBC for the original patch.

dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD.

Anonymous’s picture

Status: Fixed » Closed (fixed)