Skip to:
Content

Opened 4 years ago

Closed 4 years ago

#5649 closed defect (bug) (fixed)

Missing argument 2 in bp-xprofile-classes

Reported by: danbp Owned by: boonebgorges
Milestone: 2.1 Priority: normal
Severity: normal Version: 1.5
Component: Extended Profile Keywords: has-patch needs-unit-tests
Cc:

Description

BP 2.0.1/wp 3.9.1 single install (php 5.4.12 - mySql 5.6.12 apache 2.4.4) debug mode false

When using bp_after_has_profile_parse_args i get a php warning
Missing argument 2 for wpdb::prepare()called by bp-xprofile-classes.php:156

Changed the line to following and the warning disapeared.
$where_sql = $wpdb->prepare( "WHERE g.id = %d NOT IN({$exclude_groups})", $profile_group_id );
But not sure it's right.

Attachments (2)

5649.patch (955 bytes) - added by imath 4 years ago.
5649.02.patch (2.3 KB) - added by imath 4 years ago.

Download all attachments as: .zip

Change History (10)

#1 @danbp
4 years ago

Partially solved!
The previous modification worked on multisite, but not on single.
So i changed to

$where_sql = $wpdb->prepare( "WHERE g.id NOT IN ({$exclude_groups})", $profile_group_id );

to solve the problem. See ticket #5650

That said, when debug mode is activated, i have a notice wpdb::prepare is not correctly called

Last edited 4 years ago by danbp (previous) (diff)

#2 @imath
4 years ago

  • Component changed from Members to XProfile
  • Keywords has-patch added
  • Milestone changed from 2.0.1 to 2.1
  • Version set to 1.5

Thanks danbp for the feedback.

The problem is $wpdb->prepare is waiting for a placeholder. For instance, if you want to prepare a query that has a string as argument, you need to use the %s placeholder, for an integer it's %d. An example is line 154 of the file you're referring to. The placeholder is %d because we're looking for a specific profile group id :
$where_sql = $wpdb->prepare( 'WHERE g.id = %d', $profile_group_id );

So your patch will send a notice because you don't include the placeholder.

Now in the case of 'exclude groups' it's a comma separated list of profile group ids. So i think we should sanitize it in another way using wp_parse_id_list() see patch 5649.patch.

This was already there in 1.5, so it's not a regression introduced in 2.0. I suggest to fix this in 2.1.

@imath
4 years ago

#3 follow-up: @danbp
4 years ago

So the original 2.0.1 line is wrong.
https://buddypress.trac.wordpress.org/browser/tags/2.0.1/bp-xprofile/bp-xprofile-classes.php#L156

I applied your patch, and everything works like a charm now. Thank you.

#4 in reply to: ↑ 3 @imath
4 years ago

Replying to danbp:

So the original 2.0.1 line is wrong.

I think it's more complicated, obviously it's now wrong, but before $wpdb->prepare wasn't so strict and didn't absolutely require the placeholder.

I applied your patch, and everything works like a charm now. Thank you.

Thanks for your feedback :)

#5 @danbp
4 years ago

As i mentionned on my other ticket (#5650), my patch worked on multisite but not on single. If the culprit is the severity of wpdb::prepare, why not using a conditionnal on a query with placeholder and something similar for a query without placeholder ?

Good read here, from A. Nacin: http://wordpress-hackers.1065353.n5.nabble.com/WP-3-5-2-multisite-How-to-use-NOT-IN-in-wpdb-prepare-tp41812p41824.html

#6 @boonebgorges
4 years ago

  • Keywords needs-unit-tests added

5646.patch looks like the correct fix for me. I would like to see a unit test for the behavior in question (a query that excludes multiple xprofile groups).

@imath
4 years ago

#7 @imath
4 years ago

In 5649.02.patch i'm suggesting a unit test.

#8 @boonebgorges
4 years ago

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

In 8430:

Correct query concatenation for exclude_groups param in BP_XProfile_Group::get()

The previous format was not working properly when passing multiple group ids.

Fixes #5649

Props imath

Note: See TracTickets for help on using tickets.