From d082ec7ab99dab60ed4ff923560e94892d4e5bb8 Mon Sep 17 00:00:00 2001
From: Shivaram Lingamneni <slingamn@cs.stanford.edu>
Date: Wed, 31 May 2023 20:22:16 -0700
Subject: [PATCH] don't send multiline responses to CAP LS 301 (#2068)

* don't send multiline responses to CAP LS 301

This is more or less explicitly prohibited by the spec:

https://ircv3.net/specs/extensions/capability-negotiation.html#multiline-replies-to-cap-ls-and-cap-list

* switch to whitelist model to be future-proof

* bump irctest to include test

* add a unit test
---
 irc/caps/set.go      | 19 +++++++++++++++++++
 irc/caps/set_test.go | 20 ++++++++++++++++++--
 irctest              |  2 +-
 3 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/irc/caps/set.go b/irc/caps/set.go
index fbd72bd5..26251f2e 100644
--- a/irc/caps/set.go
+++ b/irc/caps/set.go
@@ -102,6 +102,13 @@ func (s *Set) Strings(version Version, values Values, maxLen int) (result []stri
 	var capab Capability
 	asSlice := s[:]
 	for capab = 0; capab < numCapabs; capab++ {
+		// XXX clients that only support CAP LS 301 cannot handle multiline
+		// responses. omit some CAPs in this case, forcing the response to fit on
+		// a single line. this is technically buggy for CAP LIST (as opposed to LS)
+		// but it shouldn't matter
+		if version < Cap302 && !isAllowed301(capab) {
+			continue
+		}
 		// skip any capabilities that are not enabled
 		if !utils.BitsetGet(asSlice, uint(capab)) {
 			continue
@@ -122,3 +129,15 @@ func (s *Set) Strings(version Version, values Values, maxLen int) (result []stri
 	}
 	return
 }
+
+// this is a fixed whitelist of caps that are eligible for display in CAP LS 301
+func isAllowed301(capab Capability) bool {
+	switch capab {
+	case AccountNotify, AccountTag, AwayNotify, Batch, ChgHost, Chathistory, EventPlayback,
+		Relaymsg, EchoMessage, Nope, ExtendedJoin, InviteNotify, LabeledResponse, MessageTags,
+		MultiPrefix, SASL, ServerTime, SetName, STS, UserhostInNames, ZNCSelfMessage, ZNCPlayback:
+		return true
+	default:
+		return false
+	}
+}
diff --git a/irc/caps/set_test.go b/irc/caps/set_test.go
index 4943ec91..d807e5be 100644
--- a/irc/caps/set_test.go
+++ b/irc/caps/set_test.go
@@ -3,8 +3,11 @@
 
 package caps
 
-import "testing"
-import "reflect"
+import (
+	"fmt"
+	"reflect"
+	"testing"
+)
 
 func TestSets(t *testing.T) {
 	s1 := NewSet()
@@ -60,6 +63,19 @@ func TestSets(t *testing.T) {
 	}
 }
 
+func assertEqual(found, expected interface{}) {
+	if !reflect.DeepEqual(found, expected) {
+		panic(fmt.Sprintf("found %#v, expected %#v", found, expected))
+	}
+}
+
+func Test301WhitelistNotRespectedFor302(t *testing.T) {
+	s1 := NewSet()
+	s1.Enable(AccountTag, EchoMessage, StandardReplies)
+	assertEqual(s1.Strings(Cap301, nil, 0), []string{"account-tag echo-message"})
+	assertEqual(s1.Strings(Cap302, nil, 0), []string{"account-tag echo-message standard-replies"})
+}
+
 func TestSubtract(t *testing.T) {
 	s1 := NewSet(AccountTag, EchoMessage, UserhostInNames, ServerTime)
 
diff --git a/irctest b/irctest
index bb8a6b6c..22c6743b 160000
--- a/irctest
+++ b/irctest
@@ -1 +1 @@
-Subproject commit bb8a6b6c3d3e55c1146c3c9f8224983d88a42b17
+Subproject commit 22c6743b24f2a85bf79a92fb0c7fab325c047a6c