DataTables / Editor-PHP

PHP server-side libraries for Editor
Other
35 stars 22 forks source link

Remove composer.php / DATATABLES const #13

Closed mvorisek closed 1 year ago

mvorisek commented 2 years ago

DATATABLES const is kept in config files for security

AllanJard commented 2 years ago

Thanks for the PR. I'm not clear on why if (!defined('DATATABLES')) exit(); is removed from the other files though? I wanted to ensure they couldn't be called directly from the web. I don't see they would do any damage, but just in case...

mvorisek commented 2 years ago

why if (!defined('DATATABLES')) exit(); is removed from the other files though? I wanted to ensure they couldn't be called directly from the web. I don't see they would do any damage, but just in case...

This is absolutely unneeded as including any class is not harmful as the class must be used to be hamful. And if included, the insecure place is not in the class, thus the check is fine to be removed.

mvorisek commented 1 year ago

I'm not sure why if (!defined('DATATABLES')) exit(); has been removed in most cases and reformatted in a few others? I'd rather it stayed in all cases.

In two places the if (!defined('DATATABLES')) exit(); is kept to retain the original functionality. I reformatted them as curly braces are required by the recommended/PSR-12 coding standard.

In all other cases, the checks can/should be removed as they does not make any sense above classes. Loading a class is safe operation in sense of security as nothing is executed. Class itself should also not require any config. composer.php also does not have to be preloaded then and it allows native php classes preloading.

AllanJard commented 1 year ago

Good points - thank you!

AllanJard commented 1 year ago

I've restored the DATATABLES definition and reordered the Bootstrap file a little as the checks were causing it to exit out immediately.

Edit - this was related to:

With composer, the second check will do the work and no global DATATABLES const is even needed.

It needs to work without composer as well, hence my two changes in 136e7a3b105963ff0fd7c445f00ee0398d7a3696.