[wp-hackers] PHPMailer was a mistake

[Jacob Santos]

Damn, I woke up and said to myself, "I didn't really hit send did I?" A 
no harshness reply.

Computer Guru wrote:
> I think that's a very important point Peter raises.
>
> In reality, developers aren't supposed to do what's "right" assuming
> they're starting from scratch at this very moment, but what's right
> for the project as a whole and its users too - keeping in mind where
> you started and where you are now.
>   
Yeah, I would say that you should never refactor legacy code. Right? I 
don't think so, the argument is whether PHPMailer should be replaced 
with Swift Mailer and while a plugin could easily be made and packaged 
internal or external of WordPress, I believe firmly that it should be 
packaged internally, because it would be used more if it was packaged 
internally than externally. It would also keep Plugin authors from 
having to package it themselves and with the edge case of having two or 
more plugins that have packaged Swift Mailer in the distance future.

Developers should look to the future for where development is heading. 
I'm not saying go crazy and change the project every time something new 
comes out. Hell, in a recent example, I didn't want to switch from 
Gallery 2 to Coppermine because of the support I would have to do for 
Coppermine, but it offered every feature. I just cried every time I had 
to look through the code. I mean that change, if it has clear advantages 
to the project would mean work that wouldn't have to be done.

WordPress could easily be refactored to use Swift Mailer, the only 
changes would be adding the library files and changing the default 
wp_mail code to use Swift Mailer instead.
> Like Windows or not, ... something other operating systems at the time
> did not do and it came back to haunt them.
>   
Backwards compatibility doesn't have to be sacrificed, except for some 
of the more advanced plugins which may use PHPMailer class instead of 
Swift Mailer. Research would have to be done for those people, but if 
you give enough time, they'll have a fix in a short period.

You do know that I'm crazy enough to have started exactly what you said 
WordPress shouldn't. However, I believe helping with WordPress would 
improve my skills with working with legacy code, which I lack.

The issue with Swift Mailer replacing PHPMailer has never been, "If we 
had built WordPress today..." I bought that up in a tangent to prove my 
point and even then my point was that code should be written so that 
future changes don't come back to bite you. Excusing one paradigm 
because it doesn't match yours is not reasonable development.
> Well, we're not and therefore there is no easy answer. Just like
> fishing, you have to reel it in some, and let it go loose some. You
> can't overhaul all the code just because something better is out
> there, nor can you ignore it all either.
>   
I'm not saying, overhaul all of the code. The change shouldn't be more 
than a few additional lines and perhaps an options table like some of 
the plugins offer for adding SMTP or sendmail configuration.

What Peter and Matt want to do is worse. Not only are they ignoring it, 
they want to rewrite what is already in Swift Mailer to their 
"standards". Okay, perhaps I'm being arrogant since Matt does have 
something I don't, a web application that a hell of a lot of people use 
(and love). That still doesn't mean that everything that they write is 
truth.

I'm saying that in my educated opinion, time shouldn't be wasted on 
adding SMTP support to PHPMailer, when it wouldn't (most likely) hurt 
anything to use Swift Mailer or include Swift Mailer plugin by default 
instead.
> With regards to PHPMailer, the real question we should be asking
> ourselves isn't if Switfy is a better library than it or if PHPMailer
> really is dead. Rather we should be asking what we would *LOSE*
> switching from PHPMailer and what we would *GAIN* and comparing the
> differences.
>   
Lose: Nothing to the core of WordPress. Only a few plugins, might be 
affected, but given enough time, like WordPress version 2.4 or 2.5, they 
would be able to change over.

Gain: SMTP support, Sendmail support, load balancing support for those 
dedicated servers that send a whole lot of mail.
> As it currently stands:
> The only thing actually *missing* from PM is SMTP support.
> We stand to break some stuff and open a whole can of worms by
> switching to Swifty.
>   
Not really, but I'll do a code audit, if only to prove that one of us is 
wrong, but I'm sure it won't be me. If I'm wrong, I'll be sure to let 
you know.
> Keeping in mind that anyone who wants Swifty can install a plugin (as
> I have done), _AND_ that it's almost trivial to add proper SMTP
> support to PHPMailer, I think the LOGICAL and UNBIASED answer is to
> just give adding SMTP support to PHPMailer a shot and see how it goes.
>   
Yeah. As someone who has written something that already existed in 
something else, you would merely be wasting your time, as I did. The 
only thing you would gain is some added insight, but if you already have 
that, then there is no point.
> I LOVE object-orientation. I use and recommend Swifty in my own (PHP)
> projects. But I love and understand WordPress too, and the right thing
> to do ATM, regardless of which library is "better" per-say, is to add
> SMTP to PHPMailer and go from there... IMHO of course.
>   
I *LOVE* OOP too. It is sweet! At the moment, yes, Swift Mailer 
shouldn't be added to WordPress 2.3. Swift Mailer is better, because 
there are facts to back it up. And the old argument comes up. No, it 
wouldn't be logical to add something to a library when another library 
already has that.

Would you rewrite Zend_Controller, because you didn't like how it did X? 
I would hope not. If you wish to extend the functionality, how would you 
continue doing that? Would you add more methods or would you create a 
new class with the functionality?
> If we open this particular can of worms, there is no end to it. Sure,
> switching to Swifty may not break that much code and may not pose a
> security vulnerability of any sorts (not that I know that for fact),
> but what comes next?
>   
Yes, add FUD to the discussion, that'll help your argument. I would say 
that if anything, the developer using the library should take the 
precautions, even if there weren't any.

Nothing. The problem that was in the original thread was if Swift Mailer 
could be used instead. If we stick to that, then I won't go any further. 
I care not at this moment whether HTML Purifier is used or Zend 
Framework is used, or if CakePHP was used. It matters not, my argument 
is merely the lack of giving something a second thought because you 
don't agree with its appearance and where its heading.

If this were a perfect world, all development would have already 
succeeded to PHP5, but I'm an optimist. This discussion next year, might 
be a little bit different. I believe that once developers have grasped 
the sexiness of PHP5's SPL, PDO, Filter, DateTime (this one is sweet!), 
etc that we'll all be on the same page.

I should have stated and I'm not sure if I did at one point, is that 
ultimately I have no say. I can only piss at the wind for all of the 
power I have in the direction of WordPress. However, I believe at least 
I can call out what I perceive to be inaccurate and add my reason to the 
discussion. The only opinion that matters is that of Matt, westi, Ryan, 
etc. I feel my job at the very least is to add truth, where I can to the 
argument and try to pull some developers to my side. Even then, the 
developer would have to see for themselves.
> WP has managed to strike a balance all these years, now is not the time to stop.
>   
What balance? WordPress is simple and gets the job done for the end 
user. Its plugin model gets the job done for the developer. In this 
discussion, it would be helping the core developers as they wouldn't 
have to maintain PHPMailer any longer. A win-win situation, but I can't 
figure out why this is a big issue.
> Just like switching to PDO is out of the question ATM simply because
> we don't NEED it for WP to survive and the current implementation is
> suiting us just fine (more or less, all things considered), I think
> it's the same for Swifty.
>   
I had a discussion of PDO with someone, I forget who. I realized 
something about application development is that you are right. WordPress 
doesn't need PDO to survive, since it duplicates, in part its 
functionality. It could make developers lives easier, but SQL would 
still have to be rewritten to match the change. The best way to handle 
that? I don't know.

Once WordPress reaches PHP 5 status, then $wpdb can use an instance of 
PDO. Until then, it does make sense to continue support of the DB Access 
object. However, this is only because PDO is PHP 5 only, Swift Mailer is 
PHP4 and PHP5, therefore the same argument doesn't hold true.
> Hell, if you *really* want, ship a Swifty plugin with WordPress and
> have it disabled by default. Let it overwrite the entire mailer class,
> and have fun.
I agree. The current Swift Mailer plugins should be looked at for 
inclusion of WordPress 2.4. The plugin that needs it can always 
automatically enable it, if they need to.


Peter Westwood wrote:
>
> On 15 Sep 2007, at 07:36, Jacob Santos wrote:
>
> WordPress is designed to be easy to use and install for end-users.  
> Therefore we have to support the installed base of software 
> (mysql/php/etc) not what the "manufacturers" of this software say we 
> should support - in $dayjob we have learnt this the hardway our 
> customers do not have the money to upgrade to the latest and greatest 
> version of windows - therefore even though Microsoft may not support a 
> Windows version we need to support our customers in the best possible 
> way.
>
> We should only ever deprecate  support for a version of PHP / mysql on 
> technical reasons - this is the kind of sound technical judgement we 
> as a development community have to take.
Yes, based on sound technical judgement. Not, completely ignoring change 
based on, "Well, it has too many files, and supports PHP5." Sound 
technical judgment would be to actually test the library, see what 
actually breaks, compared to what is broken in the current library. My 
argument is that the benefits of Swift Mailer was completely ignored 
based on these assumptions.

I suspect WordPress is going to support PHP4 far outside the day that 
PHP4 official support is dropped. Okay, I'm not a developer working on 
WordPress, so I don't care what the developers choose to support or not. 
What I do care about, is if I'm going to use it and have a problem, what 
will I have to do to fix it. Sure, I could easily write a plugin, and 
I've done that specifically for using HTML Purifier. No big deal, I just 
didn't use Kses, because it gave me problems.

Yes, perhaps you are correct that if the issue is an edge case, that 
developers shouldn't take the time to "fix" the issue. If this was a 
dropped case, then what argument would be given when PHP4 development 
support is dropped and how many hours were put into adding SMTP support 
to PHPMailer? I say, that since Swift Mailer offers everything we need, 
plus more, we only need to use what WordPress core needs and offer the 
rest to the developer.
>
>> While WordPress maintains its ease for end users, it is terrible, 
>> aside from the plugin implementation, for new developers. Tell me 
>> then, external libraries are for the purpose of applications to use 
>> and so that the application doesn't have to implement those features 
>> themselves. I would say that if you were to compare LOC of Swift 
>> Mailer and WordPress, WordPress would still be massive.
>>
>> What am I saying? WordPress goes so far as to create a PHP 4 
>> Exception class, that can't be handled as an exception. I suppose to 
>> get ready for a PHP 5 move in some distance future, but it is 
>> "interesting."
>>
>
> So all the OO Code written in C in the Linux Kernel is just wrong is 
> it.  Using the features of the language in a way which fits in with 
> your programming paradigm is sound development practise.

That isn't my argument. My point is recreation of the wheel. That and 
I've solved this particular problem with only passing an array, since if 
you are passing an error, except an error. The benefit of real 
Exceptions is that if they aren't caught, they are thrown and it would 
come up as an big error. A big error that would give you a clue as to 
what happened.

Error handling is error handling, if you pass the error by integer, 
string, array, or object, it is pretty much the same technique. Perhaps 
it is future proofing, as when PHP 5 support is added, the class can be 
thrown. If it extends Exception at that point, then it might make more 
sense, after I've thought about it. The shot I made was low and I take 
it back that line, but I maintain the previous paragraph.

>
> From what I know HTML Purifier and KSES don't do exactly the same 
> thing - the biggest problem in switching to HTML Purifier over KSES is 
> proving that it doesn't open any security holes.  You don't change 
> something that isn't broken.
How do you know that Kses doesn't have security holes? HTML Purifier 
runs tests against XSS attack cheat sheet and passes most of them. The 
added benefits of HTML Purifier, is that the WordPress balancing 
function would be obsolete and no longer would need to be maintain. I 
think that is a plus, unless the function is absolutely perfect. 
However, HTML Purifier is still being maintained and extended.

They do actually do the same thing, except HTML Purifier doesn't need an 
extra function to balance tags.

> As someone who spends all day at $dayjob writing highly OO code I 
> don't agree with you here - once of the biggest mistakes that nieve OO 
> developers make is to spilt there code into too many classes.
Yes, that would be accurate as it would break the "You're not going to 
need it" Extreme Programming paradigm. However, to be clear I was saying 
that once you reached the point that the methods you are adding, meet a 
different purpose than the class, it is time to make a new one. In my 
experience with writing highly OO code, I would say this is the best 
explanation or rule to live by.

Planning would create a better class structure and maybe the Swift 
Mailer did or did not do this originally. However, with v3, the 
developer had specific feedback on problems and solved those sets of 
problems. It does not mean WordPress is faced with similar problems, but 
third party developers that work with WordPress might benefit with the 
addition.
>
> Classes need a purpose - creating more just to split out the code does 
> not make good OO practise.
>
> If I was to sit down and design an OO model for sending email then I 
> can see myself coming up with the following class structure:
>
> A Class to represent an EMail - accessors and mutators for all the 
> common parts of an email - To/From/CC/Subject/Body/HTML 
> Body/Attachments etc.
> (If i wanted to process incoming emails then I would ensure this class 
> could successfully parse as well as generate emails)
Which Swift Mailer does and PHPMailer does not. I would have rather this 
be the case, but my argument is also that to do so for PHPMailer would 
be to take away what Swift Mailer already offers. If you compare what it 
would take to get PHPMailer to the standards of Swift Mailer, it would 
be insane.
>
> In my first design I suspect that sending and email would just be a 
> case of calling a method on the email itself.
>
> If I wanted to enhance this design and abstract away the sending then 
> I would probably create an interface for mail transfer and create 
> implementors of these for the transfer methods I had in mind - e.g. 
> mail(),smtp etc.
>
> This doesn't lead me to anywhere near the number of classes provided 
> by swiftmailer.
This is because Swift Mailer has been in development for longer than the 
week or two (well, it might take it shorter amount of time, but testing 
and debugging you know is included) it would take for you to complete 
your project. Swift Mailer has been in developer for at least/almost a 
year. Therefore it has had more feedback then your mental project and 
from that has improved on this base system.

The author of Swift Mailer is not some noob, and is more knowledgeable 
than me. If you put your knowledge of OOP above mine, then at least he 
is at your level. Why argue with me that his project doesn't meet your 
standards? Look at his first version and his current requirements and 
see for yourself if this project isn't something worthy of inclusion. 
Who knows, it might infect other OO developers to follow down a similar 
development style, once they get used to it of course.
>
>>>
>>>
>>> Peter Westwood wrote:
>>>> For me the things that turn me off SwiftMailer over PHPMailer are 
>>>> from a quick look at it:
>>>>
>>>>  1. The number of files - this increases the chance of someone 
>>>> breaking there install with a dodgy ftp transfer.
>>>>  2. PHP4 support is being dropped so by next year we will be in the 
>>>> same place we are now - supporting the mailer ourselves
>>>>  3. SwiftMailer is designed for mass-mailing PHP applications - not 
>>>> something required in the core
>>>>  4. SwiftMailer requires configuring with an smtp server - it 
>>>> doesn't use mail() which means we have to add more options to the 
>>>> admin which must be configured - giving a slower install (mail is 
>>>> sent during install)
>>>>
>>>> For me, especially once you take 2 into account, we are better 
>>>> staying with PHPMailer now - if we can make that work better than 
>>>> mail() for 90% of our users then we will be in the right place - 
>>>> the last 10% will realistically be difficult to support and better 
>>>> suited to a wp_mail replacement plugin.
>>>>
>>>> westi
>> Short Version:
>> 1. If this were true, then there would be issues with many more web 
>> applications.
>
> This is a problem - our user base have trouble uploading files - this 
> is something we have to be aware of and need to include in our 
> decision making process.

They must have a hell of a time uploading Gallery 2 and "real" web 
applications. I think it is a user error, than a FTP one. Perhaps PEAR 
would be a better deployment model? They would only need to do two or 
three lines on the command prompt, which you can give them. I'm not 
being sarcastic, seriously, PEAR as a deployment model would be 
wonderful, unless the user doesn't have access to shell and the host 
didn't enable PEAR.

I'm fortunate enough to not have this problem, because my host is 
awesome. However, like I said, the only time I've came across this is 
when the FTP server didn't support Passive mode. Perhaps, you should let 
them know and see if this is the problem.

>
>> 2. This is invalid because not only is PHP 4 support being dropped 
>> for Swift Mailer, but development of PHP4 is being dropped next year.
>
> As I said above - we want to support our user base - not alienate them 
> - when PHP5 is the standard for webhosts then we can consider dropping 
> PHP4 support - until then there is no technical reason to do so.
Name a web host that doesn't give you PHP 5. Even Godaddy hosting 
supports them, with an single .htaccess line. The problems are more with 
dedicated servers, and with those you have the choice between installing 
PHP5 and PHP4, so I would exclude dedicated servers from the list.

There is because they would still have the older versions, which would 
work on PHP4 on PHP5, but more importantly on PHP4. If they needs were 
met, then what is the issue? Security? I'm not saying don't maintain the 
older package, just don't dismiss something because its future paradigm 
is PHP5 and you don't like PHP5. Change is coming, whether developers 
want it to or not.
>
>> 3. Not really. It can be used to easily solve the mass mailing 
>> problem that plagues mail(), but it can easily be used for a 
>> replacement for PHPMailer and mail().
>> 4. Wrong. Swift Mailer includes three major mailing techniques: 
>> sendmail, SMTP, and mail(). Two others are for load balancing, try 
>> that with PHPMailer.
>>
>
> So it does - but why does the simple example code not use mail() then 
> - afterall that would be the simplest way of using SwiftMailer and 
> require the least code.
You ignored the part where I mentioned the problem that plagues mail(). 
With Swift Mailer, you can easily apply any one of the three in one line 
or you can test for SMTP settings, Sendmail settings, and fall back to 
mail with a simple if ... elseif ... else block.

I don't know what it would take to do the same for PHPMailer, but my 
continued argument is that it isn't worth wasting the time adding it to 
PHPMailer.
>
>> I would not take your number 2 argument into consideration and not 
>> because I prefer to develop using PHP 5, but because PHP 4 is dieing 
>> and will soon be dead. Therefore we should stick with a crappy mail() 
>> replacement until you decide that enough is enough and we should use 
>> a real class library that actually doesn't suck?
>
> If we were to start WordPress developement now then PHP5 would 
> probably be the way to go - we could write something much more OO 
> (like Habari maybe ;-))
I looked at the Habari code, good stuff, but from an developers 
perspective, probably wasn't worth it. To get where WordPress is now, it 
would require many man hours. Good luck to them, but while I like their 
paradigm, I'll be more willing to switch to S9Y and more my resources to 
helping that web app. I'm not yet at that point, but my employers have a 
lot of love for WordPress and it would require that I do a massive 
amount of mod_rewrite to keep my link structure. Not going to happen at 
this point.
>
> Our roots are PHP4, we have endusers running on PHP4 - we don't want 
> to alienate them or hold them to ransom.
>
> westi
No one is saying you have too. Swift Mailer at this point does in fact 
support PHP4, therefore there would be no issue.

Jacob Santos