Opened 2 years ago
Closed 8 months ago
#3493 closed defect (bug) (fixed)
Default arg to "bbp_add_forums_roles()" can cause errors
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 2.6.10 | Priority: | normal |
Severity: | normal | Version: | 2.2 |
Component: | API - Roles/Capabilities | Keywords: | reporter-feedback 2nd-opinion dev-reviewed has-patch |
Cc: | naxoc |
Description
The bbp_add_forums_roles()
function has a default arg of null
for $wp_roles
. That causes problems when we try to use it in the loop in the function. This patch adds a check and initializes the $wp_roles
var if necessary.
Attachments (2)
Change History (5)
#1
@
2 years ago
- Keywords reporter-feedback 2nd-opinion dev-reviewed added
- Milestone changed from Awaiting Review to 2.6.10
- Owner set to johnjamesjacoby
- Status changed from new to assigned
- Version set to 2.2
#2
@
2 years ago
- Cc naxoc added
- Keywords has-patch added
I have a question about your use case:
when we try to use it in the loop in the function
Yes, sorry. That was not very clear. The loop I meant was the foreach further down in the function where the $wp_roles
var is used. Not the WP-loop :)
the call is coming from inside the house! 👻
Woops! I didn't catch that one. Let's find a better solution. I guess it would be a breaking change to remove the default null
arg, but how about this patch that just creates a stdClass
if we need to?
Interesting! 🤔
I have a question about your use case:
Are you able to share an example? I'd love to know more about how you're using bbPress roles!
History
(When implementing them, I suppose I'd not considered that
bbp_add_forums_roles()
would be called directly, or outside of anyplace that$wp_roles
as aWP_Roles
object would not be passed in.)Usually, bbPress roles get invoked directly from
WP_Roles
(which is the$wp_roles
global):$wp_roles->__construct()
$wp_roles->for_site()
$wp_roles->init_roles()
wp_roles_init
hookbbp_roles_init
hookbbp_add_forums_roles()
Multisite blog-switching will add these to the beginning of the above stack:
switch_blog
hookwp_switch_roles_and_user()
Patch notes
Thank you again 🙏 for including a patch with this issue. I've applied & tested it, and (if I understand your use-case) it seems like it does what you (may) need it to, but with 1 caveat worth mentioning...
Calling
wp_roles()
will trigger thewp_roles_init
action, and doing so where you've suggested will cause a single runtime recursion, wherebbp_add_forums_roles()
gets called from inside ofbbp_add_forums_roles()
– the call is coming from inside the house! 👻It is possible to conditionally "juggle" the hook in there to avoid recursing (and that may be necessary here) but typically that is an early indicator there may be a better approach that involves fewer compromises or work-arounds.
Edit: Regardless of anything, you have identified code that we can improve together. bbPress should not attempt to blindly call methods on the
$wp_roles
global variable without confirming they are callable. 💫