[TYPO3-core] RFC: #11731: [Bugfix] ENABLE_INSTALL_TOOL file check in yellow box doesn't check the file age

[Moreno Feltscher]

Of course. I thought I sent one but here it is:

--- t3lib/class.t3lib_befunc.php	2009-08-17 17:34:49.000000000 +0200
+++ t3lib/class.t3lib_befunc.php	2009-08-17 17:50:42.000000000 +0200
@@ -4029,7 +4029,7 @@
  	public static function displayWarningMessages() {
  		if ($GLOBALS['BE_USER']->isAdmin()) {
  			$warnings = array();	// Array containing warnings that must be  
displayed
-			$enableInstallToolFile = PATH_site.'typo3conf/ENABLE_INSTALL_TOOL';	//  
If this file exists, the Install Tool is enabled
+			$enableInstallToolFile = PATH_site .  
'typo3conf/ENABLE_INSTALL_TOOL';	// If this file exists, it doesn't  
contain the word "KEEP_FILE" and it isn't older than one hour, the Install  
Tool is enabled

  			$cmd = t3lib_div::_GET('adminWarning_cmd');	// Cleanup command, if set
  			switch($cmd) {
@@ -4062,7 +4062,9 @@
  			}
  			$GLOBALS['TYPO3_DB']->sql_free_result($res);

-			if (@is_file($enableInstallToolFile)) {
+			$content = file_get_contents($enableInstallToolFile);
+			$verifyString = 'KEEP_FILE';
+			if (trim($content) == $verifyString) {
  				$url =  
t3lib_div::getIndpEnv('TYPO3_REQUEST_SCRIPT').'?adminWarning_cmd=remove_ENABLE_INSTALL_TOOL';
  				$warnings['install_enabled'] = sprintf(
  					$GLOBALS['LANG']->sL('LLL:EXT:lang/locallang_core.xml:warning.install_enabled'),

--- typo3/sysext/lang/locallang_core.xml	2009-08-07 21:32:36.000000000  
+0200
+++ typo3/sysext/lang/locallang_core.xml	2009-08-17 17:50:16.000000000  
+0200
@@ -241,7 +241,7 @@
  			<label index="warning.backend_admin">The default backend user "admin"  
with password &quot;password&quot; is still present. %sEdit this  
account%s, either deleting it completely or changing the username and  
password.</label>
  			<label index="warning.file_deny_pattern">The value of fileDenyPattern  
is not set to its default:%s If TYPO3 is running on Apache, a customized  
value might enable backend or frontend users to execute malicious php  
scripts.</label>
  			<label index="warning.file_deny_htaccess">The current value of  
fileDenyPattern allows to upload/create files with the name ".htaccess".  
If TYPO3 is running on Apache, this enables backend or frontend users to  
create and execute php scripts. Please reset the value of fileDenyPattern  
to its default.</label>
-			<label index="warning.install_enabled">The Install Tool is enabled.  
Delete the file &quot;%s&quot; when you have finished setting up  
TYPO3.</label>
+			<label index="warning.install_enabled">The Install Tool is permanently  
enabled. Delete the file &quot;%s&quot; when you have finished setting up  
TYPO3.</label>
  			<label index="warning.install_enabled_cmd">Click to remove the file  
now!</label>
  			<label index="warning.install_encryption">The encryption key is not  
set. Set it in the %sBasic Configuration section%s of the Install  
Tool.</label>
  			<label index="warning.install_update">This installation is not  
configured for the TYPO3 version it is running. If you did so  
intentionally, this message can be safely ignored. If you are unsure,  
visit the %sUpdate Wizard%s section of the Install Tool to see how TYPO3  
would change.</label>


On Mon, 24 Aug 2009 09:35:42 +0200, Steffen Ritter <info at rs-websystems.de>  
wrote:

> Moreno Feltscher schrieb:
>> Hi Stucki "Wunsch-Bündner" ;)
>>   So we now have different solutions for this:
>> 1) leave it as it is (personally I don't think this is a good idea  
>> because it confuses people an the message is definitely wrong)
>> 2) my solution with file age check and KEEP_FILE check (maybe a little  
>> bit too much..)
>> 3) display a message only if KEEP_FILE is TRUE (my favorite one, the  
>> file will be deleted after one hour and admins have not to care about)
>> 4) a remove function in typo3/init.php (I vote against this one because  
>> this will cost some performance due to filesystem access)
>>  So what do you think about 3)?
>>  I would write a patch for this one if nobody objects.
>>   Cheers
>>   On Mon, 17 Aug 2009 12:39:29 +0200, Michael Stucki  
>> <michael at typo3.org> wrote:
>>
>>> Hi Francois & Moreno Party-Man ;-)
>>>
>>>>> No, you didn't get anything wrong. The problem is it could confuse
>>>>> people because the file will only be removed after the install tool
>>>>> was accessed. If you see the warning in the yellow box and you want  
>>>>> to
>>>>> check whether the install tool is accessible it could be that your
>>>>> install tool deletes the file.
>>>>>
>>>>> The bug is more about "confusing people" than a technical change.
>>>
>>> I agree. Actually the real problem is that the file is already
>>> "scheduled" for removing but it will not be done before you access the
>>> Install Tool. So maybe the file removal should be integrated in
>>> typo3/init.php instead?
>>>
>>>> I understand better what you mean now. However, I'm still unfavorable  
>>>> to
>>>> the change you suggest: the fact is that the ENABLE_INSTALL_TOOL file
>>>> exists at that point and something should be done about deleting it. I
>>>> agree that it may cause some confusion, but the proposed is fixed is  
>>>> not
>>>> a good solution IMO.
>>>>
>>>> Moreover your patch is wrong, because it doesn't take into account the
>>>> "KEEP_FILE" feature.
>>>
>>> True. Now by copying the removal code from typo3/install/index.php to
>>> typo3/init.php this would be considerated, so nothing would need to be
>>> changed in class.t3lib_befunc.php.
>>>
>>> What do you think?
>>>
>>> - michael
>>>
>>
> Hi Moreno,
> I think we have enough +1 ones on idea 3, could you finish this up,  
> providing a patch? (which is not inline the message)
>
> regards
>
> Steffen


--