4
\$\begingroup\$

I don't know if this method uses a lot of time to process, or if it is bad but I wanted to know if there is any better way of actually doing this? I have a few class files that I want to stop users viewing through the browser, and only allowing them to view it if actually load it from the index.php

So I have this nice little piece of code.

In my index.php I have:

define("SECURE", true);

And I check if it has been defined in class files using

defined("SECURE") or exit('Please define variable SECURE to continue.');

So when they visit class files it actually blocks them from viewing it unless they have loaded index.php first.

Any better way?

\$\endgroup\$
1
  • \$\begingroup\$ Welcome to Code Review! There isn't much code here to review; is it possible that perhaps showing more content of your code files would maybe help reviewers in answering your question(s)? \$\endgroup\$
    – SirPython
    Commented Feb 18, 2016 at 14:26

2 Answers 2

3
\$\begingroup\$

"Common and not bad"

You way is the most common way of implementing such mechanism. Message however seem to be misleading for end-user and too helpful for potential wrong-doer, so you could simply send a 404 error header and exit, instead.

In including file:

define("INCLUDING", true);

In the file meant to be included:

if (!defined("INCLUDING")) {
  header($_SERVER["SERVER_PROTOCOL"] . " 404 Not Found");
  exit;
}


"Better"

Other approach looks, for example, like this:

if (basename($_SERVER['PHP_SELF']) === basename(__FILE__)) {
  header($_SERVER["SERVER_PROTOCOL"] . " 404 Not Found");
  exit;
}
  • basename($_SERVER["PHP_SELF"]) will give you the current filename, and
  • basename(__FILE__) will give you the name of file it is being called from (__FILE__ is so called magic constant).

This approach has that advantage that you don't need to define constant variable in files that will include the "include-only" files (no need for define("INCLUDING", true); each time you include).

You can put count(get_included_files()) === 1 in your if's body instead, too (replace 1 with 0 for PHP's versions earlier than 5.0).


"Best"

However, it would be best, if you would simply move all your "include-only" files into a particular directory, to which you would deny access.

For apache, example code to put in your server config file looks like this:

<Directory /var/www/includes/>
  Order deny,allow
  Deny from all
</Directory>

If you drop first and last line of the above code, you can put it in the .htaccess file in the said directory you want to restrict access to, but it's preferred that you don't create .htaccess files if you don't have to, for performance reasons.

For Nginx:

location /includes {
  deny all;
  return 404;
}
\$\endgroup\$
0
2
\$\begingroup\$

Your approach seems good to me.

You can find some alternatives here, but most have downsides (not every server parses htaccess files, get_included_files is self-contained but works differently for different PHP versions, etc).

Additionally, you should think about moving everything outside the web-root that should not be accessible directly.

You might also want to rethink how you handle the error. Using an imperative in the error message gives the impression that the user can fix the error, which they can't. This might confuse legitimate users that accidentally accessed the script. So instead of Please do X, something like You can't access X would be better.

Ideally, you would handle the case of accessing an existing but forbidden PHP file exactly like a non-existing PHP file is handled (for example by showing a custom 404 error page).

And lastly, SECURE isn't that great of a name, as it's very generic. Something like IS_INCLUDED, NO_DIRECT_SCRIPT_ACCESS, or similar might be better.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.