WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

#13130 closed defect (bug) (fixed)

WP Import with Multiple Authors Fails

Reported by: technosailor Owned by:
Milestone: 3.0 Priority: high
Severity: blocker Version:
Component: Import Keywords: has-patch dev-feedback
Focuses: Cc:

Description

It seems we have a regression in WordPress 3.0 and I can't figure out where.

When multiple authors are in a WXR file, only 1 is ever imported (verified on two different sites with multiple authors) and all posts are attributed to the admin. This is a regression in 3.0 as it works fine in 2.9.x. I consider it a blocker.

Attachments (3)

13130.diff (3.9 KB) - added by blepoxp 4 years ago.
Adds author email to export / import
3.0-import-emptyemail.diff (551 bytes) - added by yoavf 4 years ago.
Allow import of more than one user with empty email address
13130-3.diff (694 bytes) - added by blepoxp 4 years ago.
Adds WP_IMPORTING to conditional

Download all attachments as: .zip

Change History (10)

comment:1 westi4 years ago

  • Severity changed from normal to blocker

comment:2 blepoxp4 years ago

  • Keywords dev-feedback added

I've tested this and the problem stems from the fact that we're importing users w/o email addresses. More accurately, all of the imported users have the same email address: an empty string. This happens at wp_create_user.

Once we get past the first inserted user with an empty string for an email, all others are rejected because wp_insert_user() returns a wp_error object w/o insert if the email already exists in the database.

This worked in 2.9 because wp_insert_user (in /wp-includes/registration.php) didn't check for dups and return the error.

I can put together a patch, but there's no clear way to proceed without dev-feedback.
Possible solutions include capturing email in export/import process or modifying wp_insert_user to allow dups of empty strings.

blepoxp4 years ago

Adds author email to export / import

comment:3 blepoxp4 years ago

  • Keywords has-patch needs-testing added

The above patch fixes the problem by exporting / importing author email addresses. This was the solution suggested to pursue in #wordpress-dev

comment:4 technosailor4 years ago

Patch fails to work for me. I have a 12MB export file from 2.8 with 25ish authors. The problem still persists. In addition, I now have an offset error happening somewhere around line 788.

Notice: Undefined offset: 17470 in /Users/aaron/Sites/wp-admin/import/wordpress.php on line 788

Will dig in for code sprint.

yoavf4 years ago

Allow import of more than one user with empty email address

comment:5 yoavf4 years ago

Was broken in [12778] - we should allow more than one user with empty email address for imports.

comment:6 blepoxp4 years ago

  • Keywords needs-testing removed

Sorry... this wound up longer than i originally intended.

I did a little testing this morning after the dev meeting yesterday. nacin was concerned that yoavf's patch would create vulnerabilities with other places in the code that call this function. If the goal for 3.0 is to simply fix the importer breaking when multiple users are imported with empty strings for their email address, then yoavf's patch will work and we can develop better functionality for the dup checks in 3.1.

yoavf's patch doesn't cause any undesired side effects while creating users via admin or while creating new users from the front end registration. Each of those functions checks for existence of a correctly formatted email before passing the data to wp_insert_user().

If a plugin, theme, or other importer does get to wp_insert_user() with an empty string for an email address, yoavf's patch will not cause any greater 'bug' in functionality than 2.9 did.

2.9 (all 'already registered' emails get through if they made it to this function):

if ( empty($user_email) )
    $user_email = '';
$user_email = apply_filters('pre_user_email', $user_email);

3.0 (no 'already registered' emails get through if they made it to this function):

if ( empty($user_email) )
    $user_email = '';
$user_email = apply_filters('pre_user_email', $user_email);
	
if ( !$update && email_exists($user_email) )
   return new WP_Error('existing_user_email', __('This email address is already registered.') );

yoavf's patch (only 'already registered' emails with empty strings get through if they made it to this function):

if ( empty($user_email) )
    $user_email = '';
$user_email = apply_filters('pre_user_email', $user_email);
	
if ( !$update && email_exists($user_email) && '' != $user_email )
   return new WP_Error('existing_user_email', __('This email address is already registered.') );

If we're still concerned about unforseen bugs as a result of this, we could use the following code until a better function is developed in 3.1:

if ( !$update && email_exists($user_email) ) {
    if ( ! isset( $_GET['import'] ) && 'wordpress' != $_GET['import'] )
        return new WP_Error('existing_user_email', __('This email address is already registered.') );
}

blepoxp4 years ago

Adds WP_IMPORTING to conditional

comment:7 nacin4 years ago

  • Resolution set to fixed
  • Status changed from new to closed

(In [14504]) Allow duplicate emails (generally empty) if WP_IMPORTING to restore 2.9 behavior. props blepoxp, yoavf. fixes #13130. see r12778.

Note: See TracTickets for help on using tickets.