El Mié 23 Nov 2005 15:16, Thomas -Balu- Walter escribió:

Hello all,

I have two short suggestions regarding include-files:

IMHO the include files (in program/include/ e.g.) should have an ".inc.php" ending to avoid calling them directly in the browser. Since those are not config files it might not be a big security problem, but it might still be possible to identify the roundcube version or other "sensitive" data by having a look at them.

Well, what I normally do is name all the include files with .inc ending and deny access to does files (for example in the .htaccess file in the root directory.

Another idea for those files is to disable access completely:

The files that are allowed to be called in the browser should have a simple define at the beginning:

<?php define('ROUNDCUBE', true); ?>

The include files, etc. can now check for this define and handle it somehow - e.g.:

<?php if (!defined('APPLICATION')) exit('You are not allowed to call this file directly.'); ?>

I don't get your idea. Why should this files ever be called directly? Deny them in the htaccess file and keep coding. :-)

Another solution could be a redirect to the index page using header()?

 Balu

PS: I know, coding-style is a very picky topic and of course personal preference, but is there any chance to make you switch to the PEAR coding standards regarding indents, brackets, ...? :-) -> http://pear.php.net/manual/en/standards.php

I second this!

On Wed, Nov 23, 2005 at 03:45:16PM -0300, Martín Marqués wrote:

OK, just started seeing the program structure, and I don't get whats wrong with it. It's fine, AFAIK. All files inside program/include and program/lib with the exception of the external files (DB.php and MDB2.php) end with .inc and they are denyed in the .htaccess file.

Not everyone is allowed to use .htaccess files or perhaps uses a webserver that does not support them at all.

E.g. I didn't recognize the .htaccess file yet, just the .inc files, tried to call them in the browser and voila - I was able to see the content.[1]

After finding the file I'm not quite sure if I like it too :). As a PHP developer I understand the reason to change some of the settings, but display_errors, session.gc_maxlifetime and session.gc_divisor can and probably should be set in main.inc.php using ini_set() if necessary at all.

Grepping for ini_set also returns some other settings that are changed in index.php and main.inc:

include_path ok, helps avoiding problems with relative paths, etc.

session.name Why is that changed? Is anything wrong with "sid" I prefer or the default "PHPSESSID"?

session.use_cookies I guess cookies are absolute requirements? Or would it work with the trans_sid mechanism for people that paranoically disable cookies in there browser?

error_log log_errors display_errors error_reporting In the alpha development versions I understand the reason for those, but should they be overridden in later versions - probably changing the something the admin does not want?

session.gc_maxlifetime So you don't need that in the .htaccess, do you?

I'd suggest to put them in the main.inc.php config file. Maybe create a section for those: "Recommended PHP settings" or similar. That way people setting up RoundCube will notice them earlier.

The one remaining that can not be changed using ini_set() is upload_max_filesize which I also think the admin of the server should be responsible for. It won't make roundcube stop working if you don't set it, will it?

 Balu

[1] I am the admin of that box, so I can change that ;), but the problem still exists.