Skip to:
Content

BuddyPress.org

Opened 14 years ago

Closed 14 years ago

#1733 closed defect (bug) (fixed)

Missing filter on template check inside bp_core_catch_no_access HAS-PATCH

Reported by: mrmaz's profile MrMaz Owned by:
Milestone: 1.2 Priority: critical
Severity: Version:
Component: Keywords: has-patch bp_core_catch_no_access bp_core_catch_uri bp_located_template
Cc:

Description

In 1.2 a condition was added to bp_core_catch_no_access to look for the template and redirect to home, but the bp_located_template filter is not being applied there like it is in bp_core_catch_uri.

Since bp_core_catch_no_access runs before bp_core_catch_uri, it is impossible to reach the filtered check.

I am attaching a patch that I tested.

Attachments (2)

bp_located_template.patch (1.0 KB) - added by MrMaz 14 years ago.
patch for bp-core-catchuri.php
bp_located_template_tweak.patch (1.1 KB) - added by MrMaz 14 years ago.

Download all attachments as: .zip

Change History (8)

@MrMaz
14 years ago

patch for bp-core-catchuri.php

#1 @MrMaz
14 years ago

  • Keywords has-patch added
  • Summary changed from Missing filter on template check inside bp_core_catch_no_access to Missing filter on template check inside bp_core_catch_no_access HAS-PATCH

#2 @rvenable
14 years ago

Nice. I noticed this bug breaking my plugin yesterday but didn't have a chance to report it, so you beat me to it MrMaz!

I noticed that the offending code was added in changeset:2088, for what its worth.

I think one problem with the code that in r2088 is that it uses $bp_path differently than $bp_path is used in the bp_core_catch_uri() function. bp_core_catch_no_access() just appends '.php' to the end of $bp_path and trying to locate the template. bp_core_catch_uri() does a little more by first treating $bp_path as an array (which doesn't seem to really do anything, but might break things later), then trying to locate the template but applying filters onto the result (the crucial part for plugins).

So, I think there should be a consistent way of handling $bp_path. You can make a function bp_located_template() which handles the locating. That function can return the located path, or an empty string if the path wasn't found, and a filter can be applied on its return value.

function bp_located_template() {
	/* Can be a single template or an array of templates */
	$templates = $bp_path;

	foreach ( (array)$templates as $template )
		$filtered_templates[] = $template . '.php';

	return apply_filters( 'bp_located_template', locate_template( (array) $filtered_templates, false ), $filtered_templates ) ;
}

Or better yet, just do everything in bp_catch_uri() and save it as a global $bp_located_template instead of $bp_path.

#3 @MrMaz
14 years ago

I really like rvenable's solution, although I haven't tested yet. It makes a lot of sense to refactor this into one function now.

Btw, $bp_path needs to be globalized.

#4 @apeatling
14 years ago

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

(In [2489]) Fixes #1733 props MrMaz

#5 @MrMaz
14 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

I just ran into the problem that rvenable describes above. It didn't hit me at first, but now I understand the issue. bp_core_catch_uri sends an array to the bp_located_template filter, so the bp_core_catch_no_access should also be sending an array. MY BAD!

Will be attaching a patch to fix this.

#6 @apeatling
14 years ago

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

(In [2525]) Fixes #1733

Note: See TracTickets for help on using tickets.