Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
After a great deal of work with this module, I ended up doing several tuning:
- Fixed a FALSE negative in some greylist responses from SMTP servers.
- Improved overall SMTP communication to accept multi-line responses and read response code.
- Implemented socket timeout for unresponsive servers.
- Improved output messages for log that include complete communication with server and e-mail addresses.
- On high traffic sites, bots will use once and again the same e-mail address, you server's IP will eventually get blocked. Added a caching layer to mail verification.
Comments
Comment #1
david_garcia CreditAttribution: david_garcia commentedComment #2
oadaeh CreditAttribution: oadaeh commentedIt will be more likely to be incorporated, if you provide a patch.
With a patch, I can quickly scan the changes, apply the patch, test the functionality, and commit.
Without a patch, I also need to research your changes and where they are likely to go, manually apply them to the code base, see how they work and if they work, etc. It is more time and effort, and is something I am not likely to do, with my limited availability.
Comment #3
david_garcia CreditAttribution: david_garcia commentedI was posting just as reference, but it is good to see the mantainer is "awake". I will then provide a patch ASAP.
Greetings,
Comment #4
david_garcia CreditAttribution: david_garcia commentedNot a GIT patch, but hope it will help.
Just to note, to improve detection rate make sure that the IP used in your SMTP server has proper Reverse DNS setup, spf records and the like.
Comment #5
etron770 CreditAttribution: etron770 commentedsame here
Rejected email address: xxxx@yyyy.de. Reason: 450 4.2.0 : Recipient address rejected: Greylisted, see ..
Comment #6
david_garcia CreditAttribution: david_garcia commentedetron770, give a try to my patch, actually, you can completely replace the email_verify.inc with the one I uploaded.
Using Hidden Captcha + Honeypot + This module I am getting 0, litterally 0 bot in my websites, without annoying users with regular captcha.
I'm not sure if I made more modifications to the file since I first uploaded this, so I attach it again.
Greetings and provide some feedback so the maintainers can take care of this!
Comment #7
oadaeh CreditAttribution: oadaeh commentedYour most recent attachement is not a patch. See the Drupal documentation on patches.
Comment #8
david_garcia CreditAttribution: david_garcia commentedGlad to see someone alive with commit rights.
Those patches were the first thing I had ever worked on Drupal related, quite a while ago, so excuses for lack of formality.
Here goes a decently revised patch, there is an extra addition to the original changes, I will update the issue summary.
I know that better to have every small change in it's single issue, but these changes are all related and the module needed some looking into after years of abandonement.
Comment #9
david_garcia CreditAttribution: david_garcia commentedComment #10
etron770 CreditAttribution: etron770 commented#6
Hi David,
actually I have no information that a user was greylisted.
so I can not test your patch.
Comment #11
david_garcia CreditAttribution: david_garcia commentedBecause the current module only reads single-line output, it is very difficult to debug grayliting and other problems from current watchdog messages because full SMTP response is not logged.
This is the change responsible for the fix of false positives with graylisting:
The thing is that code 450 is used by mailservers to indicate graylisting (and other temporary issues), meaning that if we receive any 450 it should be treated like the e-mail cannot be verified, not that it is an invalid e-mail:
Comment #12
david_garcia CreditAttribution: david_garcia commentedUpdated comments to fix wrong asumption that 450 error code means invalid e-mail. Any 4xx code just means temporary unavailable, for example, when graylisting applied.
Comment #13
oadaeh CreditAttribution: oadaeh commentedI'm sorry to take so long to get back to you on this, however, I have a lot going on in my life. Your various proposed feature requests sound like excellent ideas, but since you've created a patch with a lot of different changes in it, it will take me a while to review it.
Also, it looks like you've included code changes from another patch (with no indication of that in this issue or in the other one), and so I'm going to have to spend time figuring out what code in the other patch is included in this one (so I can determine if any of the other issue is still valid or not).
Maybe you don't realize this, but I don't just blindly accept and include patches without some sort of code and functionality review and testing. So, when someone provides a patch that includes code changes for multiple features (and a little code clean up here and there), it takes time to determine which code changes are relevant to which proposed feature and which are not, before I can even start reviewing the feature.
It takes me (and anyone else) far less time to review smaller, discrete patches and changes, which means they are more likely to be included far sooner.
So, please create separate issues and patches for each feature and/or discrete set of changes you are proposing being made. You don't have to create a separate patch for each watchdog message wording change. You may include all of those that are unrelated to other proposed feature changes in one patch, but please be sparing about your "code cleanup" changes in one patch, as again, too many changes in one file makes reviewing them difficult and time consuming.
Also, when you create your patches, please follow the Drupal standards, security and best practices outlined in these pages: https://www.drupal.org/developing/best-practices
Thank you.
Comment #14
oadaeh CreditAttribution: oadaeh commentedMoving to the new current development branch.
Comment #15
killes@www.drop.org CreditAttribution: killes@www.drop.org commentedI've updated the patch.
I think that supporting greylisting is fairly important nowadays and wouldn't recommend using this module without this patch.
Comment #16
oadaeh CreditAttribution: oadaeh at Hook 42 commentedUnfortunately, the patch still fails in some or all of the points I mentioned last year.
Also unfortunately, the update came in after several other patches were already in the pipeline to be included, and they significantly modified the code this patch attempts to modify.
Fortunately, however, I have the time, the means, and the desire to update the patch and include it in the latest 2.x branch.
Also, having just been doing some significant changes in many parts of the module, it was not as time consuming to review and figure out as it would have been last year.
So, I've attached an updated patch that should apply to the latest code base.
I took the liberty of updating the comments in several places to be clearer and applying Drupal Code Standards to the code.
My basic testing was successful, but let me know if I affected any of the functionality, as I did not have any multi-line server responses.
My patch is very different from the previous ones, and so I did not bother to create an interdiff, as that would probably take as much time to review as the patch itself.
Comment #17
killes@www.drop.org CreditAttribution: killes@www.drop.org commentedThanks for updating the patch!
I've reviewed it and noticed that it removes "WATCHDOG_WARNING" in some instances. Is that intentional?
I believe it would be best if WATCHDOG_WARNING would remain for the cases where an email is actually rejected and WATCHDOG_INFO remains to be used for the cases where it is not.
Comment #18
oadaeh CreditAttribution: oadaeh at Hook 42 commentedHello Gerhard.
Sorry for not responding sooner. I had some other things I was working on that had a higher priority on my list.
That was part of the patch that both David and you submitted.
I'm not necessarily as concerned about the logging level, as I am that the messages get logged, so I left it as it was presented in the patch.
However, since you brought it up, here's a new patch with those logging levels restored. Anything else?
BTW, do you happen to know of any publicly available mail servers that return multi-line responses? I can configure mine to do it, but that won't really help other people when testing that function of the patch.
Thanks.
Comment #19
killes@www.drop.org CreditAttribution: killes@www.drop.org commented1st rule of Open Source development: Never apologize for delivering a patch. ;)
The patch looks good, but there's a problem: I can't register new users anymore, even if the email is ok.
This is due to the error check in function email_verify_verify_address($form, &$form_state) {
$error = email_verify_check($form_state['values'][$field]);
// Report the error and flag the form field.
if (!empty($error)) {
form_set_error($field, $error);
$error is now an array and will have something in it (for example debug info) at all times. So the check needs to be changed somehow.
Comment #20
killes@www.drop.org CreditAttribution: killes@www.drop.org commentedThe added patch ( to be applied after yours) does the job for me.
Comment #21
oadaeh CreditAttribution: oadaeh at Hook 42 commentedUgh! That was supposed to go in with the patch on this comment #2493515-8: Add debugging logging, but I must have missed it merging in 4 or 5 conflicting patches at one time.
I created a new issue to address it: #2704229: Fix tests of return values
Comment #23
oadaeh CreditAttribution: oadaeh at Hook 42 commentedThanks!