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

SMTP server error occurs when 'Content-Type' header is not present #265

Closed
dmbonsall opened this issue May 21, 2022 · 4 comments
Closed
Labels
🪲 bug Something isn't working server Relates to the main binary (server or client)

Comments

@dmbonsall
Copy link

I encountered an error whenever an email is sent to NTFY without a 'Content-Type' header. This error was found in docker image v1.23.0. Error was discovered while setting up email notifications from a pfsense appliance --> ntfy.

I believe that I have found the root cause of the problem, and I have proposed a solution below. I wanted to start off submitting an issue since I don't know go, but if nobody has time to address it, I can see if I can get a dev env up and running to submit a PR with a fix. Just let me know :)

Steps to reproduce:

Create test email file without 'Content-Type' header:

EHLO other.example.com
MAIL FROM: user@other.example.com
RCPT TO: topic@ntfy.example.com
DATA
Subject: Test email

Test message for NTFY
.
QUIT

Send to server with NC:

nc ntfy.example.com 22 < temp.txt

Server response:

220 ntfy.example.com ESMTP Service Ready
250-Hello other.example.com
250-PIPELINING
250-8BITMIME
250-ENHANCEDSTATUSCODES
250-CHUNKING
250-AUTH PLAIN
250 SIZE 1048576
250 2.0.0 Roger, accepting mail from <user@other.example.com>
250 2.0.0 I'll make sure <topic@ntfy.example.com> gets this
354 2.0.0 Go ahead. End your data with <CR><LF>.<CR><LF>
554 5.0.0 Error: transaction failed, blame it on the weather: mime: no media type
221 2.0.0 Bye

Diagnosis

In ntfy/server/smtp_server.go::readMailBody, the problem is with the line:

contentType, params, err := mime.ParseMediaType(msg.Header.Get("Content-Type"))

If 'Content-Type' header isn't set on the message, then msg.Header.Get("Content-Type") with return "". According to the mime library source, the function ParseMediaType returns an error if mediatype argument is "". Approximate stack trace is something like:

src/mime/mediatype.go:143 in ParseMediaType: https://cs.opensource.google/go/go/+/refs/tags/go1.18.2:src/mime/mediatype.go;l=143

    err = checkMediaTypeDisposition(mediatype)
    if err != nil {
        return "", nil, err
    }


src/mime/mediatype.go:103 in checkMediaTypeDisposition: https://cs.opensource.google/go/go/+/refs/tags/go1.18.2:src/mime/mediatype.go;l=106
    if typ == "" {
        return errors.New("mime: no media type")
    }

The error there is consistent with the SMTP server log message and error code above.

Proposed solution

Update ntfy/server/smtp_server.go::readMailBody to check if 'Content-Type' is set, and if not, default it to "text/plain" or some other, sensible default. This will ensure that any notification systems who do not set "Content-Type" will still have their notifications accepted.

@binwiederhier
Copy link
Owner

That has to be the most detailed bug report I have ever received. Thank you!! Will have it fixed in no time.

@binwiederhier binwiederhier added 🪲 bug Something isn't working server Relates to the main binary (server or client) labels May 21, 2022
@binwiederhier
Copy link
Owner

binwiederhier commented May 22, 2022

Had to read the MIME RFC to see if emails without content type are allowed, and that looks to be the case:

From https://datatracker.ietf.org/doc/html/rfc1521#page-13:

Default [RFC 822](https://datatracker.ietf.org/doc/html/rfc822) messages are typed by this protocol as plain text in
the US-ASCII character set, which can be explicitly specified as
"Content-type: text/plain; charset=us-ascii".  If no Content-Type is
specified, this default is assumed.

I fixed the issue here just like you proposed (2abd6a5), and it'll be in the next release.

Thank you for reporting the bug!

@dmbonsall
Copy link
Author

Happy to help, thanks for such a quick turn-around on this! Good find on the RFC docs, it's good to know the fix complies with the spec.

@ColonelThirtyTwo
Copy link

The swaks testing tool sends such emails.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🪲 bug Something isn't working server Relates to the main binary (server or client)
Projects
None yet
Development

No branches or pull requests

3 participants