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

fix: reachable should no when SMTP host not exists #139

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

everyx
Copy link

@everyx everyx commented Nov 29, 2024

This PR sets ret.Reachable to reachableNo and returns ErrNoSuchHost when CheckMX returns the error ...no such host...

@emilianocalzada
Copy link

emilianocalzada commented Dec 4, 2024

Hey @everyx! Thanks for the PR! 😊
You helped me to notice something interesting in my logs - I was getting quite a few errors during SMTP checks like this:

"Mail server does not exist : lookup X on 127.0.0.11:53: no such host"

I implemented your suggested fix specifically for the CheckSMTP method with a small modification:

smtpResult, err := verifier.CheckSMTP(ret.Syntax.Domain, ret.Syntax.Username)
<-GlobalSemaphore

// Handle non-existent SMTP host
if err != nil {
    errStr := err.Error()
    if insContains(errStr, "no such host") {
        ret.Reachable = reachableNo
        err = nil  // <- This is the modification I made
    }
}

When CheckSMTP encounters a "no such host" error, it actually tells us something specific: the SMTP server for this domain doesn't exist, it means it is an reachableNo address. This is different from other SMTP errors like authentication issues or connection problems.

So this fix will also need to apply to CheckSMTP and not only to Verify methods

@everyx
Copy link
Author

everyx commented Dec 4, 2024

@emilianocalzada I'm not quite clear on what you mean, CheckSMTP is called after CheckMX, why continue to judge in CheckSMTP, CheckSMTP is only used to return the smtp field, and the no such host error has already been handled in parseBasicErr

@emilianocalzada
Copy link

Just to clarify, when we call the Verify() method with SMTP checks enabled, it will also call the CheckSMTP function, which can encounter the same "no such host" error since it is making the same MX servers check. Since CheckSMTP can be used independently, I think we should apply the same fix there as well

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

Successfully merging this pull request may close these issues.

2 participants