Bug #14817
closed
Wrong variable naming with Return-Path configuration
0%
Description
We had problems on one machine, where TYPO3 didn't set the Return-Path of mails to the needed values. I remembered this had been fixed, so why didn't it work?
Looking in the source one finds
/**
* Constructor. If the configuration variable forceReturnPath is set, calls to mail will be called with a 5th parameter.
* See function sendTheMail for more info
*
* @return [type] ...
*/
function t3lib_htmlmail () {
$this->forceReturnPath = $GLOBALS['TYPO3_CONF_VARS']['SYS']['enableReturnPath'];
}
Now, in the install tool is a setting called [forceReturnPath], obviously referring to the part above. But the code looks for *enable*ReturnPath. Consequently, setting enableReturnPath to 1 in localconf.php manually made it work as expected.
Now, looking at the code again I think the flag does in fact not force anything, but enables something. To lessen the impact, we should leave the variable name as it is, but "fix" the install tool. This seems to be fine judging from this ChangeLog entry by Michael Stucki from 2005-05-21:
* Added a new option TYPO3_CONF_VARS[SYS][enableReturnPath] which is
disabled by default. Until this is enabled, the ReturnPath is not sent
by t3lib_htmlmail. Thanks to Jan-Erik Revsbech.
The attached patch renames the variable in config_default.php, thus making it accessible and removing the non-used forceReturnPath one.
(issue imported from #M1197)
Files
Updated by Karsten Dambekalns over 19 years ago
Could you have a look at this, and verify my findings? Thanks!
Updated by Michael Stucki over 19 years ago
Thanks Karsten, yes you are right. Please correct it as soon as HEAD is open again (btw. can you please send a reminder to Kasper as well?).
Maybe you should also change the variable which is called $forceReturnPath the same way.
Important: This must be fixed in the 3.7, 3.8 and in the HEAD branch!
Updated by Karsten Dambekalns over 19 years ago
Ok, I'll fix this in all branches as soon as you give your okay to HEAD changes. I'll change the variable name as well then.
Thanks for the quick feedback.
Updated by Jan-Erik Revsbech over 19 years ago
I agree, this is an error (Sorry for that). Please correct it (when CVS is open again).
On a side note, I think that the variable should be forceReturnPath, and not enable return path (Thus in my opinion t3lib/class.t3lib_htmlmail.php should be corrected, and not t3lib/config_default.php), since the returnpath really is forced by setting the -f flag to sendmail, which forces the return-path to something different than what is stated in the header of the mail (The header of the mail is then rewritten to include the correct return-path by senmail).
Updated by Karsten Dambekalns over 19 years ago
Thanks for explaining the variable naming. This way it makes sense, I had looked at it from a different perspective (where nothing was forced, if there wasn't any value to set the return path to).
Then I'll fix it in the htmlmail class, and probably change wording of the settings' description a bit.
Updated by Jan-Erik Revsbech over 19 years ago
Perfect. You are very welcome to change the wording. The only change needed (I think) in htmlmail is to change
$this->forceReturnPath = $GLOBALS['TYPO3_CONF_VARS']['SYS']['enableReturnPath'];
to
$this->forceReturnPath = $GLOBALS['TYPO3_CONF_VARS']['SYS']['forceReturnPath'];
Updated by Jan-Erik Revsbech over 19 years ago
I changed t3lib_htmlmail so it checks for forceReturnPath instead of enableReturnPath.
Sorry for not catching this error ealier.
Updated by Michael Stucki over 19 years ago
Thanks, Jan-Erik. I have just fixed the bug in the 3.8 branch as well.
Updated by Karsten Dambekalns over 19 years ago
Thanks to you two, sorry for not acting as promised. Has the explanation been touched as well?
Updated by Michael Stucki over 19 years ago
That was not needed since "enableReturnPath" is not mentioned anywhere else.
Updated by Karsten Dambekalns over 19 years ago
Michael, that's true. But it doesn't force the return-path - there isn't any return path set through this option. It rather forces the usage of a certain method of specifying it, if it should be set. No?