Skip to:
Content

BuddyPress.org

Opened 7 years ago

Closed 7 years ago

#7833 closed defect (bug) (fixed)

BP Nouveau: do not check the user exists when Accepting/rejecting friendships

Reported by: imath's profile imath Owned by: imath's profile imath
Milestone: 3.0 Priority: normal
Severity: major Version:
Component: Templates Keywords: has-patch commit
Cc:

Description

To avoid repeating various checks, i've grouped all friendship ajax actions into a unique handler. But there are two cases where one of the checks shouldn't be done: when accepting or rejecting a friendship, the $_POST['item_id'] is not a user ID but a friendship ID. So as soon as we have more friendships than users Nouveau doesn't perform these two actions because the user does not exists!

My bad & sorry for noticing it late! The attached patch is restricting the user exists check to all other actions. I'll commit it before Midnight ;)

Attachments (2)

7833.patch (1.2 KB) - added by imath 7 years ago.
7833.2.patch (8.2 KB) - added by imath 7 years ago.

Download all attachments as: .zip

Change History (7)

@imath
7 years ago

#1 @DJPaul
7 years ago

  • Keywords commit removed
  • Milestone changed from 3.0 to Awaiting Review

#2 @DJPaul
7 years ago

If you can test all possible inputs (different buttons, different button layouts) and confirm the fix works as expected and does not cause more problems, we can put it into 3.0

#3 @imath
7 years ago

@DJPaul Maybe my description/patch wasn't clear :) So as you wanted me to test all possible actions/layouts for the friendship button (having the button_element set to button or a) and with all possible Nav layouts (Customizer), I decided to do it but after improving the patch to avoid confusions.

So the trouble was :

All friendship actions are managed in one ajax handler bp_nouveau_ajax_addremove_friend(), but 2 actions are not sending an ID from the $wpdb->users table but an ID from the $bp->table_prefix . 'bp_friends' table.

For these two actions ('friends_accept_friendship' & 'friends_reject_friendship'), it makes no sense to check if an ID from the $bp->table_prefix . 'bp_friends' table exists as a user into the $wpdb->users table.

So both 7833.patch or 7833.2.patch are fixing the issue, the first one edits less code than the other which is less confusing..

It's up to what you prefer :)

The risk of not committing one of the two patches is that it quickly won't be possible to accept or refuse friendship for users at all.

I haven't notice the issue before because friendship IDs existed by coincidence into the users table. But as soon as i had more rows into the $bp->table_prefix . 'bp_friends' table than into the $wpdb->users table : bing 100% of fails for these two actions.

Last edited 7 years ago by imath (previous) (diff)

@imath
7 years ago

#4 @DJPaul
7 years ago

  • Keywords commit added
  • Milestone changed from Awaiting Review to 3.0

Thank you for the explanation and testing. Patch 1 is definitely better.

This is fine to commit. :)

#5 @imath
7 years ago

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

In 12094:

Nouveau: do not request a user with a Friendship ID in Ajax handler

Two actions managed within the common Friendship Ajax handler are not sending a user ID but a friendship ID :

  • friends_accept_friendship
  • friends_reject_friendship

As a result, the user check can wrongly prevent users to accept or decline friendships as soon as a friendship ID does not match with an existing user ID. For these two specific cases we are now omitting it but keep it for all other actions as they are sending a user ID.

Props DJPaul

Fixes #7833

Note: See TracTickets for help on using tickets.