Properly handle different attachment use cases

docs/publish.md

@@ -666,26 +666,21 @@ Here's an example that will open Reddit when the notification is clicked:
 - Preview without attachment 
 
 
-# Send attachment
+# Upload and send attachment
 curl -T image.jpg ntfy.sh/howdy
 
-# Send attachment with custom message and filename
+# Upload and send attachment with custom message and filename
 curl \
     -T flower.jpg \
     -H "Message: Here's a flower for you" \
     -H "Filename: flower.jpg" \
     ntfy.sh/howdy
 
-# Send attachment from another URL, with custom preview and message 
+# Send external attachment from other URL, with custom message 
 curl \
     -H "Attachment: https://example.com/files.zip" \
-    -H "Preview: https://example.com/filespreview.jpg" \
     "ntfy.sh/howdy?m=Important+documents+attached"
 
-# Send normal message with external image
-curl \    
-    -H "Image: https://example.com/someimage.jpg" \
-    "ntfy.sh/howdy?m=Important+documents+attached"
 ```
 
 ## E-mail notifications

scripts/postinst.sh

@@ -4,8 +4,6 @@ set -e
 # Restart systemd service if it was already running. Note that "deb-systemd-invoke try-restart" will
 # only act if the service is already running. If it's not running, it's a no-op.
 #
-# TODO: This is only tested on Debian.
-#
 if [ "$1" = "configure" ] || [ "$1" -ge 1 ]; then
   if [ -d /run/systemd/system ]; then
     # Create ntfy user/group

server/server.go

@@ -102,6 +102,7 @@ var (
 	docsRegex        = regexp.MustCompile(`^/docs(|/.*)$`)
 	fileRegex        = regexp.MustCompile(`^/file/([-_A-Za-z0-9]{1,64})(?:\.[A-Za-z0-9]{1,16})?$`)
 	disallowedTopics = []string{"docs", "static", "file"}
+	attachURLRegex   = regexp.MustCompile(`^https?://`)
 
 	templateFnMap = template.FuncMap{
 		"durationToHuman": util.DurationToHuman,
@@ -137,8 +138,13 @@ var (
 	errHTTPBadRequestSinceInvalid                    = &errHTTP{40008, http.StatusBadRequest, "invalid since parameter", "https://ntfy.sh/docs/subscribe/api/#fetch-cached-messages"}
 	errHTTPBadRequestTopicInvalid                    = &errHTTP{40009, http.StatusBadRequest, "invalid topic: path invalid", ""}
 	errHTTPBadRequestTopicDisallowed                 = &errHTTP{40010, http.StatusBadRequest, "invalid topic: topic name is disallowed", ""}
-	errHTTPBadRequestInvalidMessage          = &errHTTP{40011, http.StatusBadRequest, "invalid message: invalid encoding or too large, and attachments are not allowed", ""}
+	errHTTPBadRequestMessageNotUTF8                  = &errHTTP{40011, http.StatusBadRequest, "invalid message: message must be UTF-8 encoded", ""}
 	errHTTPBadRequestMessageTooLarge                 = &errHTTP{40012, http.StatusBadRequest, "invalid message: too large", ""}
+	errHTTPBadRequestAttachmentURLInvalid            = &errHTTP{40013, http.StatusBadRequest, "invalid request: attachment URL is invalid", ""}
+	errHTTPBadRequestAttachmentURLPeakGeneral        = &errHTTP{40014, http.StatusBadRequest, "invalid request: attachment URL peak failed", ""}
+	errHTTPBadRequestAttachmentURLPeakNon2xx         = &errHTTP{40015, http.StatusBadRequest, "invalid request: attachment URL peak failed with non-2xx status code", ""}
+	errHTTPBadRequestAttachmentsDisallowed           = &errHTTP{40016, http.StatusBadRequest, "invalid request: attachments not allowed", ""}
+	errHTTPBadRequestAttachmentsExpiryBeforeDelivery = &errHTTP{40017, http.StatusBadRequest, "invalid request: attachment expiry before delayed delivery date", ""}
 	errHTTPInternalError                             = &errHTTP{50001, http.StatusInternalServerError, "internal server error", ""}
 	errHTTPInternalErrorInvalidFilePath              = &errHTTP{50002, http.StatusInternalServerError, "internal server error: invalid file path", ""}
 )
@@ -444,27 +450,15 @@ func (s *Server) handlePublish(w http.ResponseWriter, r *http.Request, v *visito
 		return err
 	}
 	m := newDefaultMessage(t.ID, "")
-	filename := readParam(r, "x-filename", "filename", "file", "f")
+	cache, firebase, email, err := s.parsePublishParams(r, v, m)
-	if filename == "" && !body.LimitReached && utf8.Valid(body.PeakedBytes) {
-		m.Message = strings.TrimSpace(string(body.PeakedBytes))
-	} else if s.fileCache != nil {
-		if err := s.writeAttachment(r, v, m, body); err != nil {
-			return err
-		}
-	} else {
-		return errHTTPBadRequestInvalidMessage
-	}
-	cache, firebase, email, err := s.parsePublishParams(r, m)
 	if err != nil {
 		return err
 	}
-	if email != "" {
+	if err := maybePeakAttachmentURL(m); err != nil {
-		if err := v.EmailAllowed(); err != nil {
+		return err
-			return errHTTPTooManyRequestsLimitEmails
 	}
-	}
+	if err := s.handlePublishBody(v, m, body); err != nil {
-	if s.mailer == nil && email != "" {
+		return err
-		return errHTTPBadRequestEmailDisabled
 	}
 	if m.Message == "" {
 		m.Message = emptyMessageBody
@@ -503,12 +497,34 @@ func (s *Server) handlePublish(w http.ResponseWriter, r *http.Request, v *visito
 	return nil
 }
 
-func (s *Server) parsePublishParams(r *http.Request, m *message) (cache bool, firebase bool, email string, err error) {
+func (s *Server) parsePublishParams(r *http.Request, v *visitor, m *message) (cache bool, firebase bool, email string, err error) {
 	cache = readParam(r, "x-cache", "cache") != "no"
 	firebase = readParam(r, "x-firebase", "firebase") != "no"
-	email = readParam(r, "x-email", "x-e-mail", "email", "e-mail", "mail", "e")
 	m.Title = readParam(r, "x-title", "title", "t")
 	m.Click = readParam(r, "x-click", "click")
+	attach := readParam(r, "x-attachment", "attachment", "attach", "a")
+	filename := readParam(r, "x-filename", "filename", "file", "f")
+	if attach != "" || filename != "" {
+		m.Attachment = &attachment{}
+	}
+	if attach != "" {
+		if !attachURLRegex.MatchString(attach) {
+			return false, false, "", errHTTPBadRequestAttachmentURLInvalid
+		}
+		m.Attachment.URL = attach
+	}
+	if filename != "" {
+		m.Attachment.Name = filename
+	}
+	email = readParam(r, "x-email", "x-e-mail", "email", "e-mail", "mail", "e")
+	if email != "" {
+		if err := v.EmailAllowed(); err != nil {
+			return false, false, "", errHTTPTooManyRequestsLimitEmails
+		}
+	}
+	if s.mailer == nil && email != "" {
+		return false, false, "", errHTTPBadRequestEmailDisabled
+	}
 	messageStr := readParam(r, "x-message", "message", "m")
 	if messageStr != "" {
 		m.Message = messageStr
@@ -565,13 +581,57 @@ func readParam(r *http.Request, names ...string) string {
 	return ""
 }
 
-func (s *Server) writeAttachment(r *http.Request, v *visitor, m *message, body *util.PeakedReadCloser) error {
+// handlePublishBody consumes the PUT/POST body and decides whether the body is an attachment or the message.
-	contentType := http.DetectContentType(body.PeakedBytes)
+//
-	ext := util.ExtensionByType(contentType)
+// 1. curl -H "Attach: http://example.com/file.jpg" ntfy.sh/mytopic
-	fileURL := fmt.Sprintf("%s/file/%s%s", s.config.BaseURL, m.ID, ext)
+//    Body must be a message, because we attached an external URL
-	filename := readParam(r, "x-filename", "filename", "file", "f")
+// 2. curl -T short.txt -H "Filename: short.txt" ntfy.sh/mytopic
-	if filename == "" {
+//    Body must be attachment, because we passed a filename
-		filename = fmt.Sprintf("attachment%s", ext)
+// 3. curl -T file.txt ntfy.sh/mytopic
+//    If file.txt is <= 4096 (message limit) and valid UTF-8, treat it as a message
+// 4. curl -T file.txt ntfy.sh/mytopic
+//    If file.txt is > message limit, treat it as an attachment
+func (s *Server) handlePublishBody(v *visitor, m *message, body *util.PeakedReadCloser) error {
+	if m.Attachment != nil && m.Attachment.URL != "" {
+		return s.handleBodyAsMessage(m, body) // Case 1
+	} else if m.Attachment != nil && m.Attachment.Name != "" {
+		return s.handleBodyAsAttachment(v, m, body) // Case 2
+	} else if !body.LimitReached && utf8.Valid(body.PeakedBytes) {
+		return s.handleBodyAsMessage(m, body) // Case 3
+	}
+	return s.handleBodyAsAttachment(v, m, body) // Case 4
+}
+
+func (s *Server) handleBodyAsMessage(m *message, body *util.PeakedReadCloser) error {
+	if !utf8.Valid(body.PeakedBytes) {
+		return errHTTPBadRequestMessageNotUTF8
+	}
+	if len(body.PeakedBytes) > 0 { // Empty body should not override message (publish via GET!)
+		m.Message = strings.TrimSpace(string(body.PeakedBytes)) // Truncates the message to the peak limit if required
+	}
+	return nil
+}
+
+func (s *Server) handleBodyAsAttachment(v *visitor, m *message, body *util.PeakedReadCloser) error {
+	if s.fileCache == nil {
+		return errHTTPBadRequestAttachmentsDisallowed
+	} else if m.Time > time.Now().Add(s.config.AttachmentExpiryDuration).Unix() {
+		return errHTTPBadRequestAttachmentsExpiryBeforeDelivery
+	}
+	if m.Attachment == nil {
+		m.Attachment = &attachment{}
+	}
+	var err error
+	m.Attachment.Owner = v.ip // Important for attachment rate limiting
+	m.Attachment.Expires = time.Now().Add(s.config.AttachmentExpiryDuration).Unix()
+	m.Attachment.Type = http.DetectContentType(body.PeakedBytes)
+	ext := util.ExtensionByType(m.Attachment.Type)
+	m.Attachment.URL = fmt.Sprintf("%s/file/%s%s", s.config.BaseURL, m.ID, ext)
+	if m.Attachment.Name == "" {
+		m.Attachment.Name = fmt.Sprintf("attachment%s", ext)
+	}
+	if m.Message == "" {
+		m.Message = fmt.Sprintf("You received a file: %s", m.Attachment.Name)
 	}
 	// TODO do not allowed delayed delivery for attachments
 	visitorAttachmentsSize, err := s.cache.AttachmentsSize(v.ip)
@@ -579,22 +639,13 @@ func (s *Server) writeAttachment(r *http.Request, v *visitor, m *message, body *
 		return err
 	}
 	remainingVisitorAttachmentSize := s.config.VisitorAttachmentTotalSizeLimit - visitorAttachmentsSize
-	log.Printf("remaining visitor: %d", remainingVisitorAttachmentSize)
+	m.Attachment.Size, err = s.fileCache.Write(m.ID, body, util.NewLimiter(remainingVisitorAttachmentSize))
-	size, err := s.fileCache.Write(m.ID, body, util.NewLimiter(remainingVisitorAttachmentSize))
 	if err == util.ErrLimitReached {
 		return errHTTPBadRequestMessageTooLarge
 	} else if err != nil {
 		return err
 	}
-	m.Message = fmt.Sprintf("You received a file: %s", filename) // May be overwritten later
+
-	m.Attachment = &attachment{
-		Name:    filename,
-		Type:    contentType,
-		Size:    size,
-		Expires: time.Now().Add(s.config.AttachmentExpiryDuration).Unix(),
-		URL:     fileURL,
-		Owner:   v.ip, // Important for attachment rate limiting
-	}
 	return nil
 }
 
@@ -965,7 +1016,6 @@ func (s *Server) sendDelayedMessages() error {
 					log.Printf("unable to publish to Firebase: %v", err.Error())
 				}
 			}
-			// TODO delayed email sending
 		}
 		if err := s.cache.MarkPublished(m); err != nil {
 			return err

server/util.go

@@ -0,0 +1,68 @@
+package server
+
+import (
+	"fmt"
+	"heckel.io/ntfy/util"
+	"io"
+	"net/http"
+	"net/url"
+	"path"
+	"strconv"
+	"time"
+)
+
+const (
+	peakAttachmentTimeout    = 2500 * time.Millisecond
+	peakAttachmeantReadBytes = 128
+)
+
+func maybePeakAttachmentURL(m *message) error {
+	return maybePeakAttachmentURLInternal(m, peakAttachmentTimeout)
+}
+
+func maybePeakAttachmentURLInternal(m *message, timeout time.Duration) error {
+	if m.Attachment == nil || m.Attachment.URL == "" {
+		return nil
+	}
+	client := http.Client{
+		Timeout: timeout,
+		Transport: &http.Transport{
+			DisableCompression: true, // Disable "Accept-Encoding: gzip", otherwise we won't get the Content-Length
+			Proxy:              http.ProxyFromEnvironment,
+		},
+	}
+	req, err := http.NewRequest(http.MethodGet, m.Attachment.URL, nil)
+	if err != nil {
+		return err
+	}
+	req.Header.Set("User-Agent", "ntfy")
+	resp, err := client.Do(req)
+	if err != nil {
+		return errHTTPBadRequestAttachmentURLPeakGeneral
+	}
+	defer resp.Body.Close()
+	if resp.StatusCode < 200 || resp.StatusCode > 299 {
+		return errHTTPBadRequestAttachmentURLPeakNon2xx
+	}
+	if size, err := strconv.ParseInt(resp.Header.Get("Content-Length"), 10, 64); err == nil {
+		m.Attachment.Size = size
+	}
+	m.Attachment.Type = resp.Header.Get("Content-Type")
+	if m.Attachment.Type == "" || m.Attachment.Type == "application/octet-stream" {
+		buf := make([]byte, peakAttachmeantReadBytes)
+		io.ReadFull(resp.Body, buf) // Best effort: We don't care about the error
+		m.Attachment.Type = http.DetectContentType(buf)
+	}
+	if m.Attachment.Name == "" {
+		u, err := url.Parse(m.Attachment.URL)
+		if err != nil {
+			m.Attachment.Name = fmt.Sprintf("attachment%s", util.ExtensionByType(m.Attachment.Type))
+		} else {
+			m.Attachment.Name = path.Base(u.Path)
+			if m.Attachment.Name == "." || m.Attachment.Name == "/" {
+				m.Attachment.Name = fmt.Sprintf("attachment%s", util.ExtensionByType(m.Attachment.Type))
+			}
+		}
+	}
+	return nil
+}

server/util_test.go

@@ -0,0 +1,19 @@
+package server
+
+import (
+	"github.com/stretchr/testify/require"
+	"testing"
+)
+
+func TestMaybePeakAttachmentURL_Success(t *testing.T) {
+	m := &message{
+		Attachment: &attachment{
+			URL: "https://ntfy.sh/static/img/ntfy.png",
+		},
+	}
+	require.Nil(t, maybePeakAttachmentURL(m))
+	require.Equal(t, "ntfy.png", m.Attachment.Name)
+	require.Equal(t, int64(3627), m.Attachment.Size)
+	require.Equal(t, "image/png", m.Attachment.Type)
+	require.Equal(t, int64(0), m.Attachment.Expires)
+}

server/visitor.go

@@ -26,7 +26,6 @@ type visitor struct {
 	requests      *rate.Limiter
 	subscriptions *util.Limiter
 	emails        *rate.Limiter
-	attachments   *rate.Limiter
 	seen          time.Time
 	mu            sync.Mutex
 }
@@ -38,7 +37,6 @@ func newVisitor(conf *Config, ip string) *visitor {
 		requests:      rate.NewLimiter(rate.Every(conf.VisitorRequestLimitReplenish), conf.VisitorRequestLimitBurst),
 		subscriptions: util.NewLimiter(int64(conf.VisitorSubscriptionLimit)),
 		emails:        rate.NewLimiter(rate.Every(conf.VisitorEmailLimitReplenish), conf.VisitorEmailLimitBurst),
-		//attachments:   rate.NewLimiter(rate.Every(conf.VisitorAttachmentBytesLimitReplenish * 1024), conf.VisitorAttachmentBytesLimitBurst),
 		seen:          time.Now(),
 	}
 }

util/limit.go

@@ -29,12 +29,11 @@ func NewLimiter(limit int64) *Limiter {
 func (l *Limiter) Add(n int64) error {
 	l.mu.Lock()
 	defer l.mu.Unlock()
-	if l.value+n <= l.limit {
+	if l.value+n > l.limit {
-		l.value += n
-		return nil
-	} else {
 		return ErrLimitReached
 	}
+	l.value += n
+	return nil
 }
 
 // Sub subtracts a value from the limiters internal value