Make WordPress Core

Opened 3 years ago

Last modified 12 months ago

#51806 new defect (bug)

Add an early exit for files with _deprecated_file() calls

Reported by: sergeybiryukov's profile SergeyBiryukov Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: General Keywords:
Focuses: Cc:


Background: #35835

[49619] added an early exit when calling the wp-includes/rss-functions.php file directly to avoid unnecessary errors in server logs.

There are ~30 other files in core with _deprecated_file() calls so it seems like a good idea to have a consistent policy for the other instances.

That said, it's not limited to _deprecated_file(), as noted in comment:3:ticket:35835 and comment:1:ticket:35835:

You'll get a similar error if you access wp-includes/default-filters.php or any number of others too potentially.

So perhaps other files that can clutter server logs with unnecessary errors would benefit from an early exit too.

Change History (8)

#1 @ayeshrajans
3 years ago

This is probably not an ideal place to discuss this, but I'd like to raise a concern I had in my mind for a long time.

In WordPress, we have files that we can call directly from the browser (for example wp-admin/options-writing.php), and ones that we cannot (such as wp-blog-header.php). I totally agree that we need some sort of consistency, because at the moment, it is not clear which files are meant to be entry points, and which are to be included.

I wonder if there was any discussion to move the include files to where they belong, or rename them (for example to .inc in Drupal style), so they can be blocked from web server configuration and save ourselves of the early-exit calls, while also getting some potential opcache benefits?

#2 @zodiac1978
3 years ago

As a first step I checked the 30 search results which are using _deprecated_file():

These files are all resulting in a full path disclosure if display_errors is enabled:


PHPMailer is checking via function_exists first:


Not sure why this is not reproducing the FPD:


This file has a check and uses _deprecated_file() only if a my-hacks.php file exists (but would generate FPD if available, I think):


And the 30th search result is just the function itself:


And for completeness I would like to point again to the issue on the Health Check plugin to check for display_errors:

because we recommend to disable this setting:

Last edited 3 years ago by zodiac1978 (previous) (diff)

#3 @SergeyBiryukov
3 years ago

Thanks for the list of files! Just noting that this is not so much about full path disclosure specifically (which is covered by the handbook link above), but more about just preventing unnecessary errors in server logs.

#4 @zodiac1978
3 years ago

Just noting that this is not so much about full path disclosure specifically

I understand, but then we need to discuss our recommended path here.

The error log mentioned in the first ticket could be generated through testing tools, like wpcheck, which are just taking one of those files to check for full path disclosure:

For example:

If we fix it in one file, these tools will change to another file and generate unnecessary error logs again.

I think we have two (or three) possible solutions here:

  • 1. Fix every single file to not show any errors on direct access.
  • 2a. Force disabling display_errors per default (maybe not if WP_DEBUG is true)
  • 2b. Add a check in Site Health to inform the user that display_errors is on.

If we have a decision on the path, then we could go forward.

Maybe @clorith can add an opinion on the Site Health idea?

This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.

2 years ago

#6 @lukecarbis
2 years ago

  • Milestone changed from 5.7 to Future Release

#7 @zodiac1978
18 months ago

The idea from 2b about adding a check to Site Health is handled in #52652.

Note: See TracTickets for help on using tickets.