From 30a913c05c3d704bd71a9658f13f3e493542f2b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gustavo=20de=20Le=C3=B3n?= Date: Mon, 28 Aug 2023 23:20:04 -0600 Subject: [PATCH 1/5] Ignore Cloudflare Priority header With these changes, If the web request contains the new Priority header (RFC 9218), The server will ignore it and continue searching for other headers or query parameters. --- server/util.go | 23 +++++++++++++++++++++-- util/util.go | 5 ----- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/server/util.go b/server/util.go index 03eb8661..ee87f2ed 100644 --- a/server/util.go +++ b/server/util.go @@ -9,6 +9,7 @@ import ( "net/http" "net/netip" "strings" + /*"regexp"*/ ) var mimeDecoder mime.WordDecoder @@ -129,7 +130,25 @@ func fromContext[T any](r *http.Request, key contextKey) (T, error) { func maybeDecodeHeader(header string) string { decoded, err := mimeDecoder.DecodeHeader(header) if err != nil { - return header + return cloudflarePriorityIgnore(header) } - return decoded + return cloudflarePriorityIgnore(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 { + if strings.HasPrefix(value, "u=") { + return "" + } + + // The same but with regex + /* pattern := `^u=\d+\s*,\s*i|u=\d+$` + regex := regexp.MustCompile(pattern) + if regex.MatchString(value) { + return "" + } */ + return value } diff --git a/util/util.go b/util/util.go index 4a63e22f..d48487df 100644 --- a/util/util.go +++ b/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 } } From 85740d810bf0f6c2e243ce70967c3a7a7981ed0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gustavo=20de=20Le=C3=B3n?= Date: Sun, 3 Sep 2023 18:55:57 -0600 Subject: [PATCH 2/5] Fix cloudflarePriorityIgnore - Now, only if the header being processed is the "priority" header, the cloudflarePriorityIgnore function is called, solving problems with that header injected by CF - we make the check with regex now. --- server/util.go | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/server/util.go b/server/util.go index ee87f2ed..9cbae2e4 100644 --- a/server/util.go +++ b/server/util.go @@ -9,7 +9,7 @@ import ( "net/http" "net/netip" "strings" - /*"regexp"*/ + "regexp" ) var mimeDecoder mime.WordDecoder @@ -51,7 +51,7 @@ 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 := maybeDecodeHeader(r.Header.Get(name), name) if value != "" { return strings.TrimSpace(value) } @@ -127,12 +127,19 @@ func fromContext[T any](r *http.Request, key contextKey) (T, error) { return t, nil } -func maybeDecodeHeader(header string) string { +func maybeDecodeHeader(header string, name string) string { decoded, err := mimeDecoder.DecodeHeader(header) if err != nil { - return cloudflarePriorityIgnore(header) + if name == "priority"{ + return cloudflarePriorityIgnore(header) + } + return header } - return cloudflarePriorityIgnore(decoded) + + if name == "priority"{ + return cloudflarePriorityIgnore(decoded) + } + return decoded } // Ignore new HTTP Priority header (see https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-priority) @@ -140,15 +147,10 @@ func maybeDecodeHeader(header string) string { // 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 { - if strings.HasPrefix(value, "u=") { - return "" - } - - // The same but with regex - /* pattern := `^u=\d+\s*,\s*i|u=\d+$` + pattern := `^u=\d,\s(i|\d)$|^u=\d$` regex := regexp.MustCompile(pattern) if regex.MatchString(value) { return "" - } */ + } return value } From d9387dac994e9c68867337154f96e8c16d692022 Mon Sep 17 00:00:00 2001 From: binwiederhier Date: Sun, 24 Sep 2023 17:59:23 -0400 Subject: [PATCH 3/5] Refine logic --- server/server_test.go | 21 ++++++++++++++++++++ server/util.go | 45 +++++++++++++++++++++---------------------- server/util_test.go | 15 ++++++++++++++- util/util_test.go | 9 --------- 4 files changed, 57 insertions(+), 33 deletions(-) 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"} From bfc1fa51811812f627805552c50b5f29ce60559d Mon Sep 17 00:00:00 2001 From: binwiederhier Date: Sun, 24 Sep 2023 18:03:09 -0400 Subject: [PATCH 4/5] Changelog --- docs/releases.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/releases.md b/docs/releases.md index 222433c3..bc7ed406 100644 --- a/docs/releases.md +++ b/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), 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) From d556a675e9df3478de81c7195c7b19eec5e3f569 Mon Sep 17 00:00:00 2001 From: binwiederhier Date: Sun, 24 Sep 2023 18:04:13 -0400 Subject: [PATCH 5/5] Changelog --- docs/releases.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/releases.md b/docs/releases.md index bc7ed406..4d125a97 100644 --- a/docs/releases.md +++ b/docs/releases.md @@ -1289,7 +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), 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)) +* 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)