Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Plans for PHPMailer #88

Closed
Synchro opened this issue Jul 30, 2013 · 57 comments
Closed

Plans for PHPMailer #88

Synchro opened this issue Jul 30, 2013 · 57 comments

Comments

@Synchro
Copy link
Member

Synchro commented Jul 30, 2013

This ticket is the roadmap for PHPMailer's future development.

Now that even PHP 5.3 is now EOL, it's about time to make that the minimum supported version rather than 5.0. This necessarily implies BC breaks, so I propose to name it PHPMailer 6.0.0, following standard semantic versioning policy.

This means we will be able to use namespaces, switch to a PSR-0 compatible file layout, and provide better integration with composer and other frameworks that use PHPMailer.

There are quite a few architectural problems in PHPMailer. A good example is issue #36. This is a really simple problem, but it's made difficult by PHPMailer's messy and inconsistent mechanisms for handling headers - sometimes as arrays, sometimes as strings. This leads to knock-on effects like it being difficult to determine what order headers appear in - e.g. the abiity to add extra 'Received' headers first.

Another area of concern is the increasing spaghetti code involved in the handling of the various preset MIME structures. I propose to break out MIME classes that allow the flexible creation of arbitrary MIME structures, then use them to build presets for the same ease of use we have now. To their credit, this is what some other PHP mail classes (SwiftMailer and Zend Framework) already do.

There is way too much fixed dependency on the SMTP class. This should be dealt with using dependency injection and interfaces for independent transport classes.

All of these changes could be wrapped with a class that provides some degree of backward compatibility to minimise upgrade pain.

Please add any other ideas to this ticket.

@ooxi
Copy link

ooxi commented Jul 31, 2013

I totally agree, but wouldn't that mean to just rebuild SwiftMailer?

@Synchro
Copy link
Member Author

Synchro commented Jul 31, 2013

True, it would save a lot of effort!

@vicary
Copy link

vicary commented Jul 31, 2013

In my functions that handles both scalar and array inputs, I used to normalise it to always be arrays. You may take a look at isAssoc() and wrapAssoc() in my utility class here.

Hope that helps. :)

@ooxi
Copy link

ooxi commented Aug 6, 2013

@vicary, what compose function do you use? It is obvious what it does it it's quite useful, but i don't know any default implementation.

@vicary
Copy link

vicary commented Aug 8, 2013

@ooxi A bit off topic, but you can see my "functional" functions here. I used to compose() when I do my usual array_map() and array_filter(). The compose() itself is not often used, instead the derived propIn() and friends is quite neat to me. ;)

@Synchro
Copy link
Member Author

Synchro commented Aug 8, 2013

I don't see what any of this has to do with this ticket. The internal storage problems are a high level architectural issue that affects large amounts of the code, not really anything to do with low-level mapping functions.

@vicary
Copy link

vicary commented Aug 8, 2013

Don't meant for going off topic.

Regarding this

it's made difficult by PHPMailer's messy and inconsistent mechanisms for handling headers - sometimes as arrays, sometimes as strings

I was suggesting my way to easily have function parameters supports both scalar type and array type in PHP, people usually do that in Javascript and I kinda like that approach. Again, hope that helps.

@Synchro
Copy link
Member Author

Synchro commented Aug 8, 2013

It's not just array <-> string conversions, and mixed params are not the problem. There are a load of complicated formatting and encoding steps (which may be difficult to reverse) applied and used inconsistently. Things happen to end up in the right place, but it's a rats-nest in there.

@ooxi
Copy link

ooxi commented Aug 11, 2013

I'm currently following the SwiftMailer project for a bit and it has quite an active usage community but hardly any issues are answered on. Also pull requests are seldom accepted.

Therefore an alternative, active community project like PHPMailer will have it's right to exist if it has a clear niche, for example

  • Not being part or requiring a big framework
  • Compatible with older PHP versions which will stuck with us for a while (like PHP 5.2, i.e. not using namespaces)
  • Try using as view classes / files as possibly, for example by compiling several files to one prior to deployment (easy copy / paste installation)

What do you think about this approach?

@darlanalves
Copy link

I recently used PHPMailer class to a simple mailing on a server task, and I found it really messy and with a strong need of a complete review. The method casing and property acess are not intuitive to devs that are used to standards. I agree that there's a lot to consider when a BC is mostly desired, but I think that a total review with a new and future-prone architecture in mind would be great. Also, I'd like to contribute in a new version if needed.

Cheers.

@ooxi
Copy link

ooxi commented Oct 29, 2013

@darlanalves have you used the newest version from GitHub? There has happened a lot since the original developers from SourceForge

@Synchro
Copy link
Member Author

Synchro commented Oct 29, 2013

It's all about the BC really, and (not surprisingly) most of that was inherited. I've made method names use PSR-2 standard casing where I can (since PHP function names are not case sensitive), but I can't do the same for properties (which are case-sensitive), nor can I strip leading underscores without breaking stuff. I think the API is somewhat verbose, but it's not really that different to other projects that do the same thing. If I were to change something within PHPMailer, it would be the way that headers and MIME structures are handled internally. There's a lot of hard-coding that makes things very messy to work with and is the underlying root cause behind several open issues. The best MIME structure handling I've seen is in what was ezComponents (way better than Swift or Zend), but that seems to be fading away.

@darlanalves
Copy link

@Synchro What if we do two different branches, one for BC with the current API (upper cased properties, etc) and a new branch for a future release? Technically, a new version is supposed to break API and change the way things work. The current implementation could be maintained with the current stable code and small fixes. This new major version with a complete review could lead the project to a huge improvement.

I have some ideas to make it easier to use, even for newcomers that don't know PSR standards at all.

@Synchro
Copy link
Member Author

Synchro commented Oct 29, 2013

Well possibly, but as @ooxi said, wouldn't you just better off using Swift/Zend Mailer? There may be mileage in reimplementing some of PHPMailer to bring its more unusual features (like DKIM signing and S/MIME) to a more modern library, but I suspect that would represent more work than implementing those things in a mailer that has that structure already in place. While a cleaner library is desirable, it's not really that bad currently and rebuilding it represents a lot of effort to effectively reinvent the wheel. The majority of PHPMailer's popularity comes from its legacy support, not from its new bits.

@darlanalves
Copy link

Yes, I agree, it could be a reinvention of the wheel. But: Swift is a tool capable of sending emails from a NASA satellite with lasers, I don't think that beginners would get it at first. Zend is also a great tool, but using their mail implies some knowledge of Zend itself. I think of PHPMailer as a tool to easy write and send emails with PHP, with no fancyness.

I think that for most of the newbie/junior developers on PHP (or any person that don't use or don't know WTF is a framework) sending mails could be as easier as write something like this:

<?php
$email = new Email();
$email->to('john@doe.com');
$email->from('me@meenie.com');
$email->message('LLAP!');

$mailer = new PHPMailer();
$mailer->connectTo('mail.foo.com:587');
$mailer->login('mailing@mailer.com', 'lolz123');
$mailer->send($email);
// ... and it's done!

Feel free to express any ideas or tell me I'm crazy, but a cleaner and simpler syntax will not scary new PHP devs and will not make more experienced devs to roll their eyes.

P.S.: I think the DKIM stuff, or abstracting away most of the possible corner cases for mail transport is a bit too much to PHPMailer. It could be split into separated spin-offs (like plugins) to resolve these issues. The focus of PHPMailer should be only the mailing itself, to encode the message and prepare it for a MTA delivery, not the implementations of MTA drivers or abstraction of server differences (following the KISS principle). But again, just my humble opinion.

@andig
Copy link

andig commented Sep 4, 2014

I'd suggest adding Thunderbird-like auto configuration of email server settings.

@Synchro
Copy link
Member Author

Synchro commented Mar 21, 2015

I've been having more of a think about this. I think the number one feature should be arbitrary MIME structures, along with sane header/body classes and obvious boilerplate like a PHP 5.4 minimum namespaces, PSR-7 etc. That buys us a lot, solving all the structural problems we have relating to iCal, DKIM, encryption etc. I don't see that this can be accomplished within the existing code so it would involve a substantial rewrite, but I think it could be done while avoiding SwiftMailer-like overcomplication. Whatever BC breaks are done, we should also provide a BC wrapper to emulate old behaviour, a bit like Smarty did with its 2->3 transition.

I'm also very happy to see the appearance of RFC6857 which finally provides a usable path to using SMTPUTF8, which is not currently suported by any PHP library I know of, and should be a top priority.

@avindra
Copy link

avindra commented Jun 11, 2015

Currently using PHPMailer 5.2.x. Love this library. I would like to pitch in (increasing code coverage and documentation) when some design decisions are made.

@ghost
Copy link

ghost commented Jun 16, 2015

So far so good. Current version I am using works spot on and didn't even fail once. Apart I have to mod it like explained in #439 (I like to use libraries unmodified)

I might be nitpicking at this point but, to be honest, the library has lots of code that is totally unnecessary to 99% of the users. It is great that is compatibility to older versions but if you are using new version (even my is 2 year old version), I do not need compatibility checks to ancient versions. What I suggest is that you would separate compatibility files, that is, develop on the latest version you can and add compatibility add-ons for earlier versions if someone needs to. That would reduce the size of the file significantly, makes it lightweight and clean. Same applies #439 . the code I commented out does nothing for my version. :) Thoughts?

@Synchro
Copy link
Member Author

Synchro commented Jun 16, 2015

Well, to be fair, you do need compatibility checks like the others that are there because you are running a version that has a problem that will be fixed in exactly the same way, and one day that will be considered old too. You commented out large chunks of useful code (validateAddress can be used externally, not just internally), when you could have simply changed the default value of $patternselect to pcre, which is still a better validator than the PHP built-in one. There are not that many version checks (4 in total) and they really add negligible overhead.

There is a lot of spaghetti in PHPMailer, but BC is a very major thing - there are possibly hundreds of millions of sites running it (it's been in Wordpress since the start), many on very old deployments.
That said, take a look in the xoauth branch - the code for google's new auth system requires PHP 5.4, which necessarily implies a compatibilty change, however it's not a BC break because it's also a new feature and has been implemented as you suggest, using a subclass.

I don't think that's a sane model for broader development - previous comments in here already outline how I'd like it to change, but it's still a matter of whether it's worth the effort - Swift and Zend_Mail are are already perfectly adequate 'modern' takes on the same thing, though I think there is still space for a simpler, stricter, more lightweight solution. IME, the biggest bottleneck is usually SMTP - it's tempting to write something in Zephir for that!

@michield
Copy link
Contributor

Synchro, can you explain why BC is so important? I don't think those hundreds of millions of sites are going to keep track of new phpMailer versions and upgrade their systems to have the latest phpMailer. They can just stay with the version they have (in fact, they probably will). It's not that you want to continue to run on PHP3 either. Eventually it's the job of the project integrators to ensure the compatibility. You can just outline the PHP version requirements for new versions (of phpMailer). It may be good to force people to upgrade their PHP versions.

@Synchro
Copy link
Member Author

Synchro commented Jun 17, 2015

I have no problem with BC breaks in major new releases (what this ticket is all about), but I'd classify nearly everything from the last few years as maintenance, and BC breaks for no good reason are not helpful. For example, @martybalandis' problem could be fixed obtusely by refusing to run on less than PHP 5.6.10. Coming from the other direction, if something can be fixed sensibly without breaking BC, then there's no real reason not to go that way. It's not the job of a minor library to dictate major decisions like PHP version - whole frameworks maybe, but not individual components.

@chriscct7
Copy link
Contributor

Synchro, can you explain why BC is so important? I don't think those hundreds of millions of sites are going to keep track of new phpMailer versions and upgrade their systems to have the latest phpMailer.

~24% of the websites in the internet use WordPress. WordPress bundles PHPMailer. If BC was broken, there would be serious problems. It's very easy to go out and say, "hey let's require PHP 5.5+". WordPress often gets people complaining alot about why we don't upgrade our minimum version to 5.5+. Because we know, that of all of the people who are using WordPress right now only 12.5% of them are on PHP 5.5 or higher.

If you assume, and I would since hosting providers are pretty uniform, that you can take WordPress's 24% of the internet statistics for PHP usage as a representative percentage of the overall internet, then just under half of the internet is running either PHP 5.2 or PHP 5.3 (almost evenly divided at just over a third each).

This is why BC breaks should be avoided unless they are actually necessary. I agree completely with @Synchro on the matter. Hopefully, hosts will start upgrading people's PHP versions so that we can all move up.

Ref: https://wordpress.org/about/stats/. Snapshot as of this morning.

@michield
Copy link
Contributor

Sure, I completely understand that argument. However, that decision is for WordPress, not PHPMailer. Wordpress can always decide to stay with an older version, if they don't like the new dependencies, that's up to them. It's not good if downstream hampers and blocks upstream in new developments.

@Synchro
Copy link
Member Author

Synchro commented Jun 29, 2015

That's not really fair - the vast majority of PHPMailer users suffer the same problem, and nearly all PHPMailer updates are for bugfix or security reasons, exactly the reasons that you want to use a library in the first place. While I'll agree that we can probably do without retaining 5.0 compatibilty any more, dropping 5.2 would be a big deal. PHPMailer isn't about being bleeding edge - if you want that, there are other solutions. Reliability, safety and compatibility beats new and shiny.

@ivantcholakov
Copy link
Contributor

Just a humble opinion: PHPMailer's code, as it is now, may wait for the first stable PHP7 release at least and to ensure compatibility with it too. After that, next major release with refactoring and possible PHP version bump could be planned.

@michield
Copy link
Contributor

ok @Synchro that's a great explanation. So, we'd keep an eye on http://w3techs.com/technologies/details/pl-php/5/all to determine what versions should be supported. 5.2 still 16% is quite significant.

@Synchro
Copy link
Member Author

Synchro commented Aug 25, 2015

5.2 should already be fine on 7.0 - it's been in the Travis-CI config for several months, though I've not run a source analysis on it from that point of view.

@Synchro
Copy link
Member Author

Synchro commented Aug 25, 2015

I just ran php7cc on it and the only things that are flagged are the calls to set_magic_quotes_runtime, which are already inside a version check that only uses them on old versions, so we're all set for 7.0 already.

@ravisorg
Copy link

I just committed a subclass that does PGP signing / encryption and am looking for a few people to test / break it. If anyone is interested, issue #505 might be for you.

@ooxi
Copy link

ooxi commented Sep 29, 2015

@ravisorg Please don't rape other issues

@ravisorg
Copy link

@ooxi only posted it here because encryption was one of the features mentioned on the "plans for phpmailer" above. Seemed relevant.

@Synchro
Copy link
Member Author

Synchro commented Apr 19, 2016

There is a new PHPMailer 5.5 branch nearly ready to roll. It does have some BC breaks, and it bumps the version requirement to PHP 5.5 (hence the name). It's not a complete rebuild/redesign like this ticket talks about, but it does at least bring sensible file naming, namespaces, a much better XOAUTH2 implementation, and quite a lot of general cleanup. The 5.5 requirement is really down to dependencies that need it - it will probably still run ok on 5.4 if you're not using those features. Please give it a spin and open tickets for any issues you find.

@Synchro
Copy link
Member Author

Synchro commented Apr 20, 2016

I've had a thought - since there are some BC breaks, particularly for anyone not using composer, the next release will need to be called 6.0.

We've been recommending a composer.json version string of ~5.2, which will match 5.5, and result in automatic upgrades that are likely not compatible. The only way to avoid this is to follow the semver rules and bump the version to 6.0.

@michield
Copy link
Contributor

For reference, the branch is now https://github.com/PHPMailer/PHPMailer/tree/6.0

@ivantcholakov
Copy link
Contributor

Will be 5.2.15 released anyway?

@Synchro
Copy link
Member Author

Synchro commented Apr 22, 2016

Yes - there are quite a few things lurking in master that will be released as 5.2.15, and 5.2.x maintenance will be moved to the 5.2-stable branch. 6.0 will then become master.

Since we are now committed to making some BC breaks, does anyone have anything particularly pressing (but minor!) they would like changing?

@ivantcholakov
Copy link
Contributor

Not from me.

@ooxi
Copy link

ooxi commented Apr 22, 2016

I do not but I would suggest creating a new issue when asking for comments about BC breaks. When watching a project I mostly quiet issues after determining whether they are of interest to me or not.

Therefore I wouldn't normally have noticed that question.

@andig
Copy link

andig commented Apr 22, 2016

I would love to see smtp/pop/imap autoconfiguration using mozillas database. I do have sample code as demonstration available. Its not even a bc break, more an addon that would be helpful for many users imho.

Am 22.04.2016 um 15:51 schrieb ooxi notifications@github.com:

I do not but I would suggest creating a new issue when asking for comments about BC breaks. When watching a project I mostly quiet issues after determining whether they are of interest to me or not.


You are receiving this because you commented.
Reply to this email directly or view it on GitHub

@Synchro
Copy link
Member Author

Synchro commented Apr 25, 2016

@andig that would be a great addition for the examples folder, and as you say would be best done as an add-on rather than an internal change. Please make a PR!

@andig
Copy link

andig commented Apr 25, 2016

Please make a PR!

I have honestly virtually no time for this right now but feel free to take a peek at https://github.com/andig/mozilla-isp. It's hacked together for my purposes and I'm open to suggestions.

Would a simple phpmailer example plus composer.json to pull in that package suit your needs? Do you want to clone or take over the repo?

@SDKiller
Copy link
Contributor

@andig

Could you add a README just to know what is this addon intended for?

IMO, email providers like yandex and gmail give you enough info to configure mailer when you register account.
Can mozzilla database be aware of something than provider itself isn't?

@andig
Copy link

andig commented Apr 25, 2016

Can mozzilla database be aware of something than provider itself isn't?

No, it cannot. What it can do however is giving a noob user the ability to e.g. send email using his own account without caring about server configuration details. Think about IoT device applications, maybe bundled in form of a raspberry pi?

@Synchro
Copy link
Member Author

Synchro commented Apr 25, 2016

Might be nice to combine it with the DNS-based approach that things like Outlook uses. These autoconfig things are mainly for client config, but there's no good reason not to use them for server-side outbound too.

@ivantcholakov
Copy link
Contributor

@Synchro

What would you say about dual licensing MIT & LGPL?

Here is the occasion for this question:
codeigniter4/CodeIgniter4#276 (comment)

@Synchro
Copy link
Member Author

Synchro commented May 2, 2017

@ivantcholakov Is there really an incompatibility? There's nothing preventing you from redistributing LGPL libs with an MIT project - it doesn't affect the host project at all. It only matters if you make changes to the libs themselves - which you either shouldn't need to do, or should do using a subclass that can itself be MIT-licensed. Changing licenses is very hard because you have to get written consent from everyone that has ever contributed code - and that's a lot of people in a project that's been going for 16 years!

@chris001
Copy link

chris001 commented May 2, 2017

Since we are now committed to making some BC breaks, does anyone have anything particularly pressing (but minor!) they would like changing?

Security.

  1. Check and respect CRL for SSL certs. Fail on revoked certs.
  2. Check and respect DNSSEC flags. Fail DNS lookup for spoofed entries.
  3. Check and respect TLSA records for all IP socket communication with servers. Fail on spoofed entries.
  4. If using IP56, check IPsec available, if yes, enable it.

@Synchro
Copy link
Member Author

Synchro commented May 2, 2017

@chris001 PHPMailer itself is not involved in cert checking at all; All of those things are in the realm of php.ini settings and can be set (if such flags are available) via the SMTPOptions property that gets passed to the underlying PHP function.

@ooxi
Copy link

ooxi commented May 4, 2017

Moreover I would argue that the default behavior of PHPMailer should be works everywhere not as secure as possible. I'm not arguing for shipping an insecure default configuration, but a hardened configuration is not what I think the average user expects from this library.

@chris001
Copy link

chris001 commented May 4, 2017

  1. The internet traffic communications performed by PHPmailer should be encrypted by default with current widely accepted strong protocols and should fail by default if it cannot be encrypted, instead of falling back to cleartext to weak encryption like RC4/MD5/EXPORT etc.
  2. PHPmailer should use secure DNSSEC by default so as to not be tricked into sharing mail contents with a spoofed remote SMTP server.
  3. The email message should also probably be encrypted by default, where possible (depends on having keys installed at both sender and receiver), this is not always possible, so it can be best effort.
  4. PHPmailer should not contribute to the spam problem - it should help reduce spam by dropping emails which a user is trying to send as a spoofed domain. This is very easy to do. I'm not talking about emails from the famous example.com for testing the software. I'm talking about the very famous spoofed emails from a fake sender pretending to be paypal.com and swissbank.ch where the sender is trying to phish the account login details out of the victim and depends on phpmailer to forward the email into the internet in the hopes that 1/1000 people who read it will fall into the trap.

@Synchro
Copy link
Member Author

Synchro commented May 4, 2017

@chris001

  1. It is - PHPMailer uses encryption by default if the server supports it, even if you don't ask it to. The availability of recent protocols and strong ciphers in email servers is very poor, but this is an external issue handled by php.ini settings and the SMTPOptions property I mentioned before.
  2. No. That's an app-level concern that should be resolved before an app asks PHPMailer to do anything. The typical use case (and highest performance route) for PHPMailer is not affected by DNSSEC anyway - nearly every implementation will send through a local relay or nearby smarthost, so interrogation of DNS for final destinations happens there, not in PHPMailer.
  3. Definitely no. There is no easy, reliable way to auto-discover S/MIME or PGP keys, and it's extremely rare to use them at all. PHPMailer supports S/MIME and DKIM signing for integrity and authenticity, and there are some public forks that add encryption using PGP and S/MIME. There's no way you can encrypt by default - there is no such thing as universal PKI.
  4. Nope. App-level issue, not a library's job. In my own app I trap RFC2606 names and make them silently discard messages, very useful for testing, but a mechanism unique to my app. Most phishing emails do not send from real domains like paypal.com anyway, because that would cause them to fail SPF checks and never be delivered, so it's a pointless exercise.

@Synchro
Copy link
Member Author

Synchro commented Aug 28, 2017

PHPMailer 6.0 has been released, containing many of the things discussed in here. Time to start a new milestone for future changes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests