From 38e7801b419b868347e0904cc8d1cd5ed65c5233 Mon Sep 17 00:00:00 2001
From: binwiederhier <pheckel@datto.com>
Date: Fri, 17 Feb 2023 15:56:48 -0500
Subject: [PATCH] Fix panic in manager when `attachment-cache-dir` is not set,
fixes #617
---
docs/releases.md | 16 +++++++----
server/message_cache.go | 2 +-
server/server_account.go | 1 +
server/server_manager.go | 51 ++++++++++++++++++-----------------
server/server_manager_test.go | 28 +++++++++++++++++++
5 files changed, 68 insertions(+), 30 deletions(-)
create mode 100644 server/server_manager_test.go
diff --git a/docs/releases.md b/docs/releases.md
index 82cd2db6..17c24308 100644
--- a/docs/releases.md
+++ b/docs/releases.md
@@ -2,6 +2,12 @@
Binaries for all releases can be found on the GitHub releases pages for the [ntfy server](https://github.com/binwiederhier/ntfy/releases)
and the [ntfy Android app](https://github.com/binwiederhier/ntfy-android/releases).
+## ntfy server v2.0.1 (UNRELEASED)
+
+**Bug fixes + maintenance:**
+
+* Avoid panic in manager when `attachment-cache-dir` is not set ([#617](https://github.com/binwiederhier/ntfy/issues/617), thanks to [@ksurl](https://github.com/ksurl))
+
## ntfy server v2.0.0
Released February 16, 2023
@@ -64,6 +70,11 @@ going. It'll only make ntfy better.
* User account signup, login, topic reservations, access tokens, tiers etc. ([#522](https://github.com/binwiederhier/ntfy/issues/522))
* `OPTIONS` method calls are not serviced when the UI is disabled ([#598](https://github.com/binwiederhier/ntfy/issues/598), thanks to [@enticedwanderer](https://github.com/enticedwanderer) for reporting)
+**Special thanks:**
+
+A big Thank-you goes to everyone who tested the user account and payments work. I very much appreciate all the feedback,
+suggestions, and bug reports. Thank you, @nwithan8, @deadcade, @xenrox, @cmeis, and the others who I forgot.
+
## ntfy server v1.31.0
Released February 14, 2023
@@ -95,11 +106,6 @@ breaking-change upgrade, which required some work to get working again.
* Portuguese (thanks to [@ssantos](https://hosted.weblate.org/user/ssantos/))
-**Special thanks:**
-
-A big Thank-you goes to everyone who tested the user account and payments work. I very much appreciate all the feedback,
-suggestions, and bug reports. Thank you, @nwithan8, @deadcade, and @xenrox.
-
## ntfy server v1.30.1
Released December 23, 2022 🎅
diff --git a/server/message_cache.go b/server/message_cache.go
index 37683d6e..a8b8ff1a 100644
--- a/server/message_cache.go
+++ b/server/message_cache.go
@@ -536,7 +536,7 @@ func (c *messageCache) ExpireMessages(topics ...string) error {
}
defer tx.Rollback()
for _, t := range topics {
- if _, err := tx.Exec(updateMessagesForTopicExpiryQuery, time.Now().Unix(), t); err != nil {
+ if _, err := tx.Exec(updateMessagesForTopicExpiryQuery, time.Now().Unix()-1, t); err != nil {
return err
}
}
diff --git a/server/server_account.go b/server/server_account.go
index aff9f1b1..26ef7216 100644
--- a/server/server_account.go
+++ b/server/server_account.go
@@ -506,6 +506,7 @@ func (s *Server) maybeRemoveMessagesAndExcessReservations(r *http.Request, v *vi
if err := s.messageCache.ExpireMessages(topics...); err != nil {
return err
}
+ go s.pruneMessages()
return nil
}
diff --git a/server/server_manager.go b/server/server_manager.go
index 2b80ae18..cef1e227 100644
--- a/server/server_manager.go
+++ b/server/server_manager.go
@@ -116,29 +116,30 @@ func (s *Server) pruneTokens() {
}
func (s *Server) pruneAttachments() {
- if s.fileCache != nil {
- log.
- Tag(tagManager).
- Timing(func() {
- ids, err := s.messageCache.AttachmentsExpired()
- if err != nil {
- log.Tag(tagManager).Err(err).Warn("Error retrieving expired attachments")
- } else if len(ids) > 0 {
- if log.Tag(tagManager).IsDebug() {
- log.Tag(tagManager).Debug("Deleting attachments %s", strings.Join(ids, ", "))
- }
- if err := s.fileCache.Remove(ids...); err != nil {
- log.Tag(tagManager).Err(err).Warn("Error deleting attachments")
- }
- if err := s.messageCache.MarkAttachmentsDeleted(ids...); err != nil {
- log.Tag(tagManager).Err(err).Warn("Error marking attachments deleted")
- }
- } else {
- log.Tag(tagManager).Debug("No expired attachments to delete")
- }
- }).
- Debug("Deleted expired attachments")
+ if s.fileCache == nil {
+ return
}
+ log.
+ Tag(tagManager).
+ Timing(func() {
+ ids, err := s.messageCache.AttachmentsExpired()
+ if err != nil {
+ log.Tag(tagManager).Err(err).Warn("Error retrieving expired attachments")
+ } else if len(ids) > 0 {
+ if log.Tag(tagManager).IsDebug() {
+ log.Tag(tagManager).Debug("Deleting attachments %s", strings.Join(ids, ", "))
+ }
+ if err := s.fileCache.Remove(ids...); err != nil {
+ log.Tag(tagManager).Err(err).Warn("Error deleting attachments")
+ }
+ if err := s.messageCache.MarkAttachmentsDeleted(ids...); err != nil {
+ log.Tag(tagManager).Err(err).Warn("Error marking attachments deleted")
+ }
+ } else {
+ log.Tag(tagManager).Debug("No expired attachments to delete")
+ }
+ }).
+ Debug("Deleted expired attachments")
}
func (s *Server) pruneMessages() {
@@ -149,8 +150,10 @@ func (s *Server) pruneMessages() {
if err != nil {
log.Tag(tagManager).Err(err).Warn("Error retrieving expired messages")
} else if len(expiredMessageIDs) > 0 {
- if err := s.fileCache.Remove(expiredMessageIDs...); err != nil {
- log.Tag(tagManager).Err(err).Warn("Error deleting attachments for expired messages")
+ if s.fileCache != nil {
+ if err := s.fileCache.Remove(expiredMessageIDs...); err != nil {
+ log.Tag(tagManager).Err(err).Warn("Error deleting attachments for expired messages")
+ }
}
if err := s.messageCache.DeleteMessages(expiredMessageIDs...); err != nil {
log.Tag(tagManager).Err(err).Warn("Error marking attachments deleted")
diff --git a/server/server_manager_test.go b/server/server_manager_test.go
new file mode 100644
index 00000000..f17d583f
--- /dev/null
+++ b/server/server_manager_test.go
@@ -0,0 +1,28 @@
+package server
+
+import (
+ "github.com/stretchr/testify/require"
+ "testing"
+)
+
+func TestServer_Manager_Prune_Messages_Without_Attachments_DoesNotPanic(t *testing.T) {
+ // Tests that the manager runs without attachment-cache-dir set, see #617
+ c := newTestConfig(t)
+ c.AttachmentCacheDir = ""
+ s := newTestServer(t, c)
+
+ // Publish a message
+ rr := request(t, s, "POST", "/mytopic", "hi", nil)
+ require.Equal(t, 200, rr.Code)
+ m := toMessage(t, rr.Body.String())
+
+ // Expire message
+ require.Nil(t, s.messageCache.ExpireMessages("mytopic"))
+
+ // Does not panic
+ s.pruneMessages()
+
+ // Actually deleted
+ _, err := s.messageCache.Message(m.ID)
+ require.Equal(t, errMessageNotFound, err)
+}