Merge pull request #895 from binwiederhier/cf-priority

Refined `Priority` header handling

docs/releases.md

@@ -1289,6 +1289,7 @@ and the [ntfy Android app](https://github.com/binwiederhier/ntfy-android/release
 
 * Fix ACL issue with topic patterns containing underscores ([#840](https://github.com/binwiederhier/ntfy/issues/840), thanks to [@Joe-0237](https://github.com/Joe-0237) for reporting)
 * Re-add `tzdata` to Docker images for amd64 image ([#894](https://github.com/binwiederhier/ntfy/issues/894), [#307](https://github.com/binwiederhier/ntfy/pull/307))
+* Add special logic to ignore `Priority` header if it resembled a RFC 9218 value ([#851](https://github.com/binwiederhier/ntfy/pull/851)/[#895](https://github.com/binwiederhier/ntfy/pull/895), thanks to [@gusdleon](https://github.com/gusdleon), see also [#351](https://github.com/binwiederhier/ntfy/issues/351), [#353](https://github.com/binwiederhier/ntfy/issues/353), [#461](https://github.com/binwiederhier/ntfy/issues/461))
 
 ### ntfy Android app v1.16.1 (UNRELEASED)
 

server/server_test.go

@@ -329,6 +329,27 @@ func TestServer_PublishPriority(t *testing.T) {
 	require.Equal(t, 40007, toHTTPError(t, response.Body.String()).Code)
 }
 
+func TestServer_PublishPriority_SpecialHTTPHeader(t *testing.T) {
+	s := newTestServer(t, newTestConfig(t))
+
+	response := request(t, s, "POST", "/mytopic", "test", map[string]string{
+		"Priority":   "u=4",
+		"X-Priority": "5",
+	})
+	require.Equal(t, 5, toMessage(t, response.Body.String()).Priority)
+
+	response = request(t, s, "POST", "/mytopic?priority=4", "test", map[string]string{
+		"Priority": "u=9",
+	})
+	require.Equal(t, 4, toMessage(t, response.Body.String()).Priority)
+
+	response = request(t, s, "POST", "/mytopic", "test", map[string]string{
+		"p":        "2",
+		"priority": "u=9, i",
+	})
+	require.Equal(t, 2, toMessage(t, response.Body.String()).Priority)
+}
+
 func TestServer_PublishGETOnlyOneTopic(t *testing.T) {
 	// This tests a bug that allowed publishing topics with a comma in the name (no ticket)
 

server/util.go

@@ -8,10 +8,14 @@ import (
 	"mime"
 	"net/http"
 	"net/netip"
+	"regexp"
 	"strings"
 )
 
-var mimeDecoder mime.WordDecoder
+var (
+	mimeDecoder               mime.WordDecoder
+	priorityHeaderIgnoreRegex = regexp.MustCompile(`^u=\d,\s*(i|\d)$|^u=\d$`)
+)
 
 func readBoolParam(r *http.Request, defaultValue bool, names ...string) bool {
 	value := strings.ToLower(readParam(r, names...))
@@ -50,9 +54,9 @@ func readParam(r *http.Request, names ...string) string {
 
 func readHeaderParam(r *http.Request, names ...string) string {
 	for _, name := range names {
-		value := maybeDecodeHeader(r.Header.Get(name))
+		value := strings.TrimSpace(maybeDecodeHeader(name, r.Header.Get(name)))
 		if value != "" {
-			return strings.TrimSpace(value)
+			return value
 		}
 	}
 	return ""
@@ -126,10 +130,26 @@ func fromContext[T any](r *http.Request, key contextKey) (T, error) {
 	return t, nil
 }
 
-func maybeDecodeHeader(header string) string {
+// maybeDecodeHeader decodes the given header value if it is MIME encoded, e.g. "=?utf-8?q?Hello_World?=",
-	decoded, err := mimeDecoder.DecodeHeader(header)
+// or returns the original header value if it is not MIME encoded. It also calls maybeIgnoreSpecialHeader
+// to ignore new HTTP "Priority" header.
+func maybeDecodeHeader(name, value string) string {
+	decoded, err := mimeDecoder.DecodeHeader(value)
 	if err != nil {
-		return header
+		return maybeIgnoreSpecialHeader(name, value)
 	}
-	return decoded
+	return maybeIgnoreSpecialHeader(name, decoded)
+}
+
+// maybeIgnoreSpecialHeader ignores new HTTP "Priority" header (see https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-priority)
+//
+// Cloudflare (and potentially other providers) add this to requests when forwarding to the backend (ntfy),
+// so we just ignore it. If the "Priority" header is set to "u=*, i" or "u=*" (by Cloudflare), the header will be ignored.
+// Returning an empty string will allow the rest of the logic to continue searching for another header (x-priority, prio, p),
+// or in the Query parameters.
+func maybeIgnoreSpecialHeader(name, value string) string {
+	if strings.ToLower(name) == "priority" && priorityHeaderIgnoreRegex.MatchString(strings.TrimSpace(value)) {
+		return ""
+	}
+	return value
 }

server/util_test.go

@@ -2,9 +2,9 @@ package server
 
 import (
 	"bytes"
+	"crypto/rand"
 	"fmt"
 	"github.com/stretchr/testify/require"
-	"math/rand"
 	"net/http"
 	"strings"
 	"testing"
@@ -75,3 +75,16 @@ Accept: */*
 (peeked bytes not UTF-8, peek limit of 4096 bytes reached, hex: ` + fmt.Sprintf("%x", body[:4096]) + ` ...)`
 	require.Equal(t, expected, renderHTTPRequest(r))
 }
+
+func TestMaybeIgnoreSpecialHeader(t *testing.T) {
+	require.Empty(t, maybeIgnoreSpecialHeader("priority", "u=1"))
+	require.Empty(t, maybeIgnoreSpecialHeader("Priority", "u=1"))
+	require.Empty(t, maybeIgnoreSpecialHeader("Priority", "u=1, i"))
+}
+
+func TestMaybeDecodeHeaders(t *testing.T) {
+	r, _ := http.NewRequest("GET", "http://ntfy.sh/mytopic/json?since=all", nil)
+	r.Header.Set("Priority", "u=1") // Cloudflare priority header
+	r.Header.Set("X-Priority", "5") // ntfy priority header
+	require.Equal(t, "5", readHeaderParam(r, "x-priority", "priority", "p"))
+}

util/util.go

@@ -161,11 +161,6 @@ func ParsePriority(priority string) (int, error) {
 	case "5", "max", "urgent":
 		return 5, nil
 	default:
-		// Ignore new HTTP Priority header (see https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-priority)
-		// Cloudflare adds this to requests when forwarding to the backend (ntfy), so we just ignore it.
-		if strings.HasPrefix(p, "u=") {
-			return 3, nil
-		}
 		return 0, errInvalidPriority
 	}
 }

util/util_test.go

@@ -87,15 +87,6 @@ func TestParsePriority_Invalid(t *testing.T) {
 	}
 }
 
-func TestParsePriority_HTTPSpecPriority(t *testing.T) {
-	priorities := []string{"u=1", "u=3", "u=7, i"} // see https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-priority
-	for _, priority := range priorities {
-		actual, err := ParsePriority(priority)
-		require.Nil(t, err)
-		require.Equal(t, 3, actual) // Always expect 3!
-	}
-}
-
 func TestPriorityString(t *testing.T) {
 	priorities := []int{0, 1, 2, 3, 4, 5}
 	expected := []string{"default", "min", "low", "default", "high", "max"}