Skip to:
Content

BuddyPress.org

Opened 7 years ago

Closed 7 years ago

#4216 closed defect (bug) (no action required)

bp_core_redirect ignores spaces in user domains.

Reported by: DennisSmolek Owned by:
Milestone: 1.6 Priority: normal
Severity: normal Version: 1.5.5
Component: Messages Keywords: reporter-feedback
Cc: DennisSmolek

Description

While working on a site I noticed that when users send messages the redirect to view the message comes back as a 404.
I didn't notice because my display name was admin but my users are formatted as Firstname Lastname as their display name.

This means their standard $bp->loggedin_user->domain looks like:
http://mysite.com/members/Firstname Lastname/

So the expect Result from the redirect is something like:
http://mysite.com/members/Firstname Lastname/messages/view/11/

But the actual result is:
http://mysite.com/members/FirstnameLastname/messages/view/11/

Which results in a 404.

I tracked down the issue and it deals with wp_redirect() and more importantly wp_sanitize_redirect()

Now I know spaces are generally frowned upon so thats why it's happening but I don't know how to format the domain to not have the spaces.

In the mean time a am just adjusting the filter wp_redirect to change spaces to %20 which is what it should do.. And it works fine.

bp_core_redirect could do this as well to $bp->loggedin_user->domain before it passes to wp_redirect().

Change History (11)

#1 @DJPaul
7 years ago

  • Milestone changed from Awaiting Review to 1.6

We had a very similar issue with @ in usernames in the redirects with those two WP functions (i.e. email addresses as usernames; #2670). Perhaps URL encoding the ->domain variables is a solution.

#2 @boonebgorges
7 years ago

+1 to DJPaul's solution. We don't think it's wise to go messing with formatting inside of the redirect functions - things might get double-encoded.

#3 @boonebgorges
7 years ago

On further experimentation, I'm unable to reproduce the issue, mainly because I'm unable to make BP construct and parse URLs that have spaces with them. DennisSmolek, can you tell us more about your setup? How is it that your users are able to have spaces in their user_login? Are you running BP_ENABLE_USERNAME_COMPATIBILITY_MODE?

#4 @DennisSmolek
7 years ago

Sorry this fell through the cracks. My users are created / managed with a third party service. The service creates a massive export of data multiple times a day which I import and process.
I create the users using script and their login is actually their email address.

The domain is being generated off of their display name which has the spaces. I didn't do anything to change that, its how they were generated automatically. I'm actually worried about it because of generic name conflicts.

If there is a way to change the domain slug to something else I will, I have a 6 digit unique ID that would actually be better...

No, Im not running in compatibility mode, I don't know anything about it...

#5 @boonebgorges
7 years ago

By default (ie not in compatibility mode), URLs are built out of user_nicename. My guess is that the script you're using to create users is putting display names in the user_nicename field (in wp_users). This is a bad idea: user_nicename is supposed to be a unique, URL-friendly version of the user_login. See https://core.trac.wordpress.org/browser/tags/3.3.2/wp-includes/user.php#L1282, https://core.trac.wordpress.org/browser/tags/3.3.2/wp-includes/user.php#L1336

If I'm right, then you'll probably need to do the following:
1) Modify your import script so that it enforces user_nicename rules, or better yet, just uses wp_insert_user() instead of INSERTing directly to the DB; and
2) Modify your existing users' user_nicename fields so that they're valid.

#6 @DJPaul
7 years ago

  • Keywords reporter-feedback added

DennisSmolek, was your user creation script putting display names into user_nicename like Boone said?

#7 @DennisSmolek
7 years ago

It actually uses wp_insert_user.

I do a line by line file read which generates an array in a loop, then it creates this array:

$userdata = array(
		'user_pass' => $passwd,
		'user_login' => $shopper_data[21],
		'user_nicename' => $shopper_data[56],
		'user_email' => $shopper_data[21],
		'display_name' => $shopper_data[56],
		'first_name' => $shopper_data[22],
		'last_name' => $shopper_data[27],
		'description' => $shopper_data[35]
		);

Then does other junk but eventually:

$user_id = wp_insert_user($userdata);

$shopper_data[56] is the FULLNAME field from the export, so John E Doe or John Doe.

Now I'm not sure what user_nicename's other uses are, but I'm considering switching to using a unique number. It's ok if the urls are numbers, the shoppers all know themselves as numbers.
Because members/john doe and members/john doe are going to start conflicting and I don't know how that will work out.

#8 @boonebgorges
7 years ago

If you are using wp_insert_user(), then you will never have a nicename like 'John E Doe'. In the two links to the WP codebase I showed you above, you can see that it would be converted to something like 'john-e-doe', or, in the case where there was already a 'john-e-doe', it'd be 'john-e-doe-2'. If you're seeing things like 'John E Doe' in your wp_users table, then either you're incorrect that it's using wp_insert_user(), or you've got a plugin that is messing with the insert-user process (maybe something hooked to 'user_register').

Switching to numeric user_nicenames would work fine, but before you implement that workaround, it'd be really nice to know what's at the root of your problem.

#9 @DennisSmolek
7 years ago

Looking in my wp_users table I get results in the nicename column like: Bobera Juandessa Johnson
that aren't cleaned up.

I know its using wp_insert_user, I use the same function for both adding new users and updating the user info.
I can't imagine what other plugins would be doing that... My plugins are:
BP
BP Template Pack
Backup Buddy
Better Related Content
Contact Form 7
Meta Box
Sentry (my plugin)
WATS Premium
Wordpress SEO
WP-Mail-SMTP
WP Maintenance Mode

I dont think it's WATS because it was doing this before I installed WATS.
I will create a base install and create users and see if I can replicate the issue with limited code..

#10 @boonebgorges
7 years ago

Yeah, I'll be very curious to hear what happens.

#11 @boonebgorges
7 years ago

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

I'm about 98% sure that this is not a BP problem, since our system is designed never to allow spaces in URLs. I'm going to close the ticket to clear the milestone. DennisSmolek, I'll still be curious to hear what you find - don't hesitate to update this ticket (or reopen if it is our fault).

Note: See TracTickets for help on using tickets.