Ensure locate_template only loads theme files

[jorbin]

It's possible to have locate_template load files that are not template files which could be unexpected behavior that breaks the display of a page.

The chance this breaks something is, I think, not infinitesimal. Therefore, this should go in early.

An initial patch is attached.

[joemcgill]

Thanks @jorbin. The use of realpath() in this diff can have a measurable negative performance impact. Given that this function has assumed that the template names were being concatenated directly with the various constants previously, I wonder if we could use validate_file() instead here and avoid the multiple calls to realpath()?

Uses validate_file to ensure that the paths processed by locate_template are not directory traversals, Windows drive paths, etc...

Trac ticket:
https://core.trac.wordpress.org/ticket/58905

[pypwalters]

Option added that uses validate_file(). I removed the unit tests because the result was always coming up empty. I believe I am running into some complications because of the use of STYLESHEETPATH and TEMPLATEPATH in the locate_template function. Is it possible that these are not available to phpunit?

[oglekler]

Because this is an early ticket, I am moving it into the 6.5 milestone.

[swissspidy]

locate_template did see some updates in r56635 which deprecated the usage of these constants. Patch needs to be refreshed.

Added refresh patch for Validate url in locate template.

[oglekler]

This patch still needs unit-tests and is marked as early, that means that it is to late for now for it in the current milestone. In addition, it will be nice for testers to have testing instructions.

[jorbin]

I've added some automated tests and am planning to commit the code in ​https://github.com/WordPress/wordpress-develop/pull/6502/ soon. As the chance of something breaking is not infinitesimal, I also am going to publish a dev note soon after to encourage testing.

@mukeshpanchal27 Are you able to edit the comment above to note the raw difference. As the benchmarking script is using hrtime() (nanoseconds) I think the raw numbers could be more helpful than a percentage.

@peterwilsoncc Updated ​https://github.com/WordPress/wordpress-develop/pull/6502#pullrequestreview-2259763696

Converting the raw times from nanoseconds to milliseconds is showing a showdown of between 0.7ms to 2.5ms over 1000 iterations so I don't think it's worth considering the performance impacts as a reason to block the benefits of avoiding using the function to include files outside of the template paths.

[peterwilsoncc]

@jorbin I think the PR is good, are you still wanting to commit this or would you rather wait until the 6.7 branch is forked?

[joemcgill]

Assigning to @jorbin for a final decision on committing or punting for 6.7.

[jorbin]

There have been some test failures and I don't think I will be able to get them resolved before beta 1, so I'm bumping this to 6.8

[audrasjb]

This ticket was reviewed during today's bug scrub. @jorbin do you think you can address this ticket for 6.8? As it is marked early, it should ideally be committed by next week.

Updated this PR from trunk to try to address Unit Test failures, but something is causing these to error out before reporting failures in the workflow. I'm running them locally to see if I can diagnose what the issue is.

I've also found that the reason the PHPUnit Tests workflow are not showing failures is because the test suite is failing silently on Tests_Embed_Template::test_oembed_output_post. This will also require investigation.

[joemcgill]

Removing the commit keyword until the unit test failures can be addressed.

[jorbin]

I am not confident committing this during beta, I think it needs as much testing as possible so I am moving this to 6.9.

[SirLouen]

Combined Bug Reproduction and Patch Test Report

Description

❌ This report can't validate that the indicated patch works as expected.

Patch tested: ​https://github.com/WordPress/wordpress-develop/pull/6502.diff

Environment

• WordPress: 6.9-alpha-60093-src
• PHP: 8.4.6
• Server: nginx/1.27.5
• Database: mysqli (Server: 8.4.5 / Client: mysqlnd 8.4.6)
• Browser: Chrome 136.0.0.0
• OS: Windows 10/11
• Theme: My First WordPress Theme
• MU Plugins: None activated
• Plugins:
  • Test Reports 1.2.0

Bug Reproduction Steps

1. Create a dummy Theme
2. Make sure it has at least the files: style.css and index.php. Check a Basic Theme example here: ​https://developer.wordpress.org/themes/classic-themes/your-first-theme/#classic-theme
3. Create another file inside the theme directory called test-non-template.txt
4. Install this plugin ​https://raw.githubusercontent.com/SirLouen/testing-template-loading/refs/tags/Second-Version/testing-template-loading.php
5. Create a new Post and add the short code to see the results: [test_locate_template]
6. 🐞Bug reproduced: Wrongly loaded non-template file

Actual Results with Patch

1. ❌ Issue is not resolved with the patch.

Additional Comments

If there are any concerns, please review the plugin for further testing.
The plugin tests:

1. Locating a typical file in any classic theme
2. Locating a random file that should not be located
3. Locating a file outside the theme files, that could potentially be a security risk like wp-config.php

These tests can obviously be added as Unit-Tests (and should be), but for a regular tester that is not familiarized with PHPUnit this type of plugin-based testing is much more friendly and easier to be inspected.

Supplemental Artifacts

Link to the testing plugin repo: ​https://github.com/SirLouen/testing-template-loading/