WordPress-Coding-Standards icon indicating copy to clipboard operation
WordPress-Coding-Standards copied to clipboard

Empty index.php

Open dmtrmrv opened this issue 10 years ago • 25 comments

I have several directories in my plugin and add an empty index.php to each of them for some extra security. Something like this:

<?php // Silence is golden.

However this triggers an error:

----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 1 | ERROR | You must use "/**" style comments for a file comment
   |       | (Squiz.Commenting.FileComment.WrongStyle)
----------------------------------------------------------------------

I was wondering what would be the best way to fix it? Thanks.

dmtrmrv avatar Sep 10 '15 12:09 dmtrmrv

You could just exclude all index.php files in your custom XML config file:

 <exclude-pattern>*/index.php</exclude-pattern>

JDGrimes avatar Sep 10 '15 12:09 JDGrimes

I personally think the practice of empty index.php files is wrong. Leave it to the server configuration to hide directory listings.

GaryJones avatar Sep 13 '15 12:09 GaryJones

While that would be the proper solution, unfortunately the 'average' WP user doesn't even know what "server configuration" is... so I'd advocate for requiring these empty index files.

jrfnl avatar Sep 13 '15 13:09 jrfnl

We're not talking about an average user. We're talking about someone who is creating a Plugin or Theme, and thinks they need an empty index.php file. Those people can be educated. It's a pastable snippet of a line or two in. htacess.

GaryJones avatar Sep 13 '15 19:09 GaryJones

I agree with @GaryJones that it usually doesn't make sense to include index.php files in every directory in an open source project. They are only needed in directories which need to be protected for some reason.

However, on the other hand, WordPress core does continue to include index.php files in every directory, so perhaps we should make provision for that in the WordPress-Core ruleset, at least.

Also, note that using .htaccess is only for servers that are using Apache. I don't know if other options like Nginx are commonly misconfigured in this way, or not, but if so, adding .htaccess really doesn't give complete protection, especially since Nginx is becoming more popular.

JDGrimes avatar Sep 13 '15 22:09 JDGrimes

The other workaround it is to include a valid file comment in your index.php files of course:

<?php

/**
 * Do not modify the files in this folder.
 */

And just to add that indeed it is not the "average user" creating the plugin or theme but once finished and if the plugin or theme is distributed on w.org then the "average user" returns to the equation. :smirk:

ntwb avatar Sep 14 '15 04:09 ntwb

As much as I dislike the use of empty index.php, if they are to exist, then I agree that:

  • use of a file-level DocBlock instead of an inline comment makes it more consistent with every other file.
  • the DocBlock should include a Summary, or Summary and Description, that explains why that empty file exists. This makes it less cryptic to those browser through the files. So, much like @ntwb suggests:
<?php
/** 
 * Intentionally empty file.
 *
 * It exists to stop directory listings on poorly configured servers.
 */

@Drewapicture: Is this something we could decide upon and get into the Handbook, and fixed up in core, so it could then be checked against (if it exists) for plugins and themes?

GaryJones avatar Sep 17 '15 10:09 GaryJones

There's already a ticket open for standardising file headers in core, so I've commented there.

GaryJones avatar Sep 17 '15 13:09 GaryJones

I'm rethinking this. According to the handbook:

Whenever possible, all WordPress files should contain a header DocBlock, regardless of the file’s contents – this includes files containing classes. [emphasis in original]

Above I said this:

However, on the other hand, WordPress core does continue to include index.php files in every directory, so perhaps we should make provision for that in the WordPress-Core ruleset, at least.

It turns out that it isn't true. The only directories that include index.php files without any code or file level docblocks are those that don't contain bundled source code:

/wp-content/
/wp-content/plugins/
/wp-content/themes/

All of the the other directories that contain an index.php file are actually using them, that is, they contain code, and are accompanied by a file-level docblock.

So I suggest that we don't make special allowances for index.php files. WordPress can either update these files to have proper headers, or perhaps just exclude those directories from sniffing. Proposing we close this as wontfix.

JDGrimes avatar Aug 01 '16 19:08 JDGrimes

In the meantime I'm going to stick the template hierarchy in an index.php file or two 😏

https://johnblackbourn.com/ascii-wordpress-template-hierarchy

ntwb avatar Nov 01 '16 22:11 ntwb

Pinging @DrewAPicture for an opinion.

GaryJones avatar Aug 03 '17 22:08 GaryJones

@GaryJones Which part specifically are you seeking an opinion on? I can get on board with adding file headers to the empty indexes purely for standardization sake if that's what you're asking.

DrewAPicture avatar Aug 04 '17 01:08 DrewAPicture

@GaryJones Also, I disagree with your assertion that we aren't in fact talking about average users. The whole point of the indexes is to prevent listing on poorly configured user servers, which is where the plugins are going to live eventually. So it's absolutely about average users not even knowing their servers are configured poorly.

DrewAPicture avatar Aug 04 '17 01:08 DrewAPicture

I can get on board with adding file headers to the empty indexes purely for standardization sake if that's what you're asking.

Yes, that's it entirely. Checking for the existence and conformance of file headers then won't have to be skipped for the three index.php files.

The whole point of the indexes is to prevent listing on poorly configured user servers, which is where the plugins are going to live eventually.

And yet most of the directories in WP don't have them?

It's not even a safe workaround, as it's trivial for a server to have a different DirectoryIndex (or Nginx equivalent)

GaryJones avatar Aug 04 '17 01:08 GaryJones

OK, to prevent listing content. And again, DirectoryIndex/Nginx/[insert confusing jargon here] is irrelevant to the users that are actually going to be hosting the plugins.

DrewAPicture avatar Aug 04 '17 02:08 DrewAPicture

OK, to prevent listing content.

That makes sense. It's easy enough for a bad actor to find out what a site is likely to have in their wp-includes, but less so for their wp-content.

And again, DirectoryIndex/Nginx/[insert confusing jargon here] is irrelevant to the users that are actually going to be hosting the plugins.

I think we're talking about different aspects here, but it's irrelevant to the point in hand.

Question: Would you want the Handbook to advise plugin and theme developers to use index.php files in their directories?

Question: Is the previous example code a) sufficient to meet the WPCS and WP Doc standards, and b) suitably descriptive wording?

<?php
/** 
 * Intentionally empty file.
 *
 * It exists to stop directory listings on poorly configured servers.
 */

GaryJones avatar Aug 04 '17 03:08 GaryJones

Question: Would you want the Handbook to advise plugin and theme developers to use index.php files in their directories?

No directories within the plugin should contain user content, because they'll be overwritten by updates. So this wouldn't be for the directories within the plugin, but directories that the plugin creates in /wp-content for storing content in.

JDGrimes avatar Aug 04 '17 12:08 JDGrimes

I use this in the index.php file, hope it helps:

<?php // phpcs:ignore PEAR.Commenting.FileComment.Missing
// Silence is golden.

KhairulA avatar Dec 12 '19 07:12 KhairulA

Ok. I agree with empty index.php. PHP manuals teach how to fill this file with code. I can't understand how a program should run when index.php is empty.

AndrzejLan avatar May 03 '21 07:05 AndrzejLan

@AndrzejLan You should check out this amazing diagram Rarst made on wpse https://wordpress.stackexchange.com/a/26622/58895

index.php in a plugin is not the same as the one in the WordPress core. When the web request comes to your WP app the index.php file in the WordPress core will get 'pinged', and then the app execution will happen. The WP app will then execute the rest of the code, plugins included.

dingo-d avatar May 03 '21 08:05 dingo-d

Even with the workaround mentioned here https://github.com/WordPress/WordPress-Coding-Standards/issues/451#issuecomment-139961024 (which is the only correct thing to do, removing index files is not adequate as others have already expressed here too), we still get one warning, which is the "there must be exactly one blank line after the file comment"

That warning only disappears if after that blank line actually comes something EOF is not accepted as something valid and thus, it seems to trigger that warning, no matter what.

So, this triggers no notice:

<?php
/**
 * Intentionally empty file.
 *
 * It exists to stop directory listings on poorly configured servers.
 *
 * @package           whatever
 */

echo 'this';

But this, does:

<?php
/**
 * Intentionally empty file.
 *
 * It exists to stop directory listings on poorly configured servers.
 *
 * @package           whatever
 */

This is even happening after running the beautifier.

Thus, I think there is something we can improve here, which is, don't assume there should be some code after a file comment

Is this possible? Does this need a new ticket?

smileBeda avatar Jun 29 '21 10:06 smileBeda

@TukuToi The sniff used for this is not a WPCS native sniff, but one from PHPCS itself, so if you want to open an issue about it, it should be done upstream.

As a plugin/theme developer, you're probably best off to ignore these empty index.php files from within your custom ruleset using this snippet:

	<!-- Exclude the 'empty' index files from some documentation checks -->
	<rule ref="Squiz.Commenting.FileComment">
		<exclude-pattern>*/index\.php</exclude-pattern>
	</rule>
	<rule ref="Squiz.Commenting.InlineComment.NoSpaceBefore">
		<exclude-pattern>*/index\.php</exclude-pattern>
	</rule>

... for when the file content is something like this:

<?php
/**
 * Nothing to see here.
 */

Or this code snippet:

	<!-- Exclude the 'empty' index files from some documentation checks -->
	<rule ref="Squiz.Commenting.FileComment">
		<exclude-pattern>*/index\.php</exclude-pattern>
	</rule>

... for when the file content is something like the index.php file you posted above.

Mind: this does presume you have no index.php files with actual code in your project! If you do, you'll need to adjust the exclude patterns to make sure those will still get scanned.

jrfnl avatar Jun 29 '21 10:06 jrfnl

Thanks @jrfnl - since I also have theme code, that is my least preferred solution. I have reported this upstream, sorry I didn't realise that this is a PHP Sniffer issue rather than a WP Sniffer issue. Thanks!

smileBeda avatar Jun 29 '21 11:06 smileBeda

@TukuToi No worries. FYI: The CONTRIBUTING.md doc has information about how to determine whether an issue is a WPCS issue or an upstream one.

jrfnl avatar Jun 29 '21 11:06 jrfnl

This last issue has now been resolved in PHP_CodeSniffer 3.6.1 according https://github.com/squizlabs/PHP_CodeSniffer/issues/3384#issuecomment-882127645

3.6.1 was released October 11th 2021.

Thus every issue within this ticket is resolved - I guess it could be "closed" (even if it is a question and not a actionable task on WPCS side).

smileBeda avatar Nov 17 '21 06:11 smileBeda