Discussion:
[Vtigercrm-developers] ancient class.phpmailer.php
Adam Heinz
2012-10-09 16:00:59 UTC
Permalink
Even though it was only checked in a year ago, the PHPMailer class
included with vtiger 5.4 [1] is from 2003. It does not support
connecting to an SMTP server using TLS, though the current version [2]
does.

[1] http://trac.vtiger.com/cgi-bin/trac.cgi/browser/vtigercrm/branches/5.4.0/modules/Emails/class.phpmailer.php
[2] https://code.google.com/a/apache-extras.org/p/phpmailer/source/browse/trunk/class.phpmailer.php
Prasad
2012-10-10 06:32:23 UTC
Permalink
Adam,

Would be great if you can submit a patch that uses latest phpmailer against
5.4.0

Regards,
Prasad
Post by Adam Heinz
Even though it was only checked in a year ago, the PHPMailer class
included with vtiger 5.4 [1] is from 2003. It does not support
connecting to an SMTP server using TLS, though the current version [2]
does.
[1]
http://trac.vtiger.com/cgi-bin/trac.cgi/browser/vtigercrm/branches/5.4.0/modules/Emails/class.phpmailer.php
[2]
https://code.google.com/a/apache-extras.org/p/phpmailer/source/browse/trunk/class.phpmailer.php
_______________________________________________
http://www.vtiger.com/
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.vtigercrm.com/pipermail/vtigercrm-developers/attachments/20121010/f51f0c8d/attachment.html
Adam Heinz
2012-10-10 14:11:01 UTC
Permalink
Post by Prasad
Would be great if you can submit a patch that uses latest phpmailer against
5.4.0
http://trac.vtiger.com/cgi-bin/trac.cgi/ticket/7515

This patch upgrades to phpmailer 5.1. There is a slightly newer 5.2,
but some of the other PHP tools we use also use 5.1, so I went with
the version that I have direct experience with.
Adam Heinz
2012-10-10 14:52:56 UTC
Permalink
Since I just smashed the new phpmailer over top, I'm reviewing the
changes made to it over the years [1]. Even going back in the source
code as far as 4.0, I couldn't find a pristine phpmailer 1.7.2 source
file. Even the phpmailer site only has source as far back as 2.0, so
there was no good way to apply the phpmailer 1.7.2 - 5.1.0 delta.

I think the best course of action is to use a pristine phpmailer so
that subsequent changes are easier to include, and to reimplement the
following changes outside of the phpmailer files.

Review required:
@12734 - default timeout set, reapply
@11832 - default language set, reapply
@11362 - charset set, reapply
@11210 - html entity decoding, not sure
@9762 - html entity decoding, not sure

Reviewed, but no follow up necessary:
@13487 - similar change made, no action
@11389 - code no longer exists, no action
@10378 - charset set, no action
@10370 - charset set, no action
@7150 - file size, no action
@7003 - bad search/replace, no action
@3329 - version upgrade, no action
@83 - error messaging, no action

I also just noticed that there is a second copy of the files in cron?
This is somewhat concerning, as the majority of the fixes were only
applied to the modules/Emails files.

[1] http://trac.vtiger.com/cgi-bin/trac.cgi/log/vtigercrm/branches/5.4.0/modules/Emails/class.phpmailer.php?action=stop_on_copy&rev=13577&stop_rev=&mode=follow_copy
Prasad
2012-10-10 17:55:09 UTC
Permalink
Adam,

Thank you for quick review - we follow up on this thread.

Regards,
Prasad
Post by Adam Heinz
Since I just smashed the new phpmailer over top, I'm reviewing the
changes made to it over the years [1]. Even going back in the source
code as far as 4.0, I couldn't find a pristine phpmailer 1.7.2 source
file. Even the phpmailer site only has source as far back as 2.0, so
there was no good way to apply the phpmailer 1.7.2 - 5.1.0 delta.
I think the best course of action is to use a pristine phpmailer so
that subsequent changes are easier to include, and to reimplement the
following changes outside of the phpmailer files.
@12734 - default timeout set, reapply
@11832 - default language set, reapply
@11362 - charset set, reapply
@11210 - html entity decoding, not sure
@9762 - html entity decoding, not sure
@13487 - similar change made, no action
@11389 - code no longer exists, no action
@10378 - charset set, no action
@10370 - charset set, no action
@7150 - file size, no action
@7003 - bad search/replace, no action
@3329 - version upgrade, no action
@83 - error messaging, no action
I also just noticed that there is a second copy of the files in cron?
This is somewhat concerning, as the majority of the fixes were only
applied to the modules/Emails files.
[1]
http://trac.vtiger.com/cgi-bin/trac.cgi/log/vtigercrm/branches/5.4.0/modules/Emails/class.phpmailer.php?action=stop_on_copy&rev=13577&stop_rev=&mode=follow_copy
_______________________________________________
http://www.vtiger.com/
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.vtigercrm.com/pipermail/vtigercrm-developers/attachments/20121010/2c7fa681/attachment.html
Adam Heinz
2012-10-11 21:21:17 UTC
Permalink
Just added a second patch to allow admin to set TLS/SSL from Settings
Outgoing Server. I seem to have double-submitted the patch -- can
you delete the .2 version from the ticket? #7515
Prasad
2012-10-12 16:37:06 UTC
Permalink
Adam,

Don't worry. Thanks for the submission.

Regards,
Prasad
Post by Adam Heinz
Just added a second patch to allow admin to set TLS/SSL from Settings
Outgoing Server. I seem to have double-submitted the patch -- can
you delete the .2 version from the ticket? #7515
_______________________________________________
http://www.vtiger.com/
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.vtigercrm.com/pipermail/vtigercrm-developers/attachments/20121012/0c99aeab/attachment.html
Adam Heinz
2012-10-15 19:27:53 UTC
Permalink
I just noticed there's also a vtlib/Vtiger/Mailer.php that extends
PHPMailer, but doesn't appear to be used anywhere in the codebase.
This is a great example of not eating your own dogfood; whenever
possible, the core modules should be built on top of vtlib.
Prasad
2012-10-16 05:55:04 UTC
Permalink
Adam,

Mail notification trigger and dispatch is spread across in the codebase,
there was a design discussion to simplify this first before consuming the
vtlib/Vtiger/Mailer.php. A reason of not biting it soon.

vtlib/Vtiger/Mailer.php - is being used by serveral utlitly tools we use on
day-to-day basis for support.
I'm sure there would be much better and smarter implementation of the same.
Watching for the same.

Regards,
Prasad
Post by Adam Heinz
I just noticed there's also a vtlib/Vtiger/Mailer.php that extends
PHPMailer, but doesn't appear to be used anywhere in the codebase.
This is a great example of not eating your own dogfood; whenever
possible, the core modules should be built on top of vtlib.
_______________________________________________
http://www.vtiger.com/
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.vtigercrm.com/pipermail/vtigercrm-developers/attachments/20121016/415359d5/attachment.html
Loading...