From 41e3d1af10579de6c7a718906061d03b38aba613 Mon Sep 17 00:00:00 2001
From: Mechiel Lukkien <mechiel@ueber.net>
Date: Thu, 14 Dec 2023 20:09:13 +0100
Subject: [PATCH] imapserver: only send OLDNAME in LIST responses when
 IMAP4rev2 was enabled

OLDNAME is included in IMAP4rev2, but not in IMAP4rev1. it is also included in
the NOTIFY extension, but we don't implement that yet.

found by Damian Poddebniak with https://github.com/duesee/imap-flow, thanks!
---
 imapserver/create_test.go |  9 +++++++--
 imapserver/rename_test.go | 10 ++++++++--
 imapserver/server.go      | 16 +++++++++++-----
 3 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/imapserver/create_test.go b/imapserver/create_test.go
index eeda056..39837e1 100644
--- a/imapserver/create_test.go
+++ b/imapserver/create_test.go
@@ -36,10 +36,15 @@ func TestCreate(t *testing.T) {
 
 	// ../rfc/9051:1934
 	tc.transactf("ok", "create mailbox/")
-	tc.xuntagged(imapclient.UntaggedList{Flags: []string{`\Subscribed`}, Separator: '/', Mailbox: "mailbox", OldName: "mailbox/"})
+	tc.xuntagged(imapclient.UntaggedList{Flags: []string{`\Subscribed`}, Separator: '/', Mailbox: "mailbox"})
+
+	// OldName is only set for IMAP4rev2 or NOTIFY.
+	tc.client.Enable("imap4rev2")
+	tc.transactf("ok", "create mailbox2/")
+	tc.xuntagged(imapclient.UntaggedList{Flags: []string{`\Subscribed`}, Separator: '/', Mailbox: "mailbox2", OldName: "mailbox2/"})
 
 	tc2.transactf("ok", "noop")
-	tc2.xuntagged(imapclient.UntaggedList{Flags: []string{`\Subscribed`}, Separator: '/', Mailbox: "mailbox"})
+	tc2.xuntagged(imapclient.UntaggedList{Flags: []string{`\Subscribed`}, Separator: '/', Mailbox: "mailbox"}, imapclient.UntaggedList{Flags: []string{`\Subscribed`}, Separator: '/', Mailbox: "mailbox2"})
 
 	// If we are already subscribed, create should still work, and we still want to see the subscribed flag.
 	tc.transactf("ok", "subscribe newbox")
diff --git a/imapserver/rename_test.go b/imapserver/rename_test.go
index f19ffd8..ff90d18 100644
--- a/imapserver/rename_test.go
+++ b/imapserver/rename_test.go
@@ -32,9 +32,15 @@ func TestRename(t *testing.T) {
 	tc.client.Subscribe("x/y/c") // For later rename, but not affected by rename of x.
 	tc2.transactf("ok", "noop")  // Drain.
 
-	tc.transactf("ok", "rename x y")
+	tc.transactf("ok", "rename x z")
 	tc2.transactf("ok", "noop")
-	tc2.xuntagged(imapclient.UntaggedList{Separator: '/', Mailbox: "y", OldName: "x"})
+	tc2.xuntagged(imapclient.UntaggedList{Separator: '/', Mailbox: "z"})
+
+	// OldName is only set for IMAP4rev2 or NOTIFY.
+	tc2.client.Enable("IMAP4rev2")
+	tc.transactf("ok", "rename z y")
+	tc2.transactf("ok", "noop")
+	tc2.xuntagged(imapclient.UntaggedList{Separator: '/', Mailbox: "y", OldName: "z"})
 
 	// Rename to a mailbox that only exists in database as subscribed.
 	tc.transactf("ok", "rename y sub")
diff --git a/imapserver/server.go b/imapserver/server.go
index fb9a5fe..0322687 100644
--- a/imapserver/server.go
+++ b/imapserver/server.go
@@ -1311,7 +1311,12 @@ func (c *conn) applyChanges(changes []store.Change, initial bool) {
 		case store.ChangeAddMailbox:
 			c.bwritelinef(`* LIST (%s) "/" %s`, strings.Join(ch.Flags, " "), astring(ch.Mailbox.Name).pack(c))
 		case store.ChangeRenameMailbox:
-			c.bwritelinef(`* LIST (%s) "/" %s ("OLDNAME" (%s))`, strings.Join(ch.Flags, " "), astring(ch.NewName).pack(c), string0(ch.OldName).pack(c))
+			// OLDNAME only with IMAP4rev2 or NOTIFY ../rfc/9051:2726 ../rfc/5465:628
+			var oldname string
+			if c.enabled[capIMAP4rev2] {
+				oldname = fmt.Sprintf(` ("OLDNAME" (%s))`, string0(ch.OldName).pack(c))
+			}
+			c.bwritelinef(`* LIST (%s) "/" %s%s`, strings.Join(ch.Flags, " "), astring(ch.NewName).pack(c), oldname)
 		case store.ChangeAddSubscription:
 			c.bwritelinef(`* LIST (%s) "/" %s`, strings.Join(append([]string{`\Subscribed`}, ch.Flags...), " "), astring(ch.Name).pack(c))
 		default:
@@ -2211,11 +2216,12 @@ func (c *conn) cmdCreate(tag, cmd string, p *parser) {
 	})
 
 	for _, n := range created {
-		var more string
-		if n == name && name != origName && !(name == "Inbox" || strings.HasPrefix(name, "Inbox/")) {
-			more = fmt.Sprintf(` ("OLDNAME" (%s))`, string0(origName).pack(c))
+		var oldname string
+		// OLDNAME only with IMAP4rev2 or NOTIFY ../rfc/9051:2726 ../rfc/5465:628
+		if c.enabled[capIMAP4rev2] && n == name && name != origName && !(name == "Inbox" || strings.HasPrefix(name, "Inbox/")) {
+			oldname = fmt.Sprintf(` ("OLDNAME" (%s))`, string0(origName).pack(c))
 		}
-		c.bwritelinef(`* LIST (\Subscribed) "/" %s%s`, astring(n).pack(c), more)
+		c.bwritelinef(`* LIST (\Subscribed) "/" %s%s`, astring(n).pack(c), oldname)
 	}
 	c.ok(tag, cmd)
 }