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.