From 2f5cfab01cfc0dcb3553547845ccabd549d6aa8f Mon Sep 17 00:00:00 2001 From: Karmanyaah Malhotra Date: Fri, 24 Feb 2023 22:16:03 -0600 Subject: [PATCH 1/3] Fix 507 tests for UnifiedPush subscribe rate limiting --- server/server_test.go | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/server/server_test.go b/server/server_test.go index 8703425..8ab5e12 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -1141,7 +1141,14 @@ func TestServer_PublishUnifiedPushBinary_AndPoll(t *testing.T) { s := newTestServer(t, newTestConfig(t)) - response := request(t, s, "PUT", "/up123456789012?up=1", string(b), nil) + // Register a UnifiedPush subscriber + response := request(t, s, "GET", "/up123456789012/json?poll=1", "", map[string]string{ + "Rate-Topics": "up123456789012", + }) + require.Equal(t, 200, response.Code) + + // Publish message to topic + response = request(t, s, "PUT", "/up123456789012?up=1", string(b), nil) require.Equal(t, 200, response.Code) m := toMessage(t, response.Body.String()) @@ -1150,6 +1157,7 @@ func TestServer_PublishUnifiedPushBinary_AndPoll(t *testing.T) { require.Nil(t, err) require.Equal(t, b, b2) + // Retrieve and check published message response = request(t, s, "GET", "/up123456789012/json?poll=1", string(b), nil) require.Equal(t, 200, response.Code) m = toMessage(t, response.Body.String()) @@ -1165,7 +1173,15 @@ func TestServer_PublishUnifiedPushBinary_Truncated(t *testing.T) { require.Nil(t, err) s := newTestServer(t, newTestConfig(t)) - response := request(t, s, "PUT", "/mytopic?up=1", string(b), nil) + + // Register a UnifiedPush subscriber + response := request(t, s, "GET", "/mytopic/json?poll=1", "", map[string]string{ + "Rate-Topics": "mytopic", + }) + require.Equal(t, 200, response.Code) + + // Publish message to topic + response = request(t, s, "PUT", "/mytopic?up=1", string(b), nil) require.Equal(t, 200, response.Code) m := toMessage(t, response.Body.String()) @@ -1179,7 +1195,14 @@ func TestServer_PublishUnifiedPushBinary_Truncated(t *testing.T) { func TestServer_PublishUnifiedPushText(t *testing.T) { s := newTestServer(t, newTestConfig(t)) - response := request(t, s, "PUT", "/mytopic?up=1", "this is a unifiedpush text message", nil) + // Register a UnifiedPush subscriber + response := request(t, s, "GET", "/mytopic/json?poll=1", "", map[string]string{ + "Rate-Topics": "mytopic", + }) + require.Equal(t, 200, response.Code) + + // Publish UnifiedPush text message + response = request(t, s, "PUT", "/mytopic?up=1", "this is a unifiedpush text message", nil) require.Equal(t, 200, response.Code) m := toMessage(t, response.Body.String()) From fbbfa2bbc1b2fff0a64f0c430bed55c81e12889f Mon Sep 17 00:00:00 2001 From: Karmanyaah Malhotra Date: Fri, 24 Feb 2023 22:47:28 -0600 Subject: [PATCH 2/3] fix matrix tests for new error handling Test driven development --- server/server_test.go | 38 +++++++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/server/server_test.go b/server/server_test.go index 8ab5e12..9ba6569 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -1229,8 +1229,14 @@ func TestServer_MatrixGateway_Discovery_Failure_Unconfigured(t *testing.T) { func TestServer_MatrixGateway_Push_Success(t *testing.T) { s := newTestServer(t, newTestConfig(t)) + + response := request(t, s, "GET", "/mytopic/json?poll=1", "", map[string]string{ + "Rate-Topics": "mytopic", + }) + require.Equal(t, 200, response.Code) + notification := `{"notification":{"devices":[{"pushkey":"http://127.0.0.1:12345/mytopic?up=1"}]}}` - response := request(t, s, "POST", "/_matrix/push/v1/notify", notification, nil) + response = request(t, s, "POST", "/_matrix/push/v1/notify", notification, nil) require.Equal(t, 200, response.Code) require.Equal(t, `{"rejected":[]}`+"\n", response.Body.String()) @@ -1240,6 +1246,36 @@ func TestServer_MatrixGateway_Push_Success(t *testing.T) { require.Equal(t, notification, m.Message) } +func TestServer_MatrixGateway_Push_DailyLimit(t *testing.T) { + c := newTestConfig(t) + c.VisitorMessageDailyLimit = 3 + s := newTestServer(t, c) + + response := request(t, s, "GET", "/mytopic/json?poll=1", "", map[string]string{ + "Rate-Topics": "mytopic", + }) + require.Equal(t, 200, response.Code) + + notification := `{"notification":{"devices":[{"pushkey":"http://127.0.0.1:12345/mytopic?up=1"}]}}` + for i := 0; i < 3; i++ { + response = request(t, s, "POST", "/_matrix/push/v1/notify", notification, nil) + require.Equal(t, 200, response.Code) + require.Equal(t, `{"rejected":[]}`+"\n", response.Body.String()) + } + + response = request(t, s, "POST", "/_matrix/push/v1/notify", notification, nil) + require.Equal(t, 429, response.Code) + require.Equal(t, "42908", response.Header().Get("X-Ntfy-Error-Code")) +} + +func TestServer_MatrixGateway_Push_NoSubscriber(t *testing.T) { + s := newTestServer(t, newTestConfig(t)) + notification := `{"notification":{"devices":[{"pushkey":"http://127.0.0.1:12345/mytopic?up=1"}]}}` + response := request(t, s, "POST", "/_matrix/push/v1/notify", notification, nil) + require.Equal(t, 507, response.Code) + require.Equal(t, `{"rejected":[]}`+"\n", response.Body.String()) +} + func TestServer_MatrixGateway_Push_Failure_InvalidPushkey(t *testing.T) { s := newTestServer(t, newTestConfig(t)) notification := `{"notification":{"devices":[{"pushkey":"http://wrong-base-url.com/mytopic?up=1"}]}}` From 0d7aba94878e0312030a8cbd39ac448a9412d30a Mon Sep 17 00:00:00 2001 From: Karmanyaah Malhotra Date: Fri, 24 Feb 2023 23:56:57 -0600 Subject: [PATCH 3/3] Fix Matrix errors and tests --- server/server.go | 5 +++-- server/server_test.go | 26 ++------------------------ 2 files changed, 5 insertions(+), 26 deletions(-) diff --git a/server/server.go b/server/server.go index 4486a0d..ee1496b 100644 --- a/server/server.go +++ b/server/server.go @@ -661,7 +661,7 @@ func (s *Server) handlePublish(w http.ResponseWriter, r *http.Request, v *visito func (s *Server) handlePublishMatrix(w http.ResponseWriter, r *http.Request, v *visitor) error { _, err := s.handlePublishWithoutResponse(r, v) if err != nil { - return &errMatrix{pushKey: r.Header.Get(matrixPushKeyHeader), err: err} + return err } return writeMatrixSuccess(w) } @@ -1529,7 +1529,8 @@ func (s *Server) transformMatrixJSON(next handleFunc) handleFunc { } if err := next(w, newRequest, v); err != nil { logvr(v, r).Tag(tagMatrix).Err(err).Debug("Error handling Matrix request") - return &errMatrix{pushKey: newRequest.Header.Get(matrixPushKeyHeader), err: err} + // No normal error should cause pushKey rejection; don't set errMatrix.pushKey. + return &errMatrix{err: err} } return nil } diff --git a/server/server_test.go b/server/server_test.go index 9ba6569..4f0eed9 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -1246,29 +1246,7 @@ func TestServer_MatrixGateway_Push_Success(t *testing.T) { require.Equal(t, notification, m.Message) } -func TestServer_MatrixGateway_Push_DailyLimit(t *testing.T) { - c := newTestConfig(t) - c.VisitorMessageDailyLimit = 3 - s := newTestServer(t, c) - - response := request(t, s, "GET", "/mytopic/json?poll=1", "", map[string]string{ - "Rate-Topics": "mytopic", - }) - require.Equal(t, 200, response.Code) - - notification := `{"notification":{"devices":[{"pushkey":"http://127.0.0.1:12345/mytopic?up=1"}]}}` - for i := 0; i < 3; i++ { - response = request(t, s, "POST", "/_matrix/push/v1/notify", notification, nil) - require.Equal(t, 200, response.Code) - require.Equal(t, `{"rejected":[]}`+"\n", response.Body.String()) - } - - response = request(t, s, "POST", "/_matrix/push/v1/notify", notification, nil) - require.Equal(t, 429, response.Code) - require.Equal(t, "42908", response.Header().Get("X-Ntfy-Error-Code")) -} - -func TestServer_MatrixGateway_Push_NoSubscriber(t *testing.T) { +func TestServer_MatrixGateway_Push_Failure_NoSubscriber(t *testing.T) { s := newTestServer(t, newTestConfig(t)) notification := `{"notification":{"devices":[{"pushkey":"http://127.0.0.1:12345/mytopic?up=1"}]}}` response := request(t, s, "POST", "/_matrix/push/v1/notify", notification, nil) @@ -2099,7 +2077,7 @@ func TestServer_Matrix_SubscriberRateLimiting_UP_Only(t *testing.T) { } response := request(t, s, "POST", "/_matrix/push/v1/notify", notification, nil) require.Equal(t, 429, response.Code, notification) - require.Equal(t, fmt.Sprintf(`{"rejected":["http://127.0.0.1:12345/up12345678901%d?up=1"]}`+"\n", i), response.Body.String()) + require.Equal(t, `{"rejected":[]}`+"\n", response.Body.String()) } }