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

Notifications emails are not sent due to invalid From #3615

Closed
strk opened this issue Sep 7, 2016 · 7 comments
Closed

Notifications emails are not sent due to invalid From #3615

strk opened this issue Sep 7, 2016 · 7 comments

Comments

@strk
Copy link
Contributor

strk commented Sep 7, 2016

Noticing I was not receiving email notifications I checked the logs and found:

gomail: could not send email 1: gomail: invalid address "\"bjornharrtell\" <>": mail: invalid string

As the user bjornharrtell has a valid email registered (checked via web UI and on the database), I went looking at code and found this in modules/log/smtp.go:

    mailmsg := []byte("To: " + strings.Join(s.RecipientAddresses, ";") + "\r\nFrom: " + s.Username + "<" + s.Username +
        ">\r\nSubject: " + s.Subject + "\r\n" + content_type + "\r\n\r\n" + fmt.Sprintf(".%s", time.Now().Format("2006-01-02 15:04:05")) + msg)

    return smtp.SendMail(                                                       
        s.Host,                                                                 
        auth,                                                                   
        s.Username,                                                             
        s.RecipientAddresses,                                                   
        mailmsg,                                                                
    )             

Note how s.Username is used for the From field both in the mail header and in the smtp.SendMail 3rd parameter.

I suspect smtp.SendMail just refuses such From address (lacking an @ character) and fails to send anything.

I believe things did work as of 0.9.68, while are failing as of 0.9.99

@strk
Copy link
Contributor Author

strk commented Sep 7, 2016

I guess the From value should be set to the FROM parameter under mailer config section ?

@strk
Copy link
Contributor Author

strk commented Sep 7, 2016

Ok I changed analisys, now I think the problem is with composeIssueMessage in models/mail.go:

func composeIssueMessage(issue *Issue, doer *User, tplName base.TplName, tos []string, info string) *mailer.Message {
    subject := issue.MailSubject()                                              
    body := string(markdown.RenderSpecialLink([]byte(issue.Content), issue.Repo.HTMLURL(), issue.Repo.ComposeMetas()))
    data := composeTplData(subject, body, issue.HTMLURL())                      
    data["Doer"] = doer                                                         
    content, err := mailRender.HTMLString(string(tplName), data)                
    if err != nil {                                                             
        log.Error(3, "HTMLString (%s): %v", tplName, err)                       
    }                                                                           
    msg := mailer.NewMessageFrom(tos, fmt.Sprintf(`"%s" <%s>`, doer.DisplayName(), setting.MailService.User), subject, content)
    msg.Info = fmt.Sprintf("Subject: %s, %s", subject, info)                    
    return msg                                                                  
}       

The NewMessageFrom method second parameter sets the From line, and the code is using setting.MailService.User rather than setting.MailService.From. The former should be used for SMTP authentication, when needed, while the latter is the actual From. In my deploy there's no User as no authentication is needed.

@strk
Copy link
Contributor Author

strk commented Sep 7, 2016

Confirmed, setting a USER variable in the configuration is an effective workaround.
I will file a pull request with a fix to the bug.

strk added a commit to strk/gogs that referenced this issue Sep 7, 2016
It is the FROM field in mailer configuration that needs be used,
not the USER field, which is for authentication.

Closes gogs#3615
@DblK
Copy link
Contributor

DblK commented Sep 7, 2016

From my point of view, the SmtpWriter, need to have also the Username instead of using the field Username as UserEmail.

After adding this information, the from String needs to be changed according to the new field in the structure.

That way, according to the source, it will be From: Example <example@gmail.com> which is better than From: example@gmail.com <example@gmail.com>.

@strk
Copy link
Contributor Author

strk commented Sep 7, 2016

You meant need to have also the UserEmail I guess ? If I read the code correctly SmtpWriter has a From configured only once, so it should be settings.MailService.From, producing From: User Name <system@from.dns>. Or are you saing that the s.Username value is really an email ?

Anyway, this ticket was for issue notifications, which are fixed by PR #3616, if you can see a bug due to SmtpWriter please file a separate ticket, with instructions to reproduce. Thanks

@DblK
Copy link
Contributor

DblK commented Sep 7, 2016

According to the comments, Username is an UserEmail and if it's not the email could not be sent.
I will fill another issue form that specific.

strk added a commit to strk/gogs that referenced this issue Nov 2, 2016
It is the FROM field in mailer configuration that needs be used,
not the USER field, which is for authentication.

Closes gogs#3615
strk added a commit to strk/gogs that referenced this issue Nov 2, 2016
It is the FROM field in mailer configuration that needs be used,
not the USER field, which is for authentication.

Closes gogs#3615
strk added a commit to strk/gogs that referenced this issue Nov 2, 2016
It is the FROM field in mailer configuration that needs be used,
not the USER field, which is for authentication.

Closes gogs#3615
RichieB2B added a commit to RichieB2B/gogs that referenced this issue Dec 15, 2016
@unknwon
Copy link
Member

unknwon commented Jan 24, 2017

Merge this thread to #3856.

@unknwon unknwon closed this as completed Jan 24, 2017
Martchus pushed a commit to Martchus/gogs that referenced this issue Aug 27, 2018
Include both a log entry and the blocked mime type in the gitea log when
an attachment upload is blocked.

Chosen log level is info; this may need to be dialed down to trace.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants