Remove message-poll-limit again, hardcode title/tags/etc limits, count polls toward bandwidth limit

2026-10-15 07:57:54 +00:00 · 2026-08-27 19:42:30 +00:00
parent cae7816e5d
commit 7826c86f67
14 changed files with 224 additions and 81 deletions
@@ -4,7 +4,6 @@ import (
 	"crypto/sha256"
 	"encoding/json"
 	"fmt"
-	"heckel.io/ntfy/v2/message"
 	"io/fs"
 	"net/netip"
 	"reflect"
@@ -67,14 +66,23 @@ func banWeight(err *errHTTP, weight int) string {
 // - total topic limit: max number of topics overall
 // - various attachment limits
 const (
-	DefaultMessageSizeLimit            = 4096            // Bytes; note that FCM/APNS have a limit of ~4 KB for the entire message
-	DefaultMessagePollLimit            = message.NoLimit // Uncapped by default; operators cap it to bound replay memory (limit x MessageSizeLimit)
+	DefaultMessageSizeLimit            = 4096 // Bytes; note that FCM/APNS have a limit of ~4 KB for the entire message
 	DefaultTotalTopicLimit             = 15000
 	DefaultAttachmentTotalSizeLimit    = int64(5 * 1024 * 1024 * 1024) // 5 GB
 	DefaultAttachmentFileSizeLimit     = int64(15 * 1024 * 1024)       // 15 MB
 	DefaultAttachmentExpiryDuration    = 3 * time.Hour
 	DefaultAttachmentOrphanGracePeriod = time.Hour // Don't delete orphaned objects younger than this to avoid races with in-flight uploads

+	// DefaultMessagePollSizeLimit caps what one cache replay returns per topic. It is a backstop
+	// against a single request materializing an entire topic cache, not a tunable: on ntfy.sh it
+	// would fire on 2 of ~98k cached topics. See docs/subscribe/api.md#replay-limits.
+	DefaultMessagePollSizeLimit = 10 * 1024 * 1024
+
+	// messageTitleSizeLimit and messageTagsSizeLimit cap two publisher-controlled fields that
+	// otherwise have no limit of their own. Sized off ntfy.sh's own cache: title p999 is 212 bytes
+	// (16 of ~3M messages exceed 1 KB), tags p999 is 244 (197 exceed 512).
+	messageTitleSizeLimit = 1024
+	messageTagsSizeLimit  = 512
 )

 // Defines all per-visitor limits
@@ -176,7 +184,7 @@ type Config struct {
 	MessageDelayMin                      time.Duration
 	MessageDelayMax                      time.Duration
 	MessageSizeLimit                     int
-	MessagePollLimit                     int
+	MessagePollSizeLimit                 int64
 	TotalTopicLimit                      int
 	TotalAttachmentSizeLimit             int64
 	VisitorSubscriptionLimit             int
@@ -285,7 +293,7 @@ func NewConfig() *Config {
 		TwilioVerifyService:                  "",
 		TwilioCallFormat:                     nil,
 		MessageSizeLimit:                     DefaultMessageSizeLimit,
-		MessagePollLimit:                     DefaultMessagePollLimit,
+		MessagePollSizeLimit:                 DefaultMessagePollSizeLimit,
 		MessageDelayMin:                      DefaultMessageDelayMin,
 		MessageDelayMax:                      DefaultMessageDelayMax,
 		TotalTopicLimit:                      DefaultTotalTopicLimit,
@@ -149,6 +149,8 @@ var (
 	errHTTPBadRequestAnonymousEmailNotAllowed        = &errHTTP{40053, http.StatusBadRequest, "invalid request: anonymous email sending is not allowed", "https://ntfy.sh/docs/publish/#e-mail-notifications", nil}
 	errHTTPBadRequestResetLinkInvalid                = &errHTTP{40054, http.StatusBadRequest, "invalid request: password reset link invalid or expired", "", nil}
 	errHTTPBadRequestTemplateTooLarge                = &errHTTP{40056, http.StatusBadRequest, "invalid request: template too large", "https://ntfy.sh/docs/publish/#message-templating", nil}
+	errHTTPBadRequestTitleTooLarge                   = &errHTTP{40057, http.StatusBadRequest, "invalid request: title is too large", "https://ntfy.sh/docs/publish/#limitations", nil}
+	errHTTPBadRequestTagsTooLarge                    = &errHTTP{40058, http.StatusBadRequest, "invalid request: tags are too large", "https://ntfy.sh/docs/publish/#limitations", nil}
 	errHTTPNotFound                                  = &errHTTP{40401, http.StatusNotFound, "page not found", "", nil}
 	errHTTPUnauthorized                              = &errHTTP{40101, http.StatusUnauthorized, "unauthorized", "https://ntfy.sh/docs/publish/#authentication", nil}
 	errHTTPForbidden                                 = &errHTTP{40301, http.StatusForbidden, "forbidden", "https://ntfy.sh/docs/publish/#authentication", nil}
@@ -1147,6 +1147,9 @@ func (s *Server) parsePublishParams(r *http.Request, m *model.Message) (cache bo
 	cache = readBoolParam(r, true, "x-cache", "cache")
 	firebase = readBoolParam(r, true, "x-firebase", "firebase")
 	m.Title = readParam(r, "x-title", "title", "t")
+	if len(m.Title) > messageTitleSizeLimit {
+		return false, false, "", "", "", false, "", errHTTPBadRequestTitleTooLarge
+	}
 	m.Click = readParam(r, "x-click", "click")
 	icon := readParam(r, "x-icon", "icon")
 	filename := readParam(r, "x-filename", "filename", "file", "f")
@@ -1213,6 +1216,14 @@ func (s *Server) parsePublishParams(r *http.Request, m *model.Message) (cache bo
 		priorityStr = "" // Clear since it's already parsed
 	}
 	m.Tags = readCommaSeparatedParam(r, "x-tags", "tags", "tag", "ta")
+	// Measured across all tags, not each one: a publisher can add arbitrarily many
+	tagsSize := 0
+	for _, tag := range m.Tags {
+		tagsSize += len(tag)
+	}
+	if tagsSize > messageTagsSizeLimit {
+		return false, false, "", "", "", false, "", errHTTPBadRequestTagsTooLarge
+	}
 	delayStr := readParam(r, "x-delay", "delay", "x-at", "at", "x-in", "in")
 	if delayStr != "" {
 		if !cache {
@@ -1442,13 +1453,14 @@ func (s *Server) handleSubscribeHTTP(w http.ResponseWriter, r *http.Request, v *
 		if !filters.Pass(msg) {
 			return nil
 		}
-		m, err := encoder(msg)
+		encoded, err := encoder(msg)
 		if err != nil {
 			return err
 		}
-		// Charge before writing, so an exhausted budget fails the first message and surfaces as a
-		// clean 429 with nothing written.
-		if meterPollBandwidth && !v.BandwidthAllowed(int64(len(m))) {
+		// Charge the encoded length, i.e. what actually goes over the wire. Charge before writing,
+		// so an exhausted budget fails the first message and surfaces as a clean 429 with nothing
+		// written.
+		if meterPollBandwidth && !v.BandwidthAllowed(int64(len(encoded))) {
 			return errHTTPTooManyRequestsLimitAttachmentBandwidth
 		}
 		wlock.Lock()
@@ -1456,7 +1468,7 @@ func (s *Server) handleSubscribeHTTP(w http.ResponseWriter, r *http.Request, v *
 		if closed {
 			return nil
 		}
-		if _, err := w.Write([]byte(m)); err != nil {
+		if _, err := w.Write([]byte(encoded)); err != nil {
 			return err
 		}
 		if fl, ok := w.(http.Flusher); ok {
@@ -1741,7 +1753,7 @@ func (s *Server) sendOldMessages(w http.ResponseWriter, topics []*topic, since m
 	messages := make([]*model.Message, 0)
 	truncated := false
 	for _, t := range topics {
-		topicMessages, topicTruncated, err := s.messageCache.MessagesCapped(t.ID, since, scheduled, s.config.MessagePollLimit)
+		topicMessages, topicTruncated, err := s.messageCache.MessagesCapped(t.ID, since, scheduled, s.config.MessagePollSizeLimit)
 		if err != nil {
 			return err
 		}
@@ -1753,8 +1765,9 @@ func (s *Server) sendOldMessages(w http.ResponseWriter, topics []*topic, since m
 	sort.SliceStable(messages, func(i, j int) bool {
 		return messages[i].Time < messages[j].Time
 	})
-	// Must be set before the first message is written, or the header is already on the wire
-	if truncated && w != nil {
+	// Must be set before the first message is written, or the header is already on the wire. On the
+	// WebSocket path the response has been hijacked by then, so this is a no-op there.
+	if truncated {
 		w.Header().Set("X-Messages-Truncated", "1")
 	}
 	for _, m := range messages {
@@ -327,15 +327,6 @@
 # - message-delay-limit defines the max delay of a message when using the "Delay" header.
 #
 # message-size-limit: "4k"
-
-# Max number of cached messages returned per topic when the cache is replayed, i.e. a poll or a
-# "since" request. A poll without a "since" cursor returns a topic's entire cache, which is
-# unbounded in size. Uncapped by default, to preserve the historical behavior; set it to bound
-# how large a single replay can get. The newest messages are kept, and a truncated response
-# carries an "X-Messages-Truncated: 1" header. Together with message-size-limit this bounds the
-# memory one replay can allocate: limit x message-size-limit (e.g. 5000 x 4096 = ~20 MB).
-#
-# message-poll-limit: 5000
 # message-delay-limit: "3d"

 # Rate limiting: Total number of topics before the server rejects new topics.
@@ -2852,29 +2852,68 @@ func TestServer_PollOrderAcrossTopics(t *testing.T) {
 	})
 }

-func TestServer_PollMessageLimit(t *testing.T) {
+func TestServer_PublishTitleTooLarge(t *testing.T) {
 	forEachBackend(t, func(t *testing.T, databaseURL string) {
-		// A poll without "since" replays the whole cache, so the response is unbounded unless the
-		// query is capped. The cap keeps the NEWEST messages and advertises the truncation, since
-		// "since" only moves forward and a client cannot fetch what was dropped.
+		// Title has no length limit of its own, unlike the body, so it is capped here. Prod p999
+		// is 212 bytes and only 16 of ~3M cached messages exceed 1 KB.
+		s := newTestServer(t, newTestConfig(t, databaseURL))
+
+		require.Equal(t, 200, request(t, s, "PUT", "/mytopic", "x", map[string]string{
+			"Title": strings.Repeat("t", messageTitleSizeLimit),
+		}).Code)
+
+		response := request(t, s, "PUT", "/mytopic", "x", map[string]string{
+			"Title": strings.Repeat("t", messageTitleSizeLimit+1),
+		})
+		require.Equal(t, 400, response.Code)
+		require.Equal(t, 40057, toHTTPError(t, response.Body.String()).Code)
+	})
+}
+
+func TestServer_PublishTagsTooLarge(t *testing.T) {
+	forEachBackend(t, func(t *testing.T, databaseURL string) {
+		// Same for tags, measured across all of them: prod p999 is 244 bytes and only 197 of ~3M
+		// cached messages exceed 512.
+		s := newTestServer(t, newTestConfig(t, databaseURL))
+
+		require.Equal(t, 200, request(t, s, "PUT", "/mytopic", "x", map[string]string{
+			"Tags": strings.Repeat("g", messageTagsSizeLimit),
+		}).Code)
+
+		response := request(t, s, "PUT", "/mytopic", "x", map[string]string{
+			"Tags": strings.Repeat("g", messageTagsSizeLimit+1),
+		})
+		require.Equal(t, 400, response.Code)
+		require.Equal(t, 40058, toHTTPError(t, response.Body.String()).Code)
+	})
+}
+
+func TestServer_PollSizeLimit(t *testing.T) {
+	forEachBackend(t, func(t *testing.T, databaseURL string) {
+		// A poll without "since" replays the entire cache, which is unbounded in size. The cap is
+		// a byte budget rather than a message count, because message sizes vary ~20x in practice:
+		// a count cap truncates cheap high-volume topics while barely touching the expensive
+		// large-message ones it is meant to catch. The newest messages are kept.
 		c := newTestConfig(t, databaseURL)
-		c.MessagePollLimit = 5
+		c.MessagePollSizeLimit = 3500 // Fits three 1000-byte messages, not four
 		s := newTestServer(t, c)

-		for i := 1; i <= 8; i++ {
-			require.Equal(t, 200, request(t, s, "PUT", "/mytopic", fmt.Sprintf("message %d", i), nil).Code)
+		for i := 0; i < 6; i++ {
+			body := fmt.Sprintf("%04d%s", i, strings.Repeat("x", 996)) // 1000 bytes, ordered prefix
+			require.Equal(t, 200, request(t, s, "PUT", "/mytopic", body, nil).Code)
 		}

 		response := request(t, s, "GET", "/mytopic/json?poll=1", "", nil)
 		require.Equal(t, 200, response.Code)
 		require.Equal(t, "1", response.Header().Get("X-Messages-Truncated"))
 		messages := toMessages(t, response.Body.String())
-		require.Equal(t, 5, len(messages))
-		require.Equal(t, "message 4", messages[0].Message) // newest 5, oldest first
-		require.Equal(t, "message 8", messages[4].Message)
+		require.Equal(t, 3, len(messages))
+		require.Equal(t, "0003", messages[0].Message[:4]) // newest three, oldest first
+		require.Equal(t, "0004", messages[1].Message[:4])
+		require.Equal(t, "0005", messages[2].Message[:4])

-		// A topic under the limit is served in full, with no truncation header
-		require.Equal(t, 200, request(t, s, "PUT", "/othertopic", "only message", nil).Code)
+		// A topic under the budget is served whole, with no truncation header
+		require.Equal(t, 200, request(t, s, "PUT", "/othertopic", "small", nil).Code)
 		response = request(t, s, "GET", "/othertopic/json?poll=1", "", nil)
 		require.Equal(t, 200, response.Code)
 		require.Empty(t, response.Header().Get("X-Messages-Truncated"))
@@ -2882,6 +2921,59 @@ func TestServer_PollMessageLimit(t *testing.T) {
 	})
 }

+func TestServer_PollSizeLimitCountsTitle(t *testing.T) {
+	forEachBackend(t, func(t *testing.T, databaseURL string) {
+		// Title is user-controlled and has no length limit of its own, so it has to count against
+		// the replay budget too; otherwise a topic of title-heavy messages sails past the cap.
+		c := newTestConfig(t, databaseURL)
+		c.MessagePollSizeLimit = 1600 // Fits one 500-byte title + 500-byte body, not two
+		s := newTestServer(t, c)
+
+		for i := 0; i < 4; i++ {
+			body := fmt.Sprintf("%04d%s", i, strings.Repeat("b", 496))  // 500 bytes
+			title := fmt.Sprintf("%04d%s", i, strings.Repeat("t", 496)) // 500 bytes
+			require.Equal(t, 200, request(t, s, "PUT", "/mytopic", body, map[string]string{"Title": title}).Code)
+		}
+
+		response := request(t, s, "GET", "/mytopic/json?poll=1", "", nil)
+		require.Equal(t, 200, response.Code)
+		require.Equal(t, "1", response.Header().Get("X-Messages-Truncated"))
+		messages := toMessages(t, response.Body.String())
+		require.Equal(t, 1, len(messages)) // 3 if the title were not counted
+		require.Equal(t, "0003", messages[0].Message[:4])
+	})
+}
+
+func TestServer_PollSizeLimitCountsEveryField(t *testing.T) {
+	forEachBackend(t, func(t *testing.T, databaseURL string) {
+		// Every field a publisher can grow has to count against the replay budget, not just the
+		// body and title: tags, click, icon and actions are all user-controlled, so anything left
+		// out is a hole the budget can be walked through.
+		c := newTestConfig(t, databaseURL)
+		c.MessagePollSizeLimit = 900 // Two messages fit if only body+title count; one if all fields do
+		s := newTestServer(t, c)
+
+		tags := make([]string, 5)
+		for i := range tags {
+			tags[i] = strings.Repeat("g", 79) // 395 bytes of tags
+		}
+		for i := 0; i < 3; i++ {
+			require.Equal(t, 200, request(t, s, "PUT", "/mytopic", fmt.Sprintf("%04d%s", i, strings.Repeat("b", 196)), map[string]string{
+				"Title": strings.Repeat("t", 200),
+				"Tags":  strings.Join(tags, ","),
+				"Click": "https://example.com/" + strings.Repeat("c", 180),
+			}).Code)
+		}
+
+		response := request(t, s, "GET", "/mytopic/json?poll=1", "", nil)
+		require.Equal(t, 200, response.Code)
+		require.Equal(t, "1", response.Header().Get("X-Messages-Truncated"))
+		messages := toMessages(t, response.Body.String())
+		require.Equal(t, 1, len(messages)) // 2 if only body+title were counted
+		require.Equal(t, "0002", messages[0].Message[:4])
+	})
+}
+
 func TestServer_PollBandwidthLimit(t *testing.T) {
 	forEachBackend(t, func(t *testing.T, databaseURL string) {
 		// A poll without "since" replays the entire cache, so a topic that is cheap to fill is