diff --git a/server/server_test.go b/server/server_test.go index 647268fb..d78533e9 100644 --- a/server/server_test.go +++ b/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) diff --git a/server/util.go b/server/util.go index 9cbae2e4..09536765 100644 --- a/server/util.go +++ b/server/util.go @@ -8,11 +8,14 @@ import ( "mime" "net/http" "net/netip" - "strings" "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...)) @@ -51,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), name) + value := strings.TrimSpace(maybeDecodeHeader(name, r.Header.Get(name))) if value != "" { - return strings.TrimSpace(value) + return value } } return "" @@ -127,29 +130,25 @@ func fromContext[T any](r *http.Request, key contextKey) (T, error) { return t, nil } -func maybeDecodeHeader(header string, name string) string { - decoded, err := mimeDecoder.DecodeHeader(header) +// maybeDecodeHeader decodes the given header value if it is MIME encoded, e.g. "=?utf-8?q?Hello_World?=", +// 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 { - if name == "priority"{ - return cloudflarePriorityIgnore(header) - } - return header + return maybeIgnoreSpecialHeader(name, value) } - - if name == "priority"{ - return cloudflarePriorityIgnore(decoded) - } - return decoded + return maybeIgnoreSpecialHeader(name, decoded) } -// 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 the Priority header is set to "u=*, i" or "u=*" (by cloudflare), the header will be ignored. -// And continue searching for another header (x-priority, prio, p) or in the Query parameters. -func cloudflarePriorityIgnore(value string) string { - pattern := `^u=\d,\s(i|\d)$|^u=\d$` - regex := regexp.MustCompile(pattern) - if regex.MatchString(value) { +// 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 diff --git a/server/util_test.go b/server/util_test.go index 3d062b4d..6555a81b 100644 --- a/server/util_test.go +++ b/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")) +} diff --git a/util/util_test.go b/util/util_test.go index 49a24126..f0f45c28 100644 --- a/util/util_test.go +++ b/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"}