php files with the executable bit on

[jelle]

No the executable bit is not some fancy dress

i noticed some files in my  0.11.2 install were executable, so I downloaded the last beta (0.12beta) to see how the situation is there:

Code: Select all

jib@nietsch:~/temp/cmsmadesimple-0.12beta2$ find . -type f -perm /a=x
./plugins/function.cms_versionname.php
./admin/lang/lt_LT/admin.inc.php
./admin/lang/es_ES.nls.php
./admin/lang/sk_SK.nls.php
./admin/lang/lt_LT.nls.php
./admin/lang/ru_RU.nls.php
./tmp/cache/SITEDOWN
./lib/convert/License
./lib/convert/ConvertTables/windows-1250
./lib/convert/ConvertTables/cp037
./lib/convert/ConvertTables/cp850
./lib/convert/ConvertTables/cp437
./lib/convert/ConvertTables/cp852
./lib/convert/ConvertTables/cp737
./lib/convert/ConvertTables/gsm0338
./lib/convert/ConvertTables/cp855
./lib/convert/ConvertTables/cp874
./lib/convert/ConvertTables/cp775
./lib/convert/ConvertTables/cp856
./lib/convert/ConvertTables/cp875
./lib/convert/ConvertTables/cp857
./lib/convert/ConvertTables/mazovia
./lib/convert/ConvertTables/cp1006
./lib/convert/ConvertTables/cp1026
./lib/convert/ConvertTables/us-ascii
./lib/convert/ConvertTables/windows-1251
./lib/convert/ConvertTables/stdenc
./lib/convert/ConvertTables/windows-1252
./lib/convert/ConvertTables/windows-1253
./lib/convert/ConvertTables/iso-8859-10
./lib/convert/ConvertTables/windows-1254
./lib/convert/ConvertTables/iso-8859-11
./lib/convert/ConvertTables/windows-1255
./lib/convert/ConvertTables/windows-1256
./lib/convert/ConvertTables/iso-8859-13
./lib/convert/ConvertTables/windows-1257
./lib/convert/ConvertTables/iso-8859-14
./lib/convert/ConvertTables/cp500
./lib/convert/ConvertTables/windows-1258
./lib/convert/ConvertTables/iso-8859-15
./lib/convert/ConvertTables/iso-8859-16
./lib/convert/ConvertTables/turkish
./lib/convert/ConvertTables/cp424
./lib/convert/ConvertTables/cp860
./lib/convert/ConvertTables/cp861
./lib/convert/ConvertTables/cp862
./lib/convert/ConvertTables/iso-8859-1
./lib/convert/ConvertTables/cp863
./lib/convert/ConvertTables/iso-8859-2
./lib/convert/ConvertTables/cp864
./lib/convert/ConvertTables/cp865
./lib/convert/ConvertTables/iso-8859-3
./lib/convert/ConvertTables/cp866
./lib/convert/ConvertTables/iso-8859-4
./lib/convert/ConvertTables/iso-8859-5
./lib/convert/ConvertTables/iso-8859-6
./lib/convert/ConvertTables/cp869
./lib/convert/ConvertTables/iso-8859-7
./lib/convert/ConvertTables/koi8-r
./lib/convert/ConvertTables/iso-8859-8
./lib/convert/ConvertTables/iso-8859-9
./lib/convert/ConvertTables/koi8-u
./lib/convert/ConvertCharset.class.php
./lib/adodb_lite/adodbSQL_drivers/sqlite/sqlite_cmsms_module.inc
./lib/adodb_lite/adodbSQL_drivers/mysqli/mysqli_cmsms_module.inc
./lib/classes/class.content.inc.php
./lib/contenttypes/Separator.inc.php
./lib/contenttypes/Content.inc.php
./lib/contenttypes/Link.inc.php
./lib/contenttypes/SectionHeader.inc.php
./install/upgrades/upgrade.9.to.10.php
./install/upgrades/upgrade.10.to.11.php
./install/upgrades/upgrade.11.to.12.php
./install/upgrades/upgrade.12.to.13.php
./install/upgrades/upgrade.13.to.14.php
./install/upgrades/upgrade.14.to.15.php
./install/upgrades/upgrade.15.to.16.php
./install/upgrades/upgrade.16.to.17.php
./install/upgrades/upgrade.17.to.18.php
./install/upgrades/upgrade.8.to.9.php
./modules/News/lang/ext/nl_NL.php
./modules/News/lang/lt_LT.php

I can be wrong about this, but I think this is not intentional. Maybe some file should be executable, but I don't think there are any.

here is a quick shellscript to test for this situation

Code: Select all

! /bin/sh
FILES=`find . -type f -perm /a=x|grep -v ".sh"`
if [ -n "$FILES" ]
then
        echo -e "$FILES \n   these files are still executable. Are you sure they should?"
 1>&2
        exit 1
fi

Maybe it would be better to write something similar like this in php, but that would have taken me some more time to produce. (offcourse, this will not work on windows without a proper shell. I assume that is where the errors originate too)

[calguy1000]

Yeah, I don't think any of the php files need the executable bit on. and although it is probably a minor security hole, it is indeed a security flaw.  Could you please submit  a bug in the forge for this so it can be taken care of in the install and upgrade routines.

SVN records the permissions of files when they're checked in, and in the course of testing, permissions sometimes get blown open, or when doing cross platform things with samba, etc. 

It's a detail that should be taken care of.

[Ted]

Thanks for that.  It'll be fixed when 0.12 comes out.

[calguy1000]

maybe there should be a "chmod -R -x *php" on upgrade and on install just to take care of this problem in perpetuity.  Infact, that could go into the module upgrade and install process too.

[Ted]

Well, the chances of the web server owning these files is stil pretty slim...

However, I am going to add that to my build script to make sure that they're set correctly when the tarball is put together.

[jelle]

well, you could, and maybe you should.
But it touches on another issue: there are no (unit-) tests!
As far as I have understood correctly, all testing is done by hand. that will quickly wear out all testing volunteers, or not all tests(? previous bug reports I suppose?) will be run before a release.

How would the (core)developers react to a strategy like XP(extreme programming)'s 'test first'?

Personally, I think that it would be a good way to insure the code keeps working and will be more easily refactorable (no need for another phpnuke etc).  On the other hand it might mean that you'd need to write 1 line of testcode (and one line of documenattion while you are at it) for each line of product code.

[Ted]

I'd be all for this.  However, I've never seen good testing strategies for php applications.  If the tests take longer to write than the code, it'll never get done.  Let's be honest, we barely have time to write what we have.  Adding triple the work with our limited team will pretty much grind this project to screaching halt.