repository/lib.php library is the only one to explicitly include the main config.php file

Matteo Scaramuccia added a comment - 11/Oct/12 2:43 PM Starting from 2.3, the new amazing DnD has added new files to the long chain of files loaded per request; among these new ones there is repository/lib.php : course/dnduploadlib.php * Library to handle drag and drop course uploads ... require_once($CFG->dirroot.'/repository/lib.php'); ... Under some configurations - tipically running under $CFG->reverseproxy - I'm having HTTP 500 issues due to the incorrect resolving of the supposed path for the config.php requested by repository/lib.php . # strace -ttfp 3132 ... 08:15:11.358227 stat64( "/ var /www/moodle-master-git-remote/course/dnduploadlib.php" , {st_mode=S_IFREG|0644, st_size=26483, ...}) = 0 08:15:11.358891 fcntl64(19, F_SETLKW, {type=F_WRLCK, whence=SEEK_SET, start=0, len=1}) = 0 08:15:11.359320 fcntl64(19, F_SETLKW, {type=F_UNLCK, whence=SEEK_SET, start=0, len=1}) = 0 08:15:11.359888 munmap(0xafe4e000, 26483) = 0 08:15:11.360168 close(24) = 0 08:15:11.360786 time(NULL) = 1349849711 08:15:11.361084 time(NULL) = 1349849711 08:15:11.362963 open( "/ var /www/moodle-master-git-remote/repository/lib.php" , O_RDONLY) = 24 08:15:11.363142 fstat64(24, {st_mode=S_IFREG|0644, st_size=105513, ...}) = 0 08:15:11.363400 fstat64(24, {st_mode=S_IFREG|0644, st_size=105513, ...}) = 0 08:15:11.363643 fstat64(24, {st_mode=S_IFREG|0644, st_size=105513, ...}) = 0 08:15:11.364349 mmap2(NULL, 105513, PROT_READ, MAP_SHARED, 24, 0) = 0xafe3b000 08:15:11.364604 stat64( "/ var /www/moodle-master-git-remote/repository/lib.php" , {st_mode=S_IFREG|0644, st_size=105513, ...}) = 0 08:15:11.364890 fcntl64(19, F_SETLKW, {type=F_WRLCK, whence=SEEK_SET, start=0, len=1}) = 0 08:15:11.365518 fcntl64(19, F_SETLKW, {type=F_UNLCK, whence=SEEK_SET, start=0, len=1}) = 0 08:15:11.365876 munmap(0xafe3b000, 105513) = 0 08:15:11.366115 close(24) = 0 08:15:11.366734 time(NULL) = 1349849711 08:15:11.366827 time(NULL) = 1349849711 08:15:11.366913 open( "/ var /www/moodle-master-git-remote/config.php" , O_RDONLY) = 24 08:15:11.367049 fstat64(24, {st_mode=S_IFREG|0440, st_size=910, ...}) = 0 08:15:11.367178 fstat64(24, {st_mode=S_IFREG|0440, st_size=910, ...}) = 0 08:15:11.367485 fstat64(24, {st_mode=S_IFREG|0440, st_size=910, ...}) = 0 08:15:11.368337 mmap2(NULL, 910, PROT_READ, MAP_SHARED, 24, 0) = 0xafe54000 08:15:11.368680 stat64( "/ var /www/moodle-master-git-remote/config.php" , {st_mode=S_IFREG|0440, st_size=910, ...}) = 0 08:15:11.369530 fcntl64(19, F_SETLKW, {type=F_WRLCK, whence=SEEK_SET, start=0, len=1}) = 0 08:15:11.369749 fcntl64(19, F_SETLKW, {type=F_UNLCK, whence=SEEK_SET, start=0, len=1}) = 0 08:15:11.370408 munmap(0xafe54000, 910) = 0 08:15:11.370620 close(24) = 0 08:15:11.370960 time(NULL) = 1349849711 08:15:11.371223 time(NULL) = 1349849711 08:15:11.371839 lstat64( "/filelib.php" , 0xbf8ab9b0) = -1 ENOENT (No such file or directory) 08:15:11.372088 time(NULL) = 1349849711 08:15:11.372363 lstat64( "/filelib.php" , 0xbf8ab8e0) = -1 ENOENT (No such file or directory) 08:15:11.372583 time(NULL) = 1349849711 08:15:11.373174 lstat64( "/filelib.php" , 0xbf8ad9b0) = -1 ENOENT (No such file or directory) 08:15:11.373607 open( "/filelib.php" , O_RDONLY) = -1 ENOENT (No such file or directory) ... Everything should be easily resolved considering the fact that the Repository library file is the only one to explicitly require the main config.php file: 2.4 (master, at this time): $ find /var/www/moodle-master/ -type f -name lib.php |xargs grep config\.php : / var /www/moodle-master/portfolio/mahara/lib.php: return $CFG->wwwroot . '/portfolio/mahara/preconfig.php?id=' . $ this ->exporter->get('id'); / var /www/moodle-master/mod/chat/lib.php: if (file_exists($CFG->dirroot . '/mod/chat/gui_ajax/theme/'.$theme.'/config.php')) { / var /www/moodle-master/mod/chat/lib.php: include($CFG->dirroot . '/mod/chat/gui_ajax/theme/'.$theme.'/config.php'); / var /www/moodle-master/repository/lib.php:require_once(dirname(dirname(__FILE__)) . '/config.php'); 2.3: $ find /var/www/moodle-23/ -type f -name lib.php |xargs grep config\.php : / var /www/moodle-23/portfolio/mahara/lib.php: return $CFG->wwwroot . '/portfolio/mahara/preconfig.php?id=' . $ this ->exporter->get('id'); / var /www/moodle-23/mod/chat/lib.php: if (file_exists($CFG->dirroot . '/mod/chat/gui_ajax/theme/'.$theme.'/config.php')) { / var /www/moodle-23/mod/chat/lib.php: include($CFG->dirroot . '/mod/chat/gui_ajax/theme/'.$theme.'/config.php'); / var /www/moodle-23/repository/lib.php:require_once(dirname(dirname(__FILE__)) . '/config.php'); Indeed I've removed that line: diff --git a/repository/lib.php b/repository/lib.php index 5b34c56..8482a91 100644 --- a/repository/lib.php +++ b/repository/lib.php @@ -23,7 +23,6 @@ * @license http: //www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -require_once(dirname(dirname(__FILE__)) . '/config.php'); require_once($CFG->libdir . '/filelib.php'); require_once($CFG->libdir . '/formslib.php'); and while Moodle seems to run happily, my code is working too . Moodle seems to work fine without that line 'cause that library is always included when config.php i.e. $CFG has been already loaded: $ find /var/www/moodle-master/ -type f -name lib.php |xargs grep repository/lib\.php / var /www/moodle-master/mod/assignment/lib.php: require_once( "$CFG->dirroot/repository/lib.php" ); / var /www/moodle-master/repository/dropbox/lib.php:require_once($CFG->dirroot . '/repository/lib.php'); / var /www/moodle-master/repository/wikimedia/lib.php:require_once($CFG->dirroot . '/repository/lib.php'); / var /www/moodle-master/repository/alfresco/lib.php:require_once($CFG->dirroot . '/repository/lib.php'); / var /www/moodle-master/repository/flickr/lib.php:require_once($CFG->dirroot . '/repository/lib.php'); / var /www/moodle-master/repository/coursefiles/lib.php:require_once($CFG->dirroot . '/repository/lib.php'); / var /www/moodle-master/repository/user/lib.php:require_once($CFG->dirroot . '/repository/lib.php'); / var /www/moodle-master/repository/youtube/lib.php:require_once($CFG->dirroot . '/repository/lib.php'); / var /www/moodle-master/repository/url/lib.php:require_once($CFG->dirroot . '/repository/lib.php'); / var /www/moodle-master/repository/upload/lib.php:require_once($CFG->dirroot . '/repository/lib.php'); / var /www/moodle-master/repository/boxnet/lib.php:require_once($CFG->dirroot . '/repository/lib.php'); / var /www/moodle-master/repository/recent/lib.php:require_once($CFG->dirroot . '/repository/lib.php'); / var /www/moodle-master/repository/s3/lib.php:require_once($CFG->dirroot . '/repository/lib.php'); / var /www/moodle-master/repository/equella/lib.php:require_once($CFG->dirroot . '/repository/lib.php'); / var /www/moodle-master/repository/webdav/lib.php:require_once($CFG->dirroot . '/repository/lib.php'); / var /www/moodle-master/repository/filesystem/lib.php:require_once($CFG->dirroot . '/repository/lib.php'); / var /www/moodle-master/repository/googledocs/lib.php:require_once($CFG->dirroot . '/repository/lib.php'); / var /www/moodle-master/repository/merlot/lib.php:require_once($CFG->dirroot . '/repository/lib.php'); / var /www/moodle-master/repository/local/lib.php:require_once($CFG->dirroot . '/repository/lib.php'); / var /www/moodle-master/repository/picasa/lib.php:require_once($CFG->dirroot . '/repository/lib.php'); / var /www/moodle-master/repository/flickr_public/lib.php:require_once($CFG->dirroot . '/repository/lib.php'); find /var/www/moodle-23/ -type f -name lib.php |xargs grep repository/lib\.php / var /www/moodle-23/mod/assignment/lib.php: require_once( "$CFG->dirroot/repository/lib.php" ); / var /www/moodle-23/repository/dropbox/lib.php:require_once($CFG->dirroot . '/repository/lib.php'); / var /www/moodle-23/repository/wikimedia/lib.php:require_once($CFG->dirroot . '/repository/lib.php'); / var /www/moodle-23/repository/alfresco/lib.php:require_once($CFG->dirroot . '/repository/lib.php'); / var /www/moodle-23/repository/flickr/lib.php:require_once($CFG->dirroot . '/repository/lib.php'); / var /www/moodle-23/repository/coursefiles/lib.php:require_once($CFG->dirroot . '/repository/lib.php'); / var /www/moodle-23/repository/user/lib.php:require_once($CFG->dirroot . '/repository/lib.php'); / var /www/moodle-23/repository/youtube/lib.php:require_once($CFG->dirroot . '/repository/lib.php'); / var /www/moodle-23/repository/url/lib.php:require_once($CFG->dirroot . '/repository/lib.php'); / var /www/moodle-23/repository/upload/lib.php:require_once($CFG->dirroot . '/repository/lib.php'); / var /www/moodle-23/repository/boxnet/lib.php:require_once($CFG->dirroot . '/repository/lib.php'); / var /www/moodle-23/repository/recent/lib.php:require_once($CFG->dirroot . '/repository/lib.php'); / var /www/moodle-23/repository/s3/lib.php:require_once($CFG->dirroot . '/repository/lib.php'); / var /www/moodle-23/repository/equella/lib.php:require_once($CFG->dirroot . '/repository/lib.php'); / var /www/moodle-23/repository/webdav/lib.php:require_once($CFG->dirroot . '/repository/lib.php'); / var /www/moodle-23/repository/filesystem/lib.php:require_once($CFG->dirroot . '/repository/lib.php'); / var /www/moodle-23/repository/googledocs/lib.php:require_once($CFG->dirroot . '/repository/lib.php'); / var /www/moodle-23/repository/merlot/lib.php:require_once($CFG->dirroot . '/repository/lib.php'); / var /www/moodle-23/repository/local/lib.php:require_once($CFG->dirroot . '/repository/lib.php'); / var /www/moodle-23/repository/picasa/lib.php:require_once($CFG->dirroot . '/repository/lib.php'); / var /www/moodle-23/repository/flickr_public/lib.php:require_once($CFG->dirroot . '/repository/lib.php'); I'm not sure about such a deletion - that's the reason why this issue has been filed as an improvement a not as a bug - BUT looking at the overall libraries code design this is mostly a small but required cleanup/improvement, if confirmed, - please note - regardless it will resolve an issue for my specific use case.

Matteo Scaramuccia added a comment - 12/Oct/12 2:19 PM Going futher with the investigations about the "including config.php " convention, now for *lib.php file libraries, highlights one more potential issue besides the one for repository/lib.php : repository/dropbox/locallib.php , that could be cleaned up too Note: there is also a special lib(eration) which includes (not requires!) config.php BUT that is actually a page, the famous Easter egg lib/womenslib.php $ find /var/www/moodle-master/ -type f -name *lib.php |xargs grep config\.php | grep require / var /www/moodle-master/lib/adminlib.php: * require(dirname(dirname(dirname(__FILE__))).'/config.php'); / var /www/moodle-master/lib/medialib.php: // May be defined in config.php if required. / var /www/moodle-master/lib/medialib.php: // Default video height. May be defined in config.php if required. / var /www/moodle-master/lib/medialib.php: // May be defined in config.php if required. / var /www/moodle-master/lib/xsendfilelib.php: // Nginx requires paths relative to aliases, you need to specify them in config.php / var /www/moodle-master/repository/dropbox/locallib.php:require_once(dirname(dirname(dirname(__FILE__))).'/config.php'); / var /www/moodle-master/repository/lib.php:require_once(dirname(dirname(__FILE__)) . '/config.php'); $ find /var/www/moodle-23/ -type f -name *lib.php |xargs grep config\.php | grep require / var /www/moodle-23/lib/adminlib.php: * require(dirname(dirname(dirname(__FILE__))).'/config.php'); / var /www/moodle-23/lib/medialib.php: // May be defined in config.php if required. / var /www/moodle-23/lib/medialib.php: // Default video height. May be defined in config.php if required. / var /www/moodle-23/lib/medialib.php: // May be defined in config.php if required. / var /www/moodle-23/lib/xsendfilelib.php: // Nginx requires paths relative to aliases, you need to specify them in config.php / var /www/moodle-23/repository/dropbox/locallib.php:require_once(dirname(dirname(dirname(__FILE__))).'/config.php'); / var /www/moodle-23/repository/lib.php:require_once(dirname(dirname(__FILE__)) . '/config.php');

Michael de Raadt added a comment - 15/Oct/12 2:42 PM Thanks for reporting that and providing a patch.

Frédéric Massart added a comment - 16/Oct/12 2:08 PM Thanks for your patch and feedback Matteo. I think it is safe to remove the inclusion but only in master. There could be some plugins which rely on this inclusion and we do not want to break them. I have kept your patch and added a commit on top of it to remove the inclusion from locallib.php (nice spotting!) and to add the `defined('MOODLE_INTERNAL') || die();` statement. Many thanks! Fred

Ankit Agarwal added a comment - 16/Oct/12 2:35 PM Hi Fred, The changes look good. However if we are cleaning this up, there are a whole bunch of such usage which needs to be reviewed. A simple grep will show you we have widespread use of this. This needs to be taken care of as well, you can update this issues description and do it in this one or feel free to create a new one. Thanks

Frédéric Massart added a comment - 17/Oct/12 3:13 PM - edited Thanks Ankit! I'd rather not take care of those in the scope of this issue, the main reason being that I know this area a little bit and really the other ones. I will raise an issue to take care of it. Thanks! Edit: Actually decided not to create another issue about it because I don't have any specific file to point to, and each file resulting in the following command should be manually checked: find . -iname "*.php" -print0 | xargs -0 grep -E '(require|include).*config\.php'

Ankit Agarwal added a comment - 17/Oct/12 3:25 PM Hi Fred, A generic issue should be fine. Just so that this discussion doesnt get lost. Thanks

Aparup Banerjee added a comment - 19/Oct/12 9:50 AM The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao

Dan Poltawski added a comment - 29/Oct/12 2:32 PM Integrated, thanks fred

Rossiani Wijaya added a comment - 31/Oct/12 12:07 PM This is working great. Test passed.

Dan Poltawski added a comment - 01/Nov/12 3:03 PM Hurray! You did it, congratulations! You have on Mojito credit to redeem after the release of Moodle 2.4