Skip to:
Content

BuddyPress.org

Opened 5 years ago

Closed 5 years ago

#6190 closed defect (bug) (fixed)

Custom page template needs to be validated before using it in template stack

Reported by: r-a-y Owned by:
Milestone: 2.2.1 Priority: normal
Severity: normal Version: 2.2
Component: Appearance - Template Pack Keywords: has-patch
Cc:

Description

Came across an issue on an install where a non-existent custom page template was being used for the _wp_page_template meta key in postmeta.

This led to BP being unable to find a wrapper template.

Attached patch adds some validation before using whatever is passed by get_page_template_slug() as the wrapper template.

Code uses the same validation code as in wp_insert_post():
https://core.trac.wordpress.org/browser/tags/4.1/src/wp-includes/post.php#L3412

Attachments (2)

6190.01.patch (749 bytes) - added by r-a-y 5 years ago.
6190.02.patch (670 bytes) - added by r-a-y 5 years ago.

Download all attachments as: .zip

Change History (10)

@r-a-y
5 years ago

#1 @johnjamesjacoby
5 years ago

This doesn't seem like it's necessary, or else the array_unshift() bit or the bp_get_buddypress_template filter are not working correctly.

If the page-template doesn't exist, BuddyPress's theme compatibility should continue on down the stack until a file_exists() finds a file in the bp_get_buddypress_template filter stack, which comes from bp_get_theme_compat_templates().

Did none of those files exist, or was that function or filter unhooked or overridden?

#2 @johnjamesjacoby
5 years ago

Also, WP_Theme::get_page_templates() is a relatively costly function to call during page output vs. when saving a post. If the page_templates cache doesn't exist, it needs to be built by scanning the theme directory, opening each file, looking for page-template headers, and compiling any it finds into an array.

#3 @r-a-y
5 years ago

This doesn't seem like it's necessary, or else the array_unshift() bit or the bp_get_buddypress_template filter are not working correctly.

It is necessary when a custom page template does not have a valid header or does not exist. But, this is more of an edge case since, as you say, the template should be validated when WP saves the page.

Did none of those files exist, or was that function or filter unhooked or overridden?

No filters unhooked.


I took a closer look and the _wp_page_template value was activity/index.php. Did some digging and found that as of BP 1.5 and in bp-default, we deemed the activity directory page as a WP custom page template:
https://buddypress.trac.wordpress.org/browser/branches/1.5/bp-themes/bp-default/activity/index.php

On this particular install, the admin must have selected this page template. Fast-forward to 2.2 and this bug creeps up when the install has moved onto theme compatibility.

What happens is BP goes through the template stack and does find the template file in /bp-legacy/buddypress/activity/index.php. But this template file is a template part and not a real page template.

So the end result is BP just loads the template part without a wrapper template.

I get that WP_Theme::get_page_templates() is costly, but we kind of are at fault here because of our template headers in bp-default. This could be a lingering and annoying issue, so I've opted for a less-costly fix in 02.patch.

Let me know what you think.

Last edited 5 years ago by r-a-y (previous) (diff)

@r-a-y
5 years ago

This ticket was mentioned in Slack in #buddypress by jjj. View the logs.


5 years ago

This ticket was mentioned in Slack in #buddypress by r-a-y. View the logs.


5 years ago

#6 @johnjamesjacoby
5 years ago

In 9497:

Check for existence of template before adding it to stack, to avoid accidentally including an unintended file. Props r-a-y. See #6190 (for trunk)

#7 @johnjamesjacoby
5 years ago

In 9498:

Check for existence of template before adding it to stack, to avoid accidentally including an unintended file. Props r-a-y. See #6190 (for 2.2 branch)

#8 @johnjamesjacoby
5 years ago

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

Marking as resolved.

Thanks Ray!

Note: See TracTickets for help on using tickets.